Skip to content

Commit

Permalink
cli: print commits abandoned by git::import_refs() or fetch()
Browse files Browse the repository at this point in the history
This will probably help to understand why you've got conflicts after fetching.
Maybe we can also report changed local refs.

I think the stats should be redirected to stderr, but we have many other similar
messages printed to stdout. I'll probably fix them all at once later.
  • Loading branch information
yuja committed Oct 1, 2023
1 parent af81d2f commit b4529b7
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 25 deletions.
36 changes: 28 additions & 8 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use jj_lib::backend::{BackendError, ChangeId, CommitId, MergedTreeId, ObjectId};
use jj_lib::commit::Commit;
use jj_lib::git::{
FailedRefExport, FailedRefExportReason, GitConfigParseError, GitExportError, GitImportError,
GitRemoteManagementError,
GitImportStats, GitRemoteManagementError,
};
use jj_lib::git_backend::GitBackend;
use jj_lib::gitignore::GitIgnoreFile;
Expand Down Expand Up @@ -790,18 +790,18 @@ impl WorkspaceCommandHelper {
ui: &mut Ui,
git_repo: &Repository,
) -> Result<(), CommandError> {
let mut tx = self.start_transaction("import git refs").into_inner();
let git_settings = self.settings.git_settings();
let mut tx = self.start_transaction("import git refs");
// Automated import shouldn't fail because of reserved remote name.
git::import_some_refs(
tx.mut_repo(),
git_repo,
&self.settings.git_settings(),
|ref_name| !git::is_reserved_git_remote_ref(ref_name),
)?;
let stats = git::import_some_refs(tx.mut_repo(), git_repo, &git_settings, |ref_name| {
!git::is_reserved_git_remote_ref(ref_name)
})?;
if !tx.mut_repo().has_changes() {
return Ok(());
}

print_git_import_stats(ui, &tx, &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();
// If the Git HEAD has changed, abandon our old checkout and check out the new
Expand Down Expand Up @@ -1797,6 +1797,26 @@ 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> {
// 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")?;
}
}
Ok(())
}

pub fn print_failed_git_export(
ui: &Ui,
failed_branches: &[FailedRefExport],
Expand Down
16 changes: 10 additions & 6 deletions cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ use jj_lib::workspace::Workspace;
use maplit::hashset;

use crate::cli_util::{
print_failed_git_export, resolve_multiple_nonempty_revsets, short_change_hash,
short_commit_hash, user_error, user_error_with_hint, CommandError, CommandHelper, RevisionArg,
WorkspaceCommandHelper,
print_failed_git_export, print_git_import_stats, resolve_multiple_nonempty_revsets,
short_change_hash, short_commit_hash, user_error, user_error_with_hint, CommandError,
CommandHelper, RevisionArg, WorkspaceCommandHelper,
};
use crate::commands::make_branch_term;
use crate::progress::Progress;
Expand Down Expand Up @@ -332,7 +332,7 @@ fn cmd_git_fetch(
));
let branches = args.branch.iter().map(|b| b.as_str()).collect_vec();
for remote in remotes {
with_remote_callbacks(ui, |cb| {
let stats = with_remote_callbacks(ui, |cb| {
git::fetch(
tx.mut_repo(),
&git_repo,
Expand All @@ -347,6 +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)?;
}
tx.finish(ui)?;
Ok(())
Expand Down Expand Up @@ -547,6 +548,7 @@ fn do_git_clone(
unreachable!("we didn't provide any globs")
}
})?;
print_git_import_stats(ui, &fetch_tx, &stats.import_stats)?;
fetch_tx.finish(ui)?;
Ok((workspace_command, git_repo, stats))
}
Expand Down Expand Up @@ -977,7 +979,8 @@ fn cmd_git_push(
),
_ => user_error(err.to_string()),
})?;
git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?;
let stats = git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?;
print_git_import_stats(ui, &tx, &stats)?;
tx.finish(ui)?;
Ok(())
}
Expand Down Expand Up @@ -1025,7 +1028,8 @@ fn cmd_git_import(
let repo = workspace_command.repo();
let git_repo = get_git_repo(repo.store())?;
let mut tx = workspace_command.start_transaction("import git refs");
git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?;
let stats = git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?;
print_git_import_stats(ui, &tx, &stats)?;
tx.finish(ui)?;
Ok(())
}
Expand Down
11 changes: 6 additions & 5 deletions cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ use tracing::instrument;

