Skip to content

Commit

Permalink
track: only print snapshot stats and hints once
Browse files Browse the repository at this point in the history
Move the `track` specific logic to track.rs, let print_snapshot_stats
handle just the default case.

Previously print_snapshot_stats would get called twice and could warn
twice about a file (if that file was both in the auto-track fileset as
well as in the fileset passed to `jj file track`), so now we merge the
snapshot stats and display appropriate hints for each file.
  • Loading branch information
alarsyo committed Feb 18, 2025
1 parent d4ab59c commit 4b6d4f6
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 83 deletions.
80 changes: 22 additions & 58 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,12 +414,7 @@ impl CommandHelper {
#[instrument(skip(self, ui))]
pub fn workspace_helper(&self, ui: &Ui) -> Result<WorkspaceCommandHelper, CommandError> {
let (workspace_command, stats) = self.workspace_helper_with_stats(ui)?;
print_snapshot_stats(
ui,
&stats,
workspace_command.env().path_converter(),
SnapshotContext::Automatic,
)?;
print_snapshot_stats(ui, &stats, workspace_command.env().path_converter())?;
Ok(workspace_command)
}

Expand Down Expand Up @@ -1101,12 +1096,7 @@ impl WorkspaceCommandHelper {
let stats = self
.maybe_snapshot_impl(ui)
.map_err(|err| err.into_command_error())?;
print_snapshot_stats(
ui,
&stats,
self.path_converter(),
SnapshotContext::Automatic,
)?;
print_snapshot_stats(ui, &stats, self.path_converter())?;
Ok(stats)
}

Expand Down Expand Up @@ -2658,13 +2648,6 @@ pub fn print_conflicted_paths(
Ok(())
}

pub enum SnapshotContext {
Automatic,
Status,
Track,
Untrack,
}

/// Build human-readable messages explaining why the file was not tracked
fn build_untracked_reason_message<'a>(
(path, reason): (&'a RepoPathBuf, &'a UntrackedReason),
Expand Down Expand Up @@ -2717,48 +2700,29 @@ pub fn print_snapshot_stats(
ui: &Ui,
stats: &SnapshotStats,
path_converter: &RepoPathUiConverter,
context: SnapshotContext,
) -> io::Result<()> {
print_untracked_files(ui, &stats.untracked_paths, path_converter)?;

let large_files = stats
.untracked_paths
.iter()
.filter_map(|(path, reason)| match reason {
UntrackedReason::FileTooLarge { size, .. } => Some((path, size)),
UntrackedReason::FileNotAutoTracked => None,
})
.collect_vec();
if let Some(size) = large_files.iter().map(|(_, size)| size).max() {
match context {
SnapshotContext::Track => {
let large_files_list = large_files
.iter()
.map(|(p, _)| path_converter.format_file_path(p))
.join(" ");
writedoc!(
ui.hint_default(),
r"
This is to prevent large files from being added by accident. You can fix this by:
- Run `jj config set --repo snapshot.max-new-file-size {size}`
This will increase the maximum file size allowed for new files, in this repository only.
- Run `jj --config snapshot.max-new-file-size={size} file track {large_files_list}`
This will increase the maximum file size allowed for new files, for this command only.
"
)?;
}
_ => writedoc!(
ui.hint_default(),
r"
This is to prevent large files from being added by accident. You can fix this by:
- Adding the file to `.gitignore`
- Run `jj config set --repo snapshot.max-new-file-size {size}`
This will increase the maximum file size allowed for new files, in this repository only.
- Run `jj --config snapshot.max-new-file-size={size} st`
This will increase the maximum file size allowed for new files, for this command only.
"
)?,
};
let large_files_sizes =
stats
.untracked_paths
.iter()
.filter_map(|(_path, reason)| match reason {
UntrackedReason::FileTooLarge { size, .. } => Some(size),
UntrackedReason::FileNotAutoTracked => None,
});
if let Some(size) = large_files_sizes.max() {
writedoc!(
ui.hint_default(),
r"
This is to prevent large files from being added by accident. You can fix this by:
- Adding the file to `.gitignore`
- Run `jj config set --repo snapshot.max-new-file-size {size}`
This will increase the maximum file size allowed for new files, in this repository only.
- Run `jj --config snapshot.max-new-file-size={size} st`
This will increase the maximum file size allowed for new files, for this command only.
"
)?;
}
Ok(())
}
Expand Down
84 changes: 78 additions & 6 deletions cli/src/commands/file/track.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,21 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::io;
use std::io::Write;
use std::ops::Not;

use indoc::writedoc;
use itertools::Itertools;
use jj_lib::matchers::Matcher;
use jj_lib::repo_path::RepoPathUiConverter;
use jj_lib::working_copy::SnapshotStats;
use jj_lib::working_copy::UntrackedReason;
use tracing::instrument;

use crate::cli_util::print_snapshot_stats;
use crate::cli_util::print_untracked_files;
use crate::cli_util::CommandHelper;
use crate::cli_util::SnapshotContext;
use crate::command_error::CommandError;
use crate::ui::Ui;

Expand All @@ -44,26 +52,90 @@ pub(crate) fn cmd_file_track(
command: &CommandHelper,
args: &FileTrackArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let (mut workspace_command, auto_stats) = command.workspace_helper_with_stats(ui)?;
let matcher = workspace_command
.parse_file_patterns(ui, &args.paths)?
.to_matcher();

let options = workspace_command.snapshot_options_with_start_tracking_matcher(&matcher)?;

let mut tx = workspace_command.start_transaction().into_inner();
let (mut locked_ws, _wc_commit) = workspace_command.start_working_copy_mutation()?;
let (_tree_id, stats) = locked_ws.locked_wc().snapshot(&options)?;
let (_tree_id, track_stats) = locked_ws.locked_wc().snapshot(&options)?;
let num_rebased = tx.repo_mut().rebase_descendants()?;
if num_rebased > 0 {
writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?;
}
let repo = tx.commit("track paths")?;
locked_ws.finish(repo.op_id().clone())?;
print_snapshot_stats(
print_track_snapshot_stats(
ui,
&stats,
auto_stats,
track_stats,
&matcher,
workspace_command.env().path_converter(),
SnapshotContext::Track,
)?;
Ok(())
}

pub fn print_track_snapshot_stats(
ui: &Ui,
auto_stats: SnapshotStats,
track_stats: SnapshotStats,
track_matcher: &dyn Matcher,
path_converter: &RepoPathUiConverter,
) -> io::Result<()> {
if track_stats
.untracked_paths
.iter()
.any(|(path, _)| track_matcher.matches(path))
.not()
{
// we can use the "normal" report path, nothing special happened to a file we
// care about
return print_snapshot_stats(ui, &auto_stats, path_converter);
}

let mut merged_untracked_paths = auto_stats.untracked_paths;
for (path, reason) in track_stats
.untracked_paths
.into_iter()
// focus on files that are now tracked with `file track`
.filter(|(_, reason)| matches!(reason, UntrackedReason::FileNotAutoTracked).not())
{
// if the path was previously rejected because it wasn't tracked, update its
// reason
merged_untracked_paths
.entry(path)
.and_modify(|other_reason| *other_reason = reason.clone())
.or_insert(reason);
}

print_untracked_files(ui, &merged_untracked_paths, path_converter)?;

let large_files = merged_untracked_paths
.iter()
.filter_map(|(path, reason)| match reason {
UntrackedReason::FileTooLarge { size, .. } => Some((path, size)),
UntrackedReason::FileNotAutoTracked => None,
})
.collect_vec();
if let Some(size) = large_files.iter().map(|(_path, size)| size).max() {
let large_files_list = large_files
.iter()
.map(|(path, _)| path_converter.format_file_path(path))
.join(" ");
writedoc!(
ui.hint_default(),
r"
This is to prevent large files from being added by accident. You can fix this by:
- Adding the file to `.gitignore`
- Run `jj config set --repo snapshot.max-new-file-size {size}`
This will increase the maximum file size allowed for new files, in this repository only.
- Run `jj --config snapshot.max-new-file-size={size} file track {large_files_list}`
This will increase the maximum file size allowed for new files, for this command only.
"
)?;
}
Ok(())
}
8 changes: 1 addition & 7 deletions cli/src/commands/file/untrack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use tracing::instrument;

use crate::cli_util::print_snapshot_stats;
use crate::cli_util::CommandHelper;
use crate::cli_util::SnapshotContext;
use crate::command_error::user_error_with_hint;
use crate::command_error::CommandError;
use crate::complete;
Expand Down Expand Up @@ -112,11 +111,6 @@ Make sure they're ignored, then try again.",
}
let repo = tx.commit("untrack paths")?;
locked_ws.finish(repo.op_id().clone())?;
print_snapshot_stats(
ui,
&stats,
workspace_command.env().path_converter(),
SnapshotContext::Untrack,
)?;
print_snapshot_stats(ui, &stats, workspace_command.env().path_converter())?;
Ok(())
}
2 changes: 0 additions & 2 deletions cli/src/commands/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use tracing::instrument;
use crate::cli_util::print_conflicted_paths;
use crate::cli_util::print_snapshot_stats;
use crate::cli_util::CommandHelper;
use crate::cli_util::SnapshotContext;
use crate::command_error::CommandError;
use crate::diff_util::get_copy_records;
use crate::diff_util::DiffFormat;
Expand Down Expand Up @@ -59,7 +58,6 @@ pub(crate) fn cmd_status(
ui,
&snapshot_stats,
workspace_command.env().path_converter(),
SnapshotContext::Status,
)?;
let repo = workspace_command.repo();
let maybe_wc_commit = workspace_command
Expand Down
21 changes: 11 additions & 10 deletions cli/tests/test_working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,24 +58,25 @@ fn test_snapshot_large_file() {
This will increase the maximum file size allowed for new files, for this command only.
"#);

// test with file track for hint formatting
// test with file track for hint formatting, both files should appear in
// warnings even though they were snapshotted separately
std::fs::write(repo_path.join("large2"), big_string).unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["file", "track", "large", "large2"]);
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&[
"file",
"--config=snapshot.auto-track='large'",
"track",
"large2",
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
Warning: Refused to snapshot some files:
large: 11.0KiB (11264 bytes); the maximum size allowed is 10.0KiB (10240 bytes)
large2: 11.0KiB (11264 bytes); the maximum size allowed is 10.0KiB (10240 bytes)
Hint: This is to prevent large files from being added by accident. You can fix this by:
- Adding the file to `.gitignore`
- Run `jj config set --repo snapshot.max-new-file-size 11264`
This will increase the maximum file size allowed for new files, in this repository only.
- Run `jj --config snapshot.max-new-file-size=11264 st`
This will increase the maximum file size allowed for new files, for this command only.
Warning: Refused to snapshot some files:
large: 11.0KiB (11264 bytes); the maximum size allowed is 10.0KiB (10240 bytes)
large2: 11.0KiB (11264 bytes); the maximum size allowed is 10.0KiB (10240 bytes)
Hint: This is to prevent large files from being added by accident. You can fix this by:
- Run `jj config set --repo snapshot.max-new-file-size 11264`
This will increase the maximum file size allowed for new files, in this repository only.
- Run `jj --config snapshot.max-new-file-size=11264 file track large large2`
Expand Down

0 comments on commit 4b6d4f6

Please sign in to comment.