Skip to content

Commit

Permalink
cli: reorder updating and reporting for consistency.
Browse files Browse the repository at this point in the history
* See #4239 for details.
* For now, update working copy before reporting repo changes, so that
  potential errors in reporting changes don't leave the repo in a stale
  state.

Fixes: #4239
  • Loading branch information
essiene committed Sep 22, 2024
1 parent 2e9049b commit 76f40e5
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 85 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

### Breaking changes

* Fixing #4239 means the ordering of some messages have changed.

* Invalid `ui.graph.style` configuration is now an error.

* The builtin template `branch_list` has been renamed to `bookmark_list` as part
Expand Down Expand Up @@ -69,6 +71,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

### Fixed bugs

* Update working copy before reporting changes. This prevents errors during reporting
from leaving the working copy in a stale state.

* `jj git fetch -b <remote-git-branch-name>` will now warn if the branch(es)
can not be found in any of the specified/configured remotes.

Expand Down
6 changes: 5 additions & 1 deletion cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1715,8 +1715,10 @@ See https://martinvonz.github.io/jj/latest/working-copy/#stale-working-copy \
}

self.user_repo = ReadonlyUserRepo::new(tx.commit(description));
self.report_repo_changes(ui, &old_repo)?;

// Update working copy before reporting repo changes, so that
// potential errors while reporting changes (broken pipe, etc)
// don't leave the working copy in a stale state.
if self.may_update_working_copy {
if let Some(new_commit) = &maybe_new_wc_commit {
self.update_working_copy(ui, maybe_old_wc_commit.as_ref(), new_commit)?;
Expand All @@ -1726,6 +1728,8 @@ See https://martinvonz.github.io/jj/latest/working-copy/#stale-working-copy \
}
}

self.report_repo_changes(ui, &old_repo)?;

