From 0d44b02025cdba7419ee861b50cda003ce247100 Mon Sep 17 00:00:00 2001 From: Austin Seipp Date: Thu, 18 Jan 2024 18:29:56 -0600 Subject: [PATCH 1/4] cli: move tests from `test_checkout` -> `test_new_command` These didn't really need to use `jj checkout`, and it will be deprecated in a future change anyway. Move them out of there and into `new`. Ideally this would go into `test_conflicts.rs`, but that exists in `jj-lib`, so it doesn't have `TestEnvironment` available to it. Signed-off-by: Austin Seipp Change-Id: If0173b27ab4d1f6036a4ec632ec77b6824f310c3 --- cli/tests/test_checkout.rs | 57 --------------------------------- cli/tests/test_new_command.rs | 59 +++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 57 deletions(-) diff --git a/cli/tests/test_checkout.rs b/cli/tests/test_checkout.rs index c184bb6a4d..4f951e2844 100644 --- a/cli/tests/test_checkout.rs +++ b/cli/tests/test_checkout.rs @@ -101,63 +101,6 @@ fn test_checkout_not_single_rev() { "###); } -#[test] -fn test_checkout_conflicting_branches() { - let test_env = TestEnvironment::default(); - test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); - let repo_path = test_env.env_root().join("repo"); - - test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "one"]); - test_env.jj_cmd_ok(&repo_path, &["new", "-m", "two", "@-"]); - test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo"]); - test_env.jj_cmd_ok( - &repo_path, - &[ - "--at-op=@-", - "branch", - "create", - "foo", - "-r", - r#"description("one")"#, - ], - ); - - // Trigger resolution of concurrent operations - test_env.jj_cmd_ok(&repo_path, &["st"]); - - let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "foo"]); - insta::assert_snapshot!(stderr, @r###" - Error: Revset "foo" resolved to more than one revision - Hint: Branch foo resolved to multiple revisions because it's conflicted. - It resolved to these revisions: - kkmpptxz 66c6502d foo?? | (empty) two - qpvuntsm a9330854 foo?? | (empty) one - Set which revision the branch points to with `jj branch set foo -r `. - "###); -} - -#[test] -fn test_checkout_conflicting_change_ids() { - let test_env = TestEnvironment::default(); - test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); - let repo_path = test_env.env_root().join("repo"); - - test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "one"]); - test_env.jj_cmd_ok(&repo_path, &["--at-op=@-", "describe", "-m", "two"]); - - // Trigger resolution of concurrent operations - test_env.jj_cmd_ok(&repo_path, &["st"]); - - let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "qpvuntsm"]); - insta::assert_snapshot!(stderr, @r###" - Error: Revset "qpvuntsm" resolved to more than one revision - Hint: The revset "qpvuntsm" resolved to these revisions: - qpvuntsm?? d2ae6806 (empty) two - qpvuntsm?? a9330854 (empty) one - Some of these commits have the same change id. Abandon one of them with `jj abandon -r `. - "###); -} - fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String { let template = r#"commit_id ++ " " ++ description"#; test_env.jj_cmd_success(cwd, &["log", "-T", template]) diff --git a/cli/tests/test_new_command.rs b/cli/tests/test_new_command.rs index 5a8f6b3344..75f0d765c7 100644 --- a/cli/tests/test_new_command.rs +++ b/cli/tests/test_new_command.rs @@ -464,6 +464,65 @@ fn test_new_insert_before_root() { "###); } +#[test] +fn test_new_conflicting_branches() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "one"]); + test_env.jj_cmd_ok(&repo_path, &["new", "-m", "two", "@-"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo"]); + test_env.jj_cmd_ok( + &repo_path, + &[ + "--at-op=@-", + "branch", + "create", + "foo", + "-r", + r#"description("one")"#, + ], + ); + + // Trigger resolution of concurrent operations + test_env.jj_cmd_ok(&repo_path, &["st"]); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["new", "foo"]); + insta::assert_snapshot!(stderr, @r###" + Error: Revset "foo" resolved to more than one revision + Hint: Branch foo resolved to multiple revisions because it's conflicted. + It resolved to these revisions: + kkmpptxz 66c6502d foo?? | (empty) two + qpvuntsm a9330854 foo?? | (empty) one + Set which revision the branch points to with `jj branch set foo -r `. + Prefix the expression with 'all' to allow any number of revisions (i.e. 'all:foo'). + "###); +} + +#[test] +fn test_new_conflicting_change_ids() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "one"]); + test_env.jj_cmd_ok(&repo_path, &["--at-op=@-", "describe", "-m", "two"]); + + // Trigger resolution of concurrent operations + test_env.jj_cmd_ok(&repo_path, &["st"]); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["new", "qpvuntsm"]); + insta::assert_snapshot!(stderr, @r###" + Error: Revset "qpvuntsm" resolved to more than one revision + Hint: The revset "qpvuntsm" resolved to these revisions: + qpvuntsm?? d2ae6806 (empty) two + qpvuntsm?? a9330854 (empty) one + Some of these commits have the same change id. Abandon one of them with `jj abandon -r `. + Prefix the expression with 'all' to allow any number of revisions (i.e. 'all:qpvuntsm'). + "###); +} + fn setup_before_insertion(test_env: &TestEnvironment, repo_path: &Path) { test_env.jj_cmd_ok(repo_path, &["branch", "create", "A"]); test_env.jj_cmd_ok(repo_path, &["commit", "-m", "A"]); From fe82dff95e1ef622b6c0cb30ad5e2a12ba5f4099 Mon Sep 17 00:00:00 2001 From: Austin Seipp Date: Wed, 17 Jan 2024 16:41:43 -0600 Subject: [PATCH 2/4] cli: deprecate `jj checkout` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: As discussed in Discord, on GitHub, and elsewhere, this change deprecates the use of `jj checkout` and suggests users use `jj new` exclusively instead. The verb `checkout` is a relic of a bygone era the — days of RCS file locking, before 3-way merge — and is not a good, fitting name for the functionality it provides. To further drive the bit home, by default, `jj checkout` (and `jj co`) is now hidden. This will hopefully stop new users from running into it. Signed-off-by: Austin Seipp Change-Id: I7a1adfc9168fce1f25cf5d4c4c313304769e41a1 --- cli/src/commands/checkout.rs | 8 ++++++++ cli/src/commands/mod.rs | 1 + cli/tests/cli-reference@.md.snap | 24 ------------------------ cli/tests/test_checkout.rs | 10 ++++++++++ cli/tests/test_git_colocated.rs | 4 ++-- docs/git-comparison.md | 2 +- docs/tutorial.md | 12 +++++------- 7 files changed, 27 insertions(+), 34 deletions(-) diff --git a/cli/src/commands/checkout.rs b/cli/src/commands/checkout.rs index f1cb7b59a1..667c5d41ef 100644 --- a/cli/src/commands/checkout.rs +++ b/cli/src/commands/checkout.rs @@ -41,6 +41,14 @@ pub(crate) fn cmd_checkout( command: &CommandHelper, args: &CheckoutArgs, ) -> Result<(), CommandError> { + writeln!( + ui.warning(), + "warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent" + )?; + writeln!( + ui.warning(), + "warning: `jj checkout` will be removed in a future version, and this will be a hard error" + )?; let mut workspace_command = command.workspace_helper(ui)?; let target = workspace_command.resolve_single_rev(&args.revision, ui)?; let mut tx = workspace_command.start_transaction(); diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 96af7c1127..9fbdc661da 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -76,6 +76,7 @@ enum Command { Branch(branch::BranchCommand), #[command(alias = "print")] Cat(cat::CatArgs), + #[command(hide = true)] Checkout(checkout::CheckoutArgs), Chmod(chmod::ChmodArgs), Commit(commit::CommitArgs), diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 34e15a0fbc..ee8069ab0e 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -21,7 +21,6 @@ This document contains the help content for the `jj` command-line program. * [`jj branch track`↴](#jj-branch-track) * [`jj branch untrack`↴](#jj-branch-untrack) * [`jj cat`↴](#jj-cat) -* [`jj checkout`↴](#jj-checkout) * [`jj chmod`↴](#jj-chmod) * [`jj commit`↴](#jj-commit) * [`jj config`↴](#jj-config) @@ -105,7 +104,6 @@ To get started, see the tutorial at https://github.com/martinvonz/jj/blob/main/d * `backout` — Apply the reverse of a revision on top of another revision * `branch` — Manage branches * `cat` — Print contents of a file in a revision -* `checkout` — Create a new, empty change and edit it in the working copy * `chmod` — Sets or removes the executable bit for paths in the repo * `commit` — Update the description and create a new change on top * `config` — Manage config options @@ -387,28 +385,6 @@ Print contents of a file in a revision -## `jj checkout` - -Create a new, empty change and edit it in the working copy - -For more information, see https://github.com/martinvonz/jj/blob/main/docs/working-copy.md. - -**Usage:** `jj checkout [OPTIONS] ` - -###### **Arguments:** - -* `` — The revision to update to - -###### **Options:** - -* `-r` — Ignored (but lets you pass `-r` for consistency with other commands) - - Possible values: `true`, `false` - -* `-m`, `--message ` — The change description to use - - - ## `jj chmod` Sets or removes the executable bit for paths in the repo diff --git a/cli/tests/test_checkout.rs b/cli/tests/test_checkout.rs index 4f951e2844..1fa440cdd9 100644 --- a/cli/tests/test_checkout.rs +++ b/cli/tests/test_checkout.rs @@ -31,6 +31,8 @@ fn test_checkout() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["checkout", "@"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" + warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent + warning: `jj checkout` will be removed in a future version, and this will be a hard error Working copy now at: zsuskuln 05ce7118 (empty) (no description set) Parent commit : rlvkpnrz 5c52832c (empty) second "###); @@ -66,6 +68,8 @@ fn test_checkout_not_single_rev() { let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "root()..@"]); insta::assert_snapshot!(stderr, @r###" + warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent + warning: `jj checkout` will be removed in a future version, and this will be a hard error Error: Revset "root()..@" resolved to more than one revision Hint: The revset "root()..@" resolved to these revisions: royxmykx 2f859371 (empty) (no description set) @@ -78,6 +82,8 @@ fn test_checkout_not_single_rev() { let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "root()..@-"]); insta::assert_snapshot!(stderr, @r###" + warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent + warning: `jj checkout` will be removed in a future version, and this will be a hard error Error: Revset "root()..@-" resolved to more than one revision Hint: The revset "root()..@-" resolved to these revisions: mzvwutvl 5c1afd8b (empty) fifth @@ -89,6 +95,8 @@ fn test_checkout_not_single_rev() { let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "@-|@--"]); insta::assert_snapshot!(stderr, @r###" + warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent + warning: `jj checkout` will be removed in a future version, and this will be a hard error Error: Revset "@-|@--" resolved to more than one revision Hint: The revset "@-|@--" resolved to these revisions: mzvwutvl 5c1afd8b (empty) fifth @@ -97,6 +105,8 @@ fn test_checkout_not_single_rev() { let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "none()"]); insta::assert_snapshot!(stderr, @r###" + warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent + warning: `jj checkout` will be removed in a future version, and this will be a hard error Error: Revset "none()" didn't resolve to any revisions "###); } diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs index 6342156bd0..1a9867653c 100644 --- a/cli/tests/test_git_colocated.rs +++ b/cli/tests/test_git_colocated.rs @@ -127,7 +127,7 @@ fn test_git_colocated_unborn_branch() { // Stage some change, and check out root. This shouldn't clobber the HEAD. add_file_to_index("file0", ""); - let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["checkout", "root()"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["new", "root()"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Working copy now at: kkmpptxz fcdbbd73 (empty) (no description set) @@ -188,7 +188,7 @@ fn test_git_colocated_unborn_branch() { // Stage some change, and check out root again. This should unset the HEAD. // https://github.com/martinvonz/jj/issues/1495 add_file_to_index("file2", ""); - let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["checkout", "root()"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["new", "root()"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Working copy now at: znkkpsqq 10dd328b (empty) (no description set) diff --git a/docs/git-comparison.md b/docs/git-comparison.md index 4a4439dce6..d998196cac 100644 --- a/docs/git-comparison.md +++ b/docs/git-comparison.md @@ -234,7 +234,7 @@ parent. Start working on a new change based on the <main> branch - jj co main + jj new main git switch -c topic main or git checkout -b topic main (may need to stash or commit first) diff --git a/docs/tutorial.md b/docs/tutorial.md index d82d4e8d0a..87daec06e7 100644 --- a/docs/tutorial.md +++ b/docs/tutorial.md @@ -91,9 +91,7 @@ stayed the same. Each change to the working-copy commit amends the previous version. So how do we tell Jujutsu that we are done amending the current change and want to start working on a new one? That is what `jj new` is for. That will create a new commit on top of your current working-copy commit. The new commit -is for the working-copy changes. For familiarity for user coming from other -VCSs, there is also a `jj checkout/co` command, which is practically a synonym -for `jj new` (you can specify a destination for `jj new` as well). +is for the working-copy changes. So, let's say we're now done with this change, so we create a new change: @@ -116,10 +114,10 @@ very similar to `git commit --amend`, and `jj amend` is in fact an alias for Alternatively, we can use `jj edit ` to resume editing a commit in the working copy. Any further changes in the working copy will then amend the -commit. Whether you choose to checkout-and-squash or to edit typically depends -on how done you are with the change; if the change is almost done, it makes -sense to use `jj checkout` so you can easily review your adjustments with -`jj diff` before running `jj squash`. +commit. Whether you choose to create a new change and squash, or to edit, +typically depends on how done you are with the change; if the change is almost +done, it makes sense to use `jj new` so you can easily review your adjustments +with `jj diff` before running `jj squash`. ## The log command and "revsets" From 43d6f549939712c566719944f5da9e74048a262b Mon Sep 17 00:00:00 2001 From: Austin Seipp Date: Wed, 17 Jan 2024 17:35:34 -0600 Subject: [PATCH 3/4] cli: deprecate `jj merge` Summary: As discussed in Discord, on GitHub, and elsewhere, this change deprecates the use of `jj merge` and suggests users use `jj new` exclusively instead. `merge` isn't completely unfit as a name; but we think it obscures the generality of `new` and we want people to use it instead. To further drive the bit home, by default, `jj merge` is now hidden. This will hopefully stop new users from running into it. Signed-off-by: Austin Seipp Change-Id: I94938aca9d3e2aa12d1394a5fbc58acce3185b56 --- cli/src/commands/merge.rs | 8 ++++++ cli/src/commands/mod.rs | 1 + cli/tests/cli-reference@.md.snap | 48 -------------------------------- cli/tests/test_new_command.rs | 4 +++ 4 files changed, 13 insertions(+), 48 deletions(-) diff --git a/cli/src/commands/merge.rs b/cli/src/commands/merge.rs index fea077e2f8..f1e1b7c4cd 100644 --- a/cli/src/commands/merge.rs +++ b/cli/src/commands/merge.rs @@ -24,6 +24,14 @@ pub(crate) fn cmd_merge( command: &CommandHelper, args: &new::NewArgs, ) -> Result<(), CommandError> { + writeln!( + ui.warning(), + "warning: `jj merge` is deprecated; use `jj new` instead, which is equivalent" + )?; + writeln!( + ui.warning(), + "warning: `jj merge` will be removed in a future version, and this will be a hard error" + )?; if args.revisions.len() < 2 { return Err(CommandError::CliError(String::from( "Merge requires at least two revisions", diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 9fbdc661da..6acca1c5c8 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -104,6 +104,7 @@ enum Command { /// /// This is the same as `jj new`, except that it requires at least two /// arguments. + #[command(hide = true)] Merge(new::NewArgs), Move(r#move::MoveArgs), New(new::NewArgs), diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index ee8069ab0e..12065edbfa 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -49,7 +49,6 @@ This document contains the help content for the `jj` command-line program. * [`jj init`↴](#jj-init) * [`jj interdiff`↴](#jj-interdiff) * [`jj log`↴](#jj-log) -* [`jj merge`↴](#jj-merge) * [`jj move`↴](#jj-move) * [`jj new`↴](#jj-new) * [`jj next`↴](#jj-next) @@ -117,7 +116,6 @@ To get started, see the tutorial at https://github.com/martinvonz/jj/blob/main/d * `init` — Create a new repo in the given directory * `interdiff` — Compare the changes of two commits * `log` — Show commit history -* `merge` — Merge work from multiple branches * `move` — Move changes from one revision into another * `new` — Create a new, empty change and (by default) edit it in the working copy * `next` — Move the current working copy commit to the next child revision in the @@ -1012,52 +1010,6 @@ Show commit history -## `jj merge` - -Merge work from multiple branches - -Unlike most other VCSs, `jj merge` does not implicitly include the working copy revision's parent as one of the parents of the merge; you need to explicitly list all revisions that should become parents of the merge. - -This is the same as `jj new`, except that it requires at least two arguments. - -**Usage:** `jj merge [OPTIONS] [REVISIONS]...` - -###### **Arguments:** - -* `` — Parent(s) of the new change - - Default value: `@` - -###### **Options:** - -* `-r` — Ignored (but lets you pass `-r` for consistency with other commands) - - Possible values: `true`, `false` - -* `-m`, `--message ` — The change description to use -* `-L`, `--allow-large-revsets` — Deprecated. Please prefix the revset with `all:` instead - - Possible values: `true`, `false` - -* `--no-edit` — Do not edit the newly created change - - Possible values: `true`, `false` - -* `--edit` — No-op flag to pair with --no-edit - - Possible values: `true`, `false` - -* `-A`, `--insert-after` — Insert the new change between the target commit(s) and their children - - Possible values: `true`, `false` - -* `-B`, `--insert-before` — Insert the new change between the target commit(s) and their parents - - Possible values: `true`, `false` - - - - ## `jj move` Move changes from one revision into another diff --git a/cli/tests/test_new_command.rs b/cli/tests/test_new_command.rs index 75f0d765c7..e269cc2c41 100644 --- a/cli/tests/test_new_command.rs +++ b/cli/tests/test_new_command.rs @@ -123,10 +123,14 @@ fn test_new_merge() { // `jj merge` with less than two arguments is an error let stderr = test_env.jj_cmd_cli_error(&repo_path, &["merge"]); insta::assert_snapshot!(stderr, @r###" + warning: `jj merge` is deprecated; use `jj new` instead, which is equivalent + warning: `jj merge` will be removed in a future version, and this will be a hard error Error: Merge requires at least two revisions "###); let stderr = test_env.jj_cmd_cli_error(&repo_path, &["merge", "main"]); insta::assert_snapshot!(stderr, @r###" + warning: `jj merge` is deprecated; use `jj new` instead, which is equivalent + warning: `jj merge` will be removed in a future version, and this will be a hard error Error: Merge requires at least two revisions "###); From 7296354923ee717a3010c908ed9862adb7556fff Mon Sep 17 00:00:00 2001 From: Austin Seipp Date: Thu, 18 Jan 2024 18:01:11 -0600 Subject: [PATCH 4/4] docs: update changelog with `checkout`/`merge` deprecations Summary: Put both notices together at once, for ease of reading and understanding. Signed-off-by: Austin Seipp Change-Id: I2aedb42fdab346b21990a106433512d7ec119ad4 --- CHANGELOG.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e18ab7ad5c..1dd6048313 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Deprecations + +* `jj checkout` and `jj merge` are both deprecated; use `jj new` instead to + replace both of these commands in all instances. + + **Rationale**: `jj checkout` and `jj merge` both implement identical + functionality, which is a subset of `jj new`. `checkout` creates a new working + copy commit on top of a single specified revision, i.e. with one parent. + `merge` creates a new working copy commit on top of *at least* two specified + revisions, i.e. with two or more parents. + + The only difference between these commands and `jj new`, which *also* creates + a new working copy commit, is that `new` can create a working copy commit on + top of any arbitrary number of revisions, so it can handle both the previous + cases at once. The only actual difference between these three commands is the + command syntax and their name. These names were chosen to be familiar to users + of other version control systems, but we instead encourage all users to adopt + `jj new` instead; it is more general and easier to remember than both of + these. + + `jj checkout` and `jj merge` will no longer be shown as part of `jj help`, but + will still function for now, emitting a warning about their deprecation. + + **Deadline**: `jj checkout` and `jj merge` will be deleted and are expected + become a **hard error later in 2024**. + ### Breaking changes * (Minor) Diff summaries (e.g. `jj diff -s`) now use `D` for "Deleted" instead