use crate::cli_util::{
self, check_stale_working_copy, get_new_config_file_path, print_checkout_stats,
resolve_multiple_nonempty_revsets, resolve_multiple_nonempty_revsets_default_single,
run_ui_editor, serialize_config_value, short_commit_hash, user_error, user_error_with_hint,
write_config_value_to_file, Args, CommandError, CommandHelper, LogContentFormat, RevisionArg,
WorkspaceCommandHelper,
print_git_import_stats, resolve_multiple_nonempty_revsets,
resolve_multiple_nonempty_revsets_default_single, run_ui_editor, serialize_config_value,
short_commit_hash, user_error, user_error_with_hint, write_config_value_to_file, Args,
CommandError, CommandHelper, LogContentFormat, RevisionArg, WorkspaceCommandHelper,
};
use crate::config::{AnnotatedValue, ConfigSource};
use crate::diff_util::{self, DiffFormat, DiffFormatArgs};
Expand Down Expand Up @@ -1213,12 +1213,13 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(),
git::add_to_git_exclude(ui, &git_repo)?;
} else {
let mut tx = workspace_command.start_transaction("import git refs");
jj_lib::git::import_some_refs(
let stats = jj_lib::git::import_some_refs(
tx.mut_repo(),
&git_repo,
&command.settings().git_settings(),
|ref_name| !jj_lib::git::is_reserved_git_remote_ref(ref_name),
)?;
print_git_import_stats(ui, &tx, &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)?;
Expand Down
10 changes: 9 additions & 1 deletion cli/tests/test_git_colocated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ 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
@ 7f96185cfbe36341d0f9a86ebfaeab67a5922c7e
◉ 4bcbeaba9a4b309c5f45a8807fbf5499b9714315 master HEAD@git add a file
◉ 0000000000000000000000000000000000000000
Expand Down Expand Up @@ -206,6 +208,8 @@ 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
Working copy now at: yqosqzyt 096dc80d (empty) (no description set)
Parent commit : qpvuntsm 230dd059 (empty) (no description set)
@ 096dc80da67094fbaa6683e2a205dddffa31f9a8
Expand Down Expand Up @@ -294,7 +298,11 @@ fn test_git_colocated_fetch_deleted_or_moved_branch() {
// Move branch C sideways
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, @"");
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
"###);
// "original C" and "B_to_delete" are abandoned, as the corresponding branches
// were deleted or moved on the remote (#864)
insta::assert_snapshot!(get_log_output(&test_env, &clone_path), @r###"
Expand Down
26 changes: 21 additions & 5 deletions cli/tests/test_git_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,11 @@ fn test_git_fetch_all() {
master: zowqyktl ff36dc55 descr_for_trunk1
trunk1: zowqyktl ff36dc55 descr_for_trunk1
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&target_jj_repo_path, &["git", "fetch"]), @"");
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
"###);
insta::assert_snapshot!(get_branch_output(&test_env, &target_jj_repo_path), @r###"
a1: quxllqov 0424f6df descr_for_a1
a2: osusxwst 91e46b4b descr_for_a2
Expand Down Expand Up @@ -625,7 +629,10 @@ fn test_git_fetch_some_of_many_branches() {
&target_jj_repo_path,
&["git", "fetch", "--branch", "b", "--branch", "a1"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stdout, @r###"
Abandoned the following commits:
nknoxmzm 359a9a02 a1 | descr_for_a1
"###);
insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
◉ 13ac032802f1 descr_for_b b?? b@origin
│ ◉ 6f4e1c4dfe29 descr_for_a1 a1
Expand Down Expand Up @@ -657,7 +664,10 @@ fn test_git_fetch_some_of_many_branches() {
&target_jj_repo_path,
&["git", "fetch", "--branch", "b", "--branch", "a*"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stdout, @r###"
Abandoned the following commits:
qkvnknrk decaa396 a2 | descr_for_a2
"###);
insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
◉ 010977d69c5b descr_for_a2 a2
│ ◉ 13ac032802f1 descr_for_b b?? b@origin
Expand Down Expand Up @@ -979,7 +989,10 @@ 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, @"");
insta::assert_snapshot!(stdout, @r###"
Abandoned the following commits:
qkvnknrk decaa396 a2 | descr_for_a2
"###);
insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
◉ c7d4bdcbc215 descr_for_b b
│ ◉ 359a9a02457d descr_for_a1 a1
Expand Down Expand Up @@ -1046,7 +1059,10 @@ fn test_git_fetch_removed_parent_branch() {
"git", "fetch", "--branch", "master", "--branch", "trunk1", "--branch", "a1",
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stdout, @r###"
Abandoned the following commits:
nknoxmzm 359a9a02 a1 | descr_for_a1
"###);
insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
◉ c7d4bdcbc215 descr_for_b b
│ ◉ decaa3966c83 descr_for_a2 a2
Expand Down
5 changes: 5 additions & 0 deletions cli/tests/test_git_push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ 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
"###);
let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]);
insta::assert_snapshot!(stdout, @r###"
Expand Down Expand Up @@ -571,6 +574,8 @@ 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
"###);
let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push", "--deleted"]);
insta::assert_snapshot!(stdout, @r###"
Expand Down

0 comments on commit b4529b7

Please sign in to comment.