From 902a43a341badbf466bd88bd983385f56a2a3d62 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 3 Oct 2023 11:50:13 +0900 Subject: [PATCH] cli: don't list commits abandoned by import/fetch, just print the number Apparently, it gets too verbose if the remote history is actively rewritten. Let's summarize the output for now. The plan is to show the list of moved refs instead of the full list of abandoned commits. --- cli/src/cli_util.rs | 22 +++++++--------------- cli/src/commands/git.rs | 8 ++++---- cli/src/commands/mod.rs | 2 +- cli/tests/test_git_colocated.rs | 10 +++------- cli/tests/test_git_fetch.rs | 16 +++++----------- cli/tests/test_git_push.rs | 7 ++----- 6 files changed, 22 insertions(+), 43 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index a4bc331761..2ea37e9538 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -800,7 +800,7 @@ impl WorkspaceCommandHelper { return Ok(()); } - print_git_import_stats(ui, &tx, &stats)?; + print_git_import_stats(ui, &stats)?; let mut tx = tx.into_inner(); let old_git_head = self.repo().view().git_head().clone(); let new_git_head = tx.mut_repo().view().git_head().clone(); @@ -1791,22 +1791,14 @@ pub fn print_checkout_stats(ui: &mut Ui, stats: CheckoutStats) -> Result<(), std Ok(()) } -pub fn print_git_import_stats( - ui: &mut Ui, - tx: &WorkspaceCommandTransaction, - stats: &GitImportStats, -) -> Result<(), CommandError> { +pub fn print_git_import_stats(ui: &mut Ui, stats: &GitImportStats) -> Result<(), CommandError> { // TODO: maybe better to write status messages to stderr if !stats.abandoned_commits.is_empty() { - ui.write("Abandoned the following commits:\n")?; - let store = tx.base_repo().store(); - let helper = tx.base_workspace_helper(); - for id in &stats.abandoned_commits { - ui.write(" ")?; - let commit = store.get_commit(id)?; - helper.write_commit_summary(ui.stdout_formatter().as_mut(), &commit)?; - ui.write("\n")?; - } + writeln!( + ui, + "Abandoned {} commits that are no longer reachable.", + stats.abandoned_commits.len() + )?; } Ok(()) } diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index b73059c527..4f9b5228e8 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -347,7 +347,7 @@ fn cmd_git_fetch( GitFetchError::InternalGitError(err) => map_git_error(err), _ => user_error(err.to_string()), })?; - print_git_import_stats(ui, &tx, &stats.import_stats)?; + print_git_import_stats(ui, &stats.import_stats)?; } tx.finish(ui)?; Ok(()) @@ -548,7 +548,7 @@ fn do_git_clone( unreachable!("we didn't provide any globs") } })?; - print_git_import_stats(ui, &fetch_tx, &stats.import_stats)?; + print_git_import_stats(ui, &stats.import_stats)?; fetch_tx.finish(ui)?; Ok((workspace_command, git_repo, stats)) } @@ -980,7 +980,7 @@ fn cmd_git_push( _ => user_error(err.to_string()), })?; let stats = git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?; - print_git_import_stats(ui, &tx, &stats)?; + print_git_import_stats(ui, &stats)?; tx.finish(ui)?; Ok(()) } @@ -1029,7 +1029,7 @@ fn cmd_git_import( let git_repo = get_git_repo(repo.store())?; let mut tx = workspace_command.start_transaction("import git refs"); let stats = git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?; - print_git_import_stats(ui, &tx, &stats)?; + print_git_import_stats(ui, &stats)?; tx.finish(ui)?; Ok(()) } diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index aa1e7aa6ef..3836ddb038 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -1219,7 +1219,7 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(), &command.settings().git_settings(), |ref_name| !jj_lib::git::is_reserved_git_remote_ref(ref_name), )?; - print_git_import_stats(ui, &tx, &stats)?; + print_git_import_stats(ui, &stats)?; if let Some(git_head_id) = tx.mut_repo().view().git_head().as_normal().cloned() { let git_head_commit = tx.mut_repo().store().get_commit(&git_head_id)?; tx.check_out(&git_head_commit)?; diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs index a737e415ec..77d6d9a86b 100644 --- a/cli/tests/test_git_colocated.rs +++ b/cli/tests/test_git_colocated.rs @@ -158,8 +158,7 @@ fn test_git_colocated_rebase_on_import() { git_repo.branch("master", &commit1, true).unwrap(); git_repo.set_head("refs/heads/master").unwrap(); insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###" - Abandoned the following commits: - rlvkpnrz 45fbe1af master | modify a file + Abandoned 1 commits that are no longer reachable. Done importing changes from the underlying Git repo. @ 7f96185cfbe36341d0f9a86ebfaeab67a5922c7e ◉ 4bcbeaba9a4b309c5f45a8807fbf5499b9714315 master HEAD@git add a file @@ -209,8 +208,7 @@ fn test_git_colocated_branches() { ) .unwrap(); insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###" - Abandoned the following commits: - kkmpptxz 35605592 master | (empty) bar + Abandoned 1 commits that are no longer reachable. Working copy now at: yqosqzyt 096dc80d (empty) (no description set) Parent commit : qpvuntsm 230dd059 (empty) (no description set) Done importing changes from the underlying Git repo. @@ -298,9 +296,7 @@ fn test_git_colocated_fetch_deleted_or_moved_branch() { test_env.jj_cmd_success(&origin_path, &["describe", "C_to_move", "-m", "moved C"]); let stdout = test_env.jj_cmd_success(&clone_path, &["git", "fetch"]); insta::assert_snapshot!(stdout, @r###" - Abandoned the following commits: - vnotlzrn 8d4e006f C_to_move | (empty) original C - wvpolxwl 929e298a B_to_delete | (empty) B_to_delete + Abandoned 2 commits that are no longer reachable. "###); // "original C" and "B_to_delete" are abandoned, as the corresponding branches // were deleted or moved on the remote (#864) diff --git a/cli/tests/test_git_fetch.rs b/cli/tests/test_git_fetch.rs index c21e40df24..5463b0ced6 100644 --- a/cli/tests/test_git_fetch.rs +++ b/cli/tests/test_git_fetch.rs @@ -474,9 +474,7 @@ fn test_git_fetch_all() { trunk1: zowqyktl ff36dc55 descr_for_trunk1 "###); insta::assert_snapshot!(test_env.jj_cmd_success(&target_jj_repo_path, &["git", "fetch"]), @r###" - Abandoned the following commits: - qkvnknrk decaa396 a2 | descr_for_a2 - nknoxmzm 359a9a02 a1 | descr_for_a1 + Abandoned 2 commits that are no longer reachable. "###); insta::assert_snapshot!(get_branch_output(&test_env, &target_jj_repo_path), @r###" a1: quxllqov 0424f6df descr_for_a1 @@ -628,8 +626,7 @@ fn test_git_fetch_some_of_many_branches() { &["git", "fetch", "--branch", "b", "--branch", "a1"], ); insta::assert_snapshot!(stdout, @r###" - Abandoned the following commits: - nknoxmzm 359a9a02 a1 | descr_for_a1 + Abandoned 1 commits that are no longer reachable. "###); insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###" ◉ 13ac032802f1 descr_for_b b?? b@origin @@ -663,8 +660,7 @@ fn test_git_fetch_some_of_many_branches() { &["git", "fetch", "--branch", "b", "--branch", "a*"], ); insta::assert_snapshot!(stdout, @r###" - Abandoned the following commits: - qkvnknrk decaa396 a2 | descr_for_a2 + Abandoned 1 commits that are no longer reachable. "###); insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###" ◉ 010977d69c5b descr_for_a2 a2 @@ -988,8 +984,7 @@ fn test_git_fetch_removed_branch() { // Fetch branches a2 from origin, and check that it has been removed locally let stdout = test_env.jj_cmd_success(&target_jj_repo_path, &["git", "fetch", "--branch", "a2"]); insta::assert_snapshot!(stdout, @r###" - Abandoned the following commits: - qkvnknrk decaa396 a2 | descr_for_a2 + Abandoned 1 commits that are no longer reachable. "###); insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###" ◉ c7d4bdcbc215 descr_for_b b @@ -1058,8 +1053,7 @@ fn test_git_fetch_removed_parent_branch() { ], ); insta::assert_snapshot!(stdout, @r###" - Abandoned the following commits: - nknoxmzm 359a9a02 a1 | descr_for_a1 + Abandoned 1 commits that are no longer reachable. "###); insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###" ◉ c7d4bdcbc215 descr_for_b b diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index cdf44be648..ae48acb63f 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -295,9 +295,7 @@ fn test_git_push_multiple() { Delete branch branch1 from 45a3aa29e907 Force branch branch2 from 8476341eb395 to 15dcdaa4f12f Add branch my-branch to 15dcdaa4f12f - Abandoned the following commits: - rlzusymt 8476341e branch2@origin | (empty) description 2 - lzmmnrxq 45a3aa29 branch1@origin | (empty) description 1 + Abandoned 2 commits that are no longer reachable. "###); let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]); insta::assert_snapshot!(stdout, @r###" @@ -574,8 +572,7 @@ fn test_git_push_deleted() { insta::assert_snapshot!(stdout, @r###" Branch changes to push to origin: Delete branch branch1 from 45a3aa29e907 - Abandoned the following commits: - lzmmnrxq 45a3aa29 branch1@origin | (empty) description 1 + Abandoned 1 commits that are no longer reachable. "###); let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push", "--deleted"]); insta::assert_snapshot!(stdout, @r###"