let settings = self.settings();
if settings.user_name().is_empty() || settings.user_email().is_empty() {
writeln!(
Expand Down
16 changes: 8 additions & 8 deletions cli/tests/test_file_chmod_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,21 +215,21 @@ fn test_chmod_file_dir_deletion_conflicts() {
&["file", "chmod", "x", "file", "-r=file_deletion"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
insta::assert_snapshot!(stderr, @r###"
Working copy now at: kmkuslsw 1b2ef84c file_deletion | (conflict) file_deletion
Parent commit : zsuskuln c51c9c55 file | file
Parent commit : royxmykx 6b18b3c1 deletion | deletion
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion and an executable
New conflicts appeared in these commits:
kmkuslsw 1b2ef84c file_deletion | (conflict) file_deletion
To resolve the conflicts, start by updating to it:
jj new kmkuslsw
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Working copy now at: kmkuslsw 1b2ef84c file_deletion | (conflict) file_deletion
Parent commit : zsuskuln c51c9c55 file | file
Parent commit : royxmykx 6b18b3c1 deletion | deletion
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion and an executable
"#);
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "tree", "-r=file_deletion"]);
insta::assert_snapshot!(stdout,
@r###"
Expand Down
72 changes: 36 additions & 36 deletions cli/tests/test_repo_change_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,13 @@ fn test_report_conflicts() {
let (stdout, stderr) =
test_env.jj_cmd_ok(&repo_path, &["rebase", "-s=description(B)", "-d=root()"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
insta::assert_snapshot!(stderr, @r###"
Rebased 3 commits
Working copy now at: zsuskuln aa73e2ae (conflict) (empty) (no description set)
Parent commit : kkmpptxz 64bdec0c (conflict) C
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
New conflicts appeared in these commits:
kkmpptxz 64bdec0c (conflict) C
rlvkpnrz 10a5fd45 (conflict) B
Expand All @@ -40,23 +45,18 @@ fn test_report_conflicts() {
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Working copy now at: zsuskuln aa73e2ae (conflict) (empty) (no description set)
Parent commit : kkmpptxz 64bdec0c (conflict) C
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
"#);
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=description(A)"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 3 commits
Existing conflicts were resolved or abandoned from these commits:
kkmpptxz hidden 64bdec0c (conflict) C
rlvkpnrz hidden 10a5fd45 (conflict) B
Working copy now at: zsuskuln d70c003d (empty) (no description set)
Parent commit : kkmpptxz 43e94449 C
Added 0 files, modified 1 files, removed 0 files
Existing conflicts were resolved or abandoned from these commits:
kkmpptxz hidden 64bdec0c (conflict) C
rlvkpnrz hidden 10a5fd45 (conflict) B
"###);

// Can get hint about multiple root commits
Expand All @@ -66,6 +66,11 @@ fn test_report_conflicts() {
insta::assert_snapshot!(stderr, @r###"
Rebased 1 commits onto destination
Rebased 2 descendant commits
Working copy now at: zsuskuln 99fb9018 (conflict) (empty) (no description set)
Parent commit : kkmpptxz 17c72220 (conflict) C
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict
New conflicts appeared in these commits:
kkmpptxz 17c72220 (conflict) C
rlvkpnrz eb93a73d (conflict) B
Expand All @@ -75,11 +80,6 @@ fn test_report_conflicts() {
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Working copy now at: zsuskuln 99fb9018 (conflict) (empty) (no description set)
Parent commit : kkmpptxz 17c72220 (conflict) C
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict
"###);

// Resolve one of the conflicts by (mostly) following the instructions
Expand All @@ -96,10 +96,10 @@ fn test_report_conflicts() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Existing conflicts were resolved or abandoned from these commits:
rlvkpnrz hidden eb93a73d (conflict) B
Working copy now at: yostqsxw f5a0cf8c (empty) (no description set)
Parent commit : rlvkpnrz 87370844 B
Existing conflicts were resolved or abandoned from these commits:
rlvkpnrz hidden eb93a73d (conflict) B
"###);
}

Expand All @@ -121,9 +121,14 @@ fn test_report_conflicts_with_divergent_commits() {
let (stdout, stderr) =
test_env.jj_cmd_ok(&repo_path, &["rebase", "-s=description(B)", "-d=root()"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
insta::assert_snapshot!(stderr, @r###"
Concurrent modification detected, resolving automatically.
Rebased 3 commits
Working copy now at: zsuskuln?? 97ce1783 (conflict) C2
Parent commit : kkmpptxz eb93a73d (conflict) B
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
New conflicts appeared in these commits:
zsuskuln?? b535189c (conflict) C3
zsuskuln?? 97ce1783 (conflict) C2
Expand All @@ -133,45 +138,40 @@ fn test_report_conflicts_with_divergent_commits() {
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Working copy now at: zsuskuln?? 97ce1783 (conflict) C2
Parent commit : kkmpptxz eb93a73d (conflict) B
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
"#);
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=description(A)"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 3 commits
Working copy now at: zsuskuln?? f2d7a228 C2
Parent commit : kkmpptxz db069a22 B
Added 0 files, modified 1 files, removed 0 files
Existing conflicts were resolved or abandoned from these commits:
zsuskuln hidden b535189c (conflict) C3
zsuskuln hidden 97ce1783 (conflict) C2
kkmpptxz hidden eb93a73d (conflict) B
Working copy now at: zsuskuln?? f2d7a228 C2
Parent commit : kkmpptxz db069a22 B
Added 0 files, modified 1 files, removed 0 files
"###);

// Same thing when rebasing the divergent commits one at a time
let (stdout, stderr) =
test_env.jj_cmd_ok(&repo_path, &["rebase", "-s=description(C2)", "-d=root()"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
insta::assert_snapshot!(stderr, @r###"
Rebased 1 commits
Working copy now at: zsuskuln?? b15416ac (conflict) C2
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
New conflicts appeared in these commits:
zsuskuln?? b15416ac (conflict) C2
To resolve the conflicts, start by updating to it:
jj new zsuskuln
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Working copy now at: zsuskuln?? b15416ac (conflict) C2
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
"#);
"###);

let (stdout, stderr) =
test_env.jj_cmd_ok(&repo_path, &["rebase", "-s=description(C3)", "-d=root()"]);
Expand All @@ -194,11 +194,11 @@ fn test_report_conflicts_with_divergent_commits() {
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 1 commits
Existing conflicts were resolved or abandoned from these commits:
zsuskuln hidden b15416ac (conflict) C2
Working copy now at: zsuskuln?? 1f9680bd C2
Parent commit : kkmpptxz db069a22 B
Added 0 files, modified 1 files, removed 0 files
Existing conflicts were resolved or abandoned from these commits:
zsuskuln hidden b15416ac (conflict) C2
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(
Expand Down
50 changes: 25 additions & 25 deletions cli/tests/test_resolve_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,22 +235,22 @@ fn test_resolution() {
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
insta::assert_snapshot!(stderr, @r###"
Resolving conflicts in: file
Working copy now at: vruxwmqv 7699b9c3 conflict | (conflict) conflict
Parent commit : zsuskuln aa493daf a | a
Parent commit : royxmykx db6a4daf b | b
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict
New conflicts appeared in these commits:
vruxwmqv 7699b9c3 conflict | (conflict) conflict
To resolve the conflicts, start by updating to it:
jj new vruxwmqv
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Working copy now at: vruxwmqv 7699b9c3 conflict | (conflict) conflict
Parent commit : zsuskuln aa493daf a | a
Parent commit : royxmykx db6a4daf b | b
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict
"#);
"###);
insta::assert_snapshot!(
std::fs::read_to_string(test_env.env_root().join("editor2")).unwrap(), @r###"
<<<<<<< Conflict 1 of 1
Expand Down Expand Up @@ -590,23 +590,23 @@ fn test_simplify_conflict_sides() {
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
insta::assert_snapshot!(stderr, @r###"
Resolving conflicts in: fileB
New conflicts appeared in these commits:
nkmrtpmo 4b14662a conflict | (conflict) conflict
To resolve the conflicts, start by updating to it:
jj new nkmrtpmo
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Working copy now at: nkmrtpmo 4b14662a conflict | (conflict) conflict
Parent commit : kmkuslsw 18c1fb00 conflictA | (conflict) (empty) conflictA
Parent commit : lylxulpl d11c92eb conflictB | (conflict) (empty) conflictB
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
fileA 2-sided conflict
fileB 2-sided conflict
"#);
New conflicts appeared in these commits:
nkmrtpmo 4b14662a conflict | (conflict) conflict
To resolve the conflicts, start by updating to it:
jj new nkmrtpmo
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
"###);
insta::assert_snapshot!(std::fs::read_to_string(repo_path.join("fileB")).unwrap(), @r###"
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
Expand Down Expand Up @@ -860,22 +860,22 @@ fn test_multiple_conflicts() {
std::fs::write(&editor_script, "expect\n\0write\nresolution another_file\n").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["resolve", "another_file"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
insta::assert_snapshot!(stderr, @r###"
Resolving conflicts in: another_file
Working copy now at: vruxwmqv 6a90e546 conflict | (conflict) conflict
Parent commit : zsuskuln de7553ef a | a
Parent commit : royxmykx f68bc2f0 b | b
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
this_file_has_a_very_long_name_to_test_padding 2-sided conflict
New conflicts appeared in these commits:
vruxwmqv 6a90e546 conflict | (conflict) conflict
To resolve the conflicts, start by updating to it:
jj new vruxwmqv
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Working copy now at: vruxwmqv 6a90e546 conflict | (conflict) conflict
Parent commit : zsuskuln de7553ef a | a
Parent commit : royxmykx f68bc2f0 b | b
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
this_file_has_a_very_long_name_to_test_padding 2-sided conflict
"#);
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", "--git"]),
@r###"
diff --git a/another_file b/another_file
Expand Down
14 changes: 7 additions & 7 deletions cli/tests/test_restore_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,22 @@ fn test_restore() {
"###);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["restore", "-c=@-"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
insta::assert_snapshot!(stderr, @r###"
Created rlvkpnrz b9b6011e (empty) (no description set)
Rebased 1 descendant commits
Working copy now at: kkmpptxz d05c4d2a (conflict) (no description set)
Parent commit : rlvkpnrz b9b6011e (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file2 2-sided conflict including 1 deletion
New conflicts appeared in these commits:
kkmpptxz d05c4d2a (conflict) (no description set)
To resolve the conflicts, start by updating to it:
jj new kkmpptxz
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Working copy now at: kkmpptxz d05c4d2a (conflict) (no description set)
Parent commit : rlvkpnrz b9b6011e (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file2 2-sided conflict including 1 deletion
"#);
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s", "-r=@-"]);
insta::assert_snapshot!(stdout, @"");

Expand Down
Loading

0 comments on commit 76f40e5

Please sign in to comment.