From e5ab0d0f8117d837b6d0fc0442ff4c3290cbb82d Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 28 Oct 2024 17:43:16 +0900 Subject: [PATCH] cli: push new non-tracking bookmarks by default, rename --allow-new flag This partially reverts 296961cb1942 "cli: git push: do not push new bookmarks by default." Apparently, the restriction introduced by that patch was too strict for some use cases. The new rule is that any non-tracking (or local-only) bookmarks will be pushed to new remote by default, but --allow-new-tracking is required if a bookmark is already tracking other (real) remotes. Closes #5094 --- CHANGELOG.md | 5 + cli/src/commands/git/push.rs | 40 ++-- cli/tests/cli-reference@.md.snap | 4 +- cli/tests/test_bookmark_command.rs | 14 +- cli/tests/test_completion.rs | 5 +- cli/tests/test_git_private_commits.rs | 18 +- cli/tests/test_git_push.rs | 278 +++++++++++++++----------- cli/tests/test_undo.rs | 10 +- docs/bookmarks.md | 4 +- docs/github.md | 4 +- 10 files changed, 213 insertions(+), 169 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f167ad4958..e4e582e1d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * `jj absorb` now abandons the source commit if it becomes empty and has no description. +* `jj git push` now pushes new non-tracking bookmarks by default. The + `--allow-new` flag (introduced by 0.24.0) and relates changes are reverted. + New `--allow-new-tracking` flag is required in order to push tracking + bookmarks to other remotes. [#5094](https://github.com/jj-vcs/jj/issues/5094) + ### Deprecations * `--config-toml=TOML` is deprecated in favor of `--config=NAME=VALUE` and diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index 92e014f302..094b06692c 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -57,8 +57,8 @@ use crate::ui::Ui; /// Push to a Git remote /// -/// By default, pushes tracking bookmarks pointing to -/// `remote_bookmarks(remote=)..@`. Use `--bookmark` to push specific +/// By default, pushes new non-tracking and existing tracking bookmarks pointing +/// to `remote_bookmarks(remote=)..@`. Use `--bookmark` to push specific /// bookmarks. Use `--all` to push all bookmarks. Use `--change` to generate /// bookmark names based on the change IDs of specific commits. /// @@ -117,11 +117,11 @@ pub struct GitPushArgs { /// correspond to missing local bookmarks. #[arg(long)] deleted: bool, - /// Allow pushing new bookmarks + /// Allow pushing tracking bookmarks to new remote /// /// Newly-created remote bookmarks will be tracked automatically. #[arg(long, short = 'N', conflicts_with = "what")] - allow_new: bool, + allow_new_tracking: bool, /// Allow pushing commits with empty descriptions #[arg(long)] allow_empty_description: bool, @@ -181,7 +181,7 @@ pub fn cmd_git_push( if args.all { for (bookmark_name, targets) in view.local_remote_bookmarks(&remote) { let allow_new = true; // implied by --all - match classify_bookmark_update(bookmark_name, &remote, targets, allow_new) { + match classify_bookmark_update(view, bookmark_name, &remote, targets, allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -194,7 +194,7 @@ pub fn cmd_git_push( continue; } let allow_new = false; // doesn't matter - match classify_bookmark_update(bookmark_name, &remote, targets, allow_new) { + match classify_bookmark_update(view, bookmark_name, &remote, targets, allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -207,7 +207,7 @@ pub fn cmd_git_push( continue; } let allow_new = false; // doesn't matter - match classify_bookmark_update(bookmark_name, &remote, targets, allow_new) { + match classify_bookmark_update(view, bookmark_name, &remote, targets, allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -234,7 +234,7 @@ pub fn cmd_git_push( continue; } let allow_new = true; // --change implies creation of remote bookmark - match classify_bookmark_update(bookmark_name, &remote, targets, allow_new) { + match classify_bookmark_update(view, bookmark_name, &remote, targets, allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => writeln!( ui.status(), @@ -244,12 +244,13 @@ pub fn cmd_git_push( } } + let allow_new = args.allow_new_tracking; let bookmarks_by_name = find_bookmarks_to_push(view, &args.bookmark, &remote)?; for &(bookmark_name, targets) in &bookmarks_by_name { if !seen_bookmarks.insert(bookmark_name) { continue; } - match classify_bookmark_update(bookmark_name, &remote, targets, args.allow_new) { + match classify_bookmark_update(view, bookmark_name, &remote, targets, allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => writeln!( ui.status(), @@ -272,7 +273,7 @@ pub fn cmd_git_push( if !seen_bookmarks.insert(bookmark_name) { continue; } - match classify_bookmark_update(bookmark_name, &remote, targets, args.allow_new) { + match classify_bookmark_update(view, bookmark_name, &remote, targets, allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -549,11 +550,20 @@ impl From for CommandError { } fn classify_bookmark_update( + view: &View, bookmark_name: &str, remote_name: &str, targets: LocalAndRemoteRef, allow_new: bool, ) -> Result, RejectedBookmarkUpdateReason> { + let any_tracking = || { + view.remote_bookmarks_matching( + &StringPattern::exact(bookmark_name), + &StringPattern::everything(), + ) + .filter(|&((_, remote), _)| remote != git::REMOTE_NAME_FOR_LOCAL_GIT_REPO) + .any(|(_, remote_ref)| remote_ref.is_tracking()) + }; let push_action = classify_bookmark_push_action(targets); match push_action { BookmarkPushAction::AlreadyMatches => Ok(None), @@ -575,14 +585,16 @@ fn classify_bookmark_update( bookmark." )), }), - BookmarkPushAction::Update(update) if update.old_target.is_none() && !allow_new => { + BookmarkPushAction::Update(update) + if update.old_target.is_none() && !allow_new && any_tracking() => + { Err(RejectedBookmarkUpdateReason { message: format!( - "Refusing to create new remote bookmark {bookmark_name}@{remote_name}" + "Refusing to track new remote {remote_name} by bookmark {bookmark_name}" ), hint: Some( - "Use --allow-new to push new bookmark. Use --remote to specify the remote to \ - push to." + "Use --allow-new-tracking to push bookmark to new remote. Use --remote to \ + specify the remote to push to." .to_owned(), ), }) diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index d9fc70a9cc..d7e997e4f4 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1154,7 +1154,7 @@ Create a new Git backed repo Push to a Git remote -By default, pushes tracking bookmarks pointing to `remote_bookmarks(remote=)..@`. Use `--bookmark` to push specific bookmarks. Use `--all` to push all bookmarks. Use `--change` to generate bookmark names based on the change IDs of specific commits. +By default, pushes new non-tracking and existing tracking bookmarks pointing to `remote_bookmarks(remote=)..@`. Use `--bookmark` to push specific bookmarks. Use `--all` to push all bookmarks. Use `--change` to generate bookmark names based on the change IDs of specific commits. Unlike in Git, the remote to push to is not derived from the tracked remote bookmarks. Use `--remote` to select the remote Git repository by name. There is no option to push to multiple remotes. @@ -1181,7 +1181,7 @@ Before the command actually moves, creates, or deletes a remote bookmark, it mak * `--deleted` — Push all deleted bookmarks Only tracked bookmarks can be successfully deleted on the remote. A warning will be printed if any untracked bookmarks on the remote correspond to missing local bookmarks. -* `-N`, `--allow-new` — Allow pushing new bookmarks +* `-N`, `--allow-new-tracking` — Allow pushing tracking bookmarks to new remote Newly-created remote bookmarks will be tracked automatically. * `--allow-empty-description` — Allow pushing commits with empty descriptions diff --git a/cli/tests/test_bookmark_command.rs b/cli/tests/test_bookmark_command.rs index 6d89338258..0f626a3455 100644 --- a/cli/tests/test_bookmark_command.rs +++ b/cli/tests/test_bookmark_command.rs @@ -196,7 +196,7 @@ fn test_bookmark_move() { // Delete bookmark locally, but is still tracking remote test_env.jj_cmd_ok(&repo_path, &["describe", "@-", "-mcommit"]); - test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new", "-r@-"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "-r@-"]); test_env.jj_cmd_ok(&repo_path, &["bookmark", "delete", "foo"]); insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" foo (deleted) @@ -441,7 +441,7 @@ fn test_bookmark_rename() { test_env.jj_cmd_ok(&repo_path, &["new"]); test_env.jj_cmd_ok(&repo_path, &["describe", "-m=commit-2"]); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "bremote"]); - test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new", "-b=bremote"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "-b=bremote"]); let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["bookmark", "rename", "bremote", "bremote2"]); insta::assert_snapshot!(stderr, @r###" @@ -1081,7 +1081,7 @@ fn test_bookmark_track_conflict() { ); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "main"]); test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "a"]); - test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new", "-b", "main"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "-b", "main"]); test_env.jj_cmd_ok(&repo_path, &["bookmark", "untrack", "main@origin"]); test_env.jj_cmd_ok( &repo_path, @@ -1757,11 +1757,9 @@ fn test_bookmark_list_tracked() { &[ "git", "push", - "--allow-new", - "--remote", - "upstream", - "--bookmark", - "remote-unsync", + "--allow-new-tracking", + "--remote=upstream", + "--bookmark=remote-unsync", ], ); test_env.jj_cmd_ok( diff --git a/cli/tests/test_completion.rs b/cli/tests/test_completion.rs index 66c1015c71..ef4cc2bcd6 100644 --- a/cli/tests/test_completion.rs +++ b/cli/tests/test_completion.rs @@ -49,10 +49,7 @@ fn test_bookmark_names() { test_env.jj_cmd_ok(&repo_path, &["desc", "-r", "aaa-tracked", "-m", "x"]); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "bbb-tracked"]); test_env.jj_cmd_ok(&repo_path, &["desc", "-r", "bbb-tracked", "-m", "x"]); - test_env.jj_cmd_ok( - &repo_path, - &["git", "push", "--allow-new", "--bookmark", "glob:*-tracked"], - ); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "--bookmark", "glob:*-tracked"]); test_env.jj_cmd_ok(&origin_path, &["bookmark", "create", "aaa-untracked"]); test_env.jj_cmd_ok(&origin_path, &["desc", "-r", "aaa-untracked", "-m", "x"]); diff --git a/cli/tests/test_git_private_commits.rs b/cli/tests/test_git_private_commits.rs index 5d68e54045..8ec288d7d1 100644 --- a/cli/tests/test_git_private_commits.rs +++ b/cli/tests/test_git_private_commits.rs @@ -70,7 +70,7 @@ fn set_up_remote_at_main(test_env: &TestEnvironment, workspace_root: &Path, remo &[ "git", "push", - "--allow-new", + "--allow-new-tracking", "--remote", remote_name, "-b=main", @@ -166,10 +166,7 @@ fn test_git_private_commits_not_directly_in_line_block_pushing() { test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "bookmark1"]); test_env.add_config(r#"git.private-commits = "description(glob:'private*')""#); - let stderr = test_env.jj_cmd_failure( - &workspace_root, - &["git", "push", "--allow-new", "-b=bookmark1"], - ); + let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "-b=bookmark1"]); insta::assert_snapshot!(stderr, @r" Error: Won't push commit f1253a9b1ea9 since it is private Hint: Rejected commit: yqosqzyt f1253a9b (empty) private 1 @@ -208,10 +205,8 @@ fn test_git_private_commits_already_on_the_remote_do_not_block_push() { test_env.jj_cmd_ok(&workspace_root, &["new", "main", "-m=private 1"]); test_env.jj_cmd_ok(&workspace_root, &["new", "-m=public 3"]); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "set", "main"]); - let (_, stderr) = test_env.jj_cmd_ok( - &workspace_root, - &["git", "push", "--allow-new", "-b=main", "-b=bookmark1"], - ); + let (_, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-b=main", "-b=bookmark1"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move forward bookmark main from 7eb97bf230ad to fbb352762352 @@ -241,10 +236,7 @@ fn test_git_private_commits_already_on_the_remote_do_not_block_push() { &["new", "description('private 1')", "-m=public 4"], ); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "bookmark2"]); - let (_, stderr) = test_env.jj_cmd_ok( - &workspace_root, - &["git", "push", "--allow-new", "-b=bookmark2"], - ); + let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-b=bookmark2"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark bookmark2 to ee5b808b0b95 diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index eec18a1bef..cbcc4b4359 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -88,10 +88,7 @@ fn test_git_push_current_bookmark() { my-bookmark: yostqsxw bc7610b6 (empty) foo "###); // First dry-run. `bookmark1` should not get pushed. - let (stdout, stderr) = test_env.jj_cmd_ok( - &workspace_root, - &["git", "push", "--allow-new", "--dry-run"], - ); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--dry-run"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -99,7 +96,7 @@ fn test_git_push_current_bookmark() { Add bookmark my-bookmark to bc7610b65a91 Dry-run requested, not pushing. "#); - let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -237,7 +234,7 @@ fn test_git_push_other_remote_has_bookmark() { // TODO: Saner test? let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, - &["git", "push", "--allow-new", "--remote=other"], + &["git", "push", "--allow-new-tracking", "--remote=other"], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -422,7 +419,7 @@ fn test_git_push_creation_unexpectedly_already_exists() { @origin: rlzusymt 8476341e (empty) description 2 "###); - let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--allow-new"]); + let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark bookmark1 to cb17dcdc74d5 @@ -441,12 +438,6 @@ fn test_git_push_locally_created_and_rewritten() { test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-mlocal 1"]); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "my"]); let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); - insta::assert_snapshot!(stderr, @r" - Warning: Refusing to create new remote bookmark my@origin - Hint: Use --allow-new to push new bookmark. Use --remote to specify the remote to push to. - Nothing changed. - "); - let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark my to fcc999921ce9 @@ -460,13 +451,13 @@ fn test_git_push_locally_created_and_rewritten() { @origin: xtvrqkyv d13ecdbd (empty) description 1 bookmark2: rlzusymt 8476341e (empty) description 2 @origin: rlzusymt 8476341e (empty) description 2 - my: vruxwmqv eaf7a52c (empty) local 2 + my: vruxwmqv bde1d2e4 (empty) local 2 @origin (ahead by 1 commits, behind by 1 commits): vruxwmqv hidden fcc99992 (empty) local 1 "); let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stderr, @r" Changes to push to origin: - Move sideways bookmark my from fcc999921ce9 to eaf7a52c8906 + Move sideways bookmark my from fcc999921ce9 to bde1d2e44b2a "); } @@ -502,14 +493,7 @@ fn test_git_push_multiple() { // Dry run requesting two specific bookmarks let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, - &[ - "git", - "push", - "--allow-new", - "-b=bookmark1", - "-b=my-bookmark", - "--dry-run", - ], + &["git", "push", "-b=bookmark1", "-b=my-bookmark", "--dry-run"], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -524,7 +508,6 @@ fn test_git_push_multiple() { &[ "git", "push", - "--allow-new", "-b=bookmark1", "-b=my-bookmark", "-b=bookmark1", @@ -732,27 +715,21 @@ fn test_git_push_revisions() { std::fs::write(workspace_root.join("file"), "modified again").unwrap(); // Push an empty set - let (_stdout, stderr) = test_env.jj_cmd_ok( - &workspace_root, - &["git", "push", "--allow-new", "-r=none()"], - ); + let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-r=none()"]); insta::assert_snapshot!(stderr, @r###" Warning: No bookmarks point to the specified revisions: none() Nothing changed. "###); // Push a revision with no bookmarks - let (stdout, stderr) = - test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new", "-r=@--"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-r=@--"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Warning: No bookmarks point to the specified revisions: @-- Nothing changed. "###); // Push a revision with a single bookmark - let (stdout, stderr) = test_env.jj_cmd_ok( - &workspace_root, - &["git", "push", "--allow-new", "-r=@-", "--dry-run"], - ); + let (stdout, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-r=@-", "--dry-run"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -762,7 +739,7 @@ fn test_git_push_revisions() { // Push multiple revisions of which some have bookmarks let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, - &["git", "push", "--allow-new", "-r=@--", "-r=@-", "--dry-run"], + &["git", "push", "-r=@--", "-r=@-", "--dry-run"], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -772,10 +749,8 @@ fn test_git_push_revisions() { Dry-run requested, not pushing. "#); // Push a revision with a multiple bookmarks - let (stdout, stderr) = test_env.jj_cmd_ok( - &workspace_root, - &["git", "push", "--allow-new", "-r=@", "--dry-run"], - ); + let (stdout, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-r=@", "--dry-run"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -786,7 +761,7 @@ fn test_git_push_revisions() { // Repeating a commit doesn't result in repeated messages about the bookmark let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, - &["git", "push", "--allow-new", "-r=@-", "-r=@-", "--dry-run"], + &["git", "push", "-r=@-", "-r=@-", "--dry-run"], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -809,29 +784,11 @@ fn test_git_push_mixed() { test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "bookmark-2b"]); std::fs::write(workspace_root.join("file"), "modified again").unwrap(); - // --allow-new is not implied for --bookmark=.. and -r=.. - let stderr = test_env.jj_cmd_failure( - &workspace_root, - &[ - "git", - "push", - "--change=@--", - "--bookmark=bookmark-1", - "-r=@", - ], - ); - insta::assert_snapshot!(stderr, @r" - Creating bookmark push-yqosqzytrlsw for revision yqosqzytrlsw - Error: Refusing to create new remote bookmark bookmark-1@origin - Hint: Use --allow-new to push new bookmark. Use --remote to specify the remote to push to. - "); - let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &[ "git", "push", - "--allow-new", "--change=@--", "--bookmark=bookmark-1", "-r=@", @@ -905,7 +862,7 @@ fn test_git_push_no_description() { test_env.jj_cmd_ok(&workspace_root, &["describe", "-m="]); let stderr = test_env.jj_cmd_failure( &workspace_root, - &["git", "push", "--allow-new", "--bookmark", "my-bookmark"], + &["git", "push", "--bookmark", "my-bookmark"], ); insta::assert_snapshot!(stderr, @r" Error: Won't push commit 5b36783cd11c since it has no description @@ -916,7 +873,6 @@ fn test_git_push_no_description() { &[ "git", "push", - "--allow-new", "--bookmark", "my-bookmark", "--allow-empty-description", @@ -935,13 +891,7 @@ fn test_git_push_no_description_in_immutable() { let stderr = test_env.jj_cmd_failure( &workspace_root, - &[ - "git", - "push", - "--allow-new", - "--bookmark=my-bookmark", - "--dry-run", - ], + &["git", "push", "--bookmark=my-bookmark", "--dry-run"], ); insta::assert_snapshot!(stderr, @r" Error: Won't push commit 5b36783cd11c since it has no description @@ -951,13 +901,7 @@ fn test_git_push_no_description_in_immutable() { test_env.add_config(r#"revset-aliases."immutable_heads()" = "imm""#); let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, - &[ - "git", - "push", - "--allow-new", - "--bookmark=my-bookmark", - "--dry-run", - ], + &["git", "push", "--bookmark=my-bookmark", "--dry-run"], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -981,7 +925,7 @@ fn test_git_push_missing_author() { run_without_var("JJ_USER", &["bookmark", "create", "missing-name"]); let stderr = test_env.jj_cmd_failure( &workspace_root, - &["git", "push", "--allow-new", "--bookmark", "missing-name"], + &["git", "push", "--bookmark", "missing-name"], ); insta::assert_snapshot!(stderr, @r" Error: Won't push commit 944313939bbd since it has no author and/or committer set @@ -991,7 +935,7 @@ fn test_git_push_missing_author() { run_without_var("JJ_EMAIL", &["bookmark", "create", "missing-email"]); let stderr = test_env.jj_cmd_failure( &workspace_root, - &["git", "push", "--allow-new", "--bookmark=missing-email"], + &["git", "push", "--bookmark=missing-email"], ); insta::assert_snapshot!(stderr, @r" Error: Won't push commit 59354714f789 since it has no author and/or committer set @@ -1018,13 +962,7 @@ fn test_git_push_missing_author_in_immutable() { let stderr = test_env.jj_cmd_failure( &workspace_root, - &[ - "git", - "push", - "--allow-new", - "--bookmark=my-bookmark", - "--dry-run", - ], + &["git", "push", "--bookmark=my-bookmark", "--dry-run"], ); insta::assert_snapshot!(stderr, @r" Error: Won't push commit 011f740bf8b5 since it has no author and/or committer set @@ -1034,13 +972,7 @@ fn test_git_push_missing_author_in_immutable() { test_env.add_config(r#"revset-aliases."immutable_heads()" = "imm""#); let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, - &[ - "git", - "push", - "--allow-new", - "--bookmark=my-bookmark", - "--dry-run", - ], + &["git", "push", "--bookmark=my-bookmark", "--dry-run"], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -1062,10 +994,8 @@ fn test_git_push_missing_committer() { }; test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "missing-name"]); run_without_var("JJ_USER", &["describe", "-m=no committer name"]); - let stderr = test_env.jj_cmd_failure( - &workspace_root, - &["git", "push", "--allow-new", "--bookmark=missing-name"], - ); + let stderr = + test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--bookmark=missing-name"]); insta::assert_snapshot!(stderr, @r" Error: Won't push commit 4fd190283d1a since it has no author and/or committer set Hint: Rejected commit: yqosqzyt 4fd19028 missing-name | (empty) no committer name @@ -1075,7 +1005,7 @@ fn test_git_push_missing_committer() { run_without_var("JJ_EMAIL", &["describe", "-m=no committer email"]); let stderr = test_env.jj_cmd_failure( &workspace_root, - &["git", "push", "--allow-new", "--bookmark=missing-email"], + &["git", "push", "--bookmark=missing-email"], ); insta::assert_snapshot!(stderr, @r" Error: Won't push commit eab97428a6ec since it has no author and/or committer set @@ -1087,7 +1017,7 @@ fn test_git_push_missing_committer() { run_without_var("JJ_EMAIL", &["describe", "-m=", "missing-email"]); let stderr = test_env.jj_cmd_failure( &workspace_root, - &["git", "push", "--allow-new", "--bookmark=missing-email"], + &["git", "push", "--bookmark=missing-email"], ); insta::assert_snapshot!(stderr, @r" Error: Won't push commit 1143ed607f54 since it has no description and it has no author and/or committer set @@ -1115,13 +1045,7 @@ fn test_git_push_missing_committer_in_immutable() { let stderr = test_env.jj_cmd_failure( &workspace_root, - &[ - "git", - "push", - "--allow-new", - "--bookmark=my-bookmark", - "--dry-run", - ], + &["git", "push", "--bookmark=my-bookmark", "--dry-run"], ); insta::assert_snapshot!(stderr, @r" Error: Won't push commit 7e61dc727a8f since it has no author and/or committer set @@ -1131,13 +1055,7 @@ fn test_git_push_missing_committer_in_immutable() { test_env.add_config(r#"revset-aliases."immutable_heads()" = "imm""#); let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, - &[ - "git", - "push", - "--allow-new", - "--bookmark=my-bookmark", - "--dry-run", - ], + &["git", "push", "--bookmark=my-bookmark", "--dry-run"], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -1210,7 +1128,7 @@ fn test_git_push_conflicting_bookmarks() { }; // Conflicting bookmark at @ - let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Warning: Bookmark bookmark2 is conflicted @@ -1219,10 +1137,8 @@ fn test_git_push_conflicting_bookmarks() { "###); // --bookmark should be blocked by conflicting bookmark - let stderr = test_env.jj_cmd_failure( - &workspace_root, - &["git", "push", "--allow-new", "--bookmark", "bookmark2"], - ); + let stderr = + test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--bookmark", "bookmark2"]); insta::assert_snapshot!(stderr, @r###" Error: Bookmark bookmark2 is conflicted Hint: Run `jj bookmark list` to inspect, and use `jj bookmark set` to fix it up. @@ -1241,8 +1157,7 @@ fn test_git_push_conflicting_bookmarks() { // --revisions shouldn't be blocked by conflicting bookmark bump_bookmark1(); - let (stdout, stderr) = - test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new", "-rall()"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-rall()"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Warning: Bookmark bookmark2 is conflicted @@ -1356,7 +1271,7 @@ fn test_git_push_moved_forward_untracked() { &workspace_root, &["bookmark", "untrack", "bookmark1@origin"], ); - let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); + let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stderr, @r###" Warning: Non-tracking remote bookmark bookmark1@origin exists Hint: Run `jj bookmark track bookmark1@origin` to import the remote bookmark. @@ -1377,7 +1292,7 @@ fn test_git_push_moved_sideways_untracked() { &workspace_root, &["bookmark", "untrack", "bookmark1@origin"], ); - let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); + let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stderr, @r###" Warning: Non-tracking remote bookmark bookmark1@origin exists Hint: Run `jj bookmark track bookmark1@origin` to import the remote bookmark. @@ -1385,6 +1300,131 @@ fn test_git_push_moved_sideways_untracked() { "###); } +#[test] +fn test_git_push_already_tracking() { + let test_env = TestEnvironment::default(); + // Set up colocated local workspace, which has the @git remote. + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "--colocate", "local"]); + let workspace_root = test_env.env_root().join("local"); + for remote in ["origin", "other"] { + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", remote]); + let path = test_env + .env_root() + .join(PathBuf::from_iter([remote, ".jj", "repo", "store", "git"])); + test_env.jj_cmd_ok( + &workspace_root, + &["git", "remote", "add", remote, path.to_str().unwrap()], + ); + } + + // Push newly-created bookmark to default + test_env.jj_cmd_ok(&workspace_root, &["commit", "-m1"]); + test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "-r@-", "foo"]); + let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::assert_snapshot!(stderr, @r" + Changes to push to origin: + Add bookmark foo to 8ee5f4bfcc8f + "); + + // Push newly-created bookmark to other + test_env.jj_cmd_ok(&workspace_root, &["commit", "-m2"]); + test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "-r@-", "bar"]); + let (_stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--remote=other", "--bookmark=bar"], + ); + insta::assert_snapshot!(stderr, @r" + Changes to push to other: + Add bookmark bar to 6836627dd409 + "); + + // Push newly-created --change bookmark to other + test_env.jj_cmd_ok(&workspace_root, &["commit", "-m3"]); + let (_stdout, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--remote=other", "-c@-"]); + insta::assert_snapshot!(stderr, @r" + Creating bookmark push-yostqsxwqrlt for revision yostqsxwqrlt + Changes to push to other: + Add bookmark push-yostqsxwqrlt to f8179f4c6018 + "); + + // Try to push some existing bookmarks to default + // (--change implies new tracking/creation, but -r doesn't) + let (_stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "-c@-", "-r@--", "--dry-run"], + ); + insta::assert_snapshot!(stderr, @r" + Warning: Refusing to track new remote origin by bookmark bar + Hint: Use --allow-new-tracking to push bookmark to new remote. Use --remote to specify the remote to push to. + Changes to push to origin: + Add bookmark push-yostqsxwqrlt to f8179f4c6018 + Dry-run requested, not pushing. + "); + + // Try to push tracking and newly-created bookmarks to default + test_env.jj_cmd_ok(&workspace_root, &["commit", "-m4"]); + test_env.jj_cmd_ok(&workspace_root, &["bookmark", "move", "--to=@-", "foo"]); + test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "-r@-", "baz"]); + let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--dry-run"]); + insta::assert_snapshot!(stderr, @r" + Warning: Refusing to track new remote origin by bookmark bar + Hint: Use --allow-new-tracking to push bookmark to new remote. Use --remote to specify the remote to push to. + Warning: Refusing to track new remote origin by bookmark push-yostqsxwqrlt + Hint: Use --allow-new-tracking to push bookmark to new remote. Use --remote to specify the remote to push to. + Changes to push to origin: + Add bookmark baz to 2333511334a0 + Move forward bookmark foo from 8ee5f4bfcc8f to 2333511334a0 + Dry-run requested, not pushing. + "); + + // Try with --allow-new-tracking + let (_stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--allow-new-tracking", "--dry-run"], + ); + insta::assert_snapshot!(stderr, @r" + Changes to push to origin: + Add bookmark bar to 6836627dd409 + Add bookmark baz to 2333511334a0 + Move forward bookmark foo from 8ee5f4bfcc8f to 2333511334a0 + Add bookmark push-yostqsxwqrlt to f8179f4c6018 + Dry-run requested, not pushing. + "); + + // Try with --all, which implies --allow-new-tracking + let (_stdout, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all", "--dry-run"]); + insta::assert_snapshot!(stderr, @r" + Changes to push to origin: + Add bookmark bar to 6836627dd409 + Add bookmark baz to 2333511334a0 + Move forward bookmark foo from 8ee5f4bfcc8f to 2333511334a0 + Add bookmark push-yostqsxwqrlt to f8179f4c6018 + Dry-run requested, not pushing. + "); + + // Try with --tracked, which effectively silences the warning + let (_stdout, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--tracked", "--dry-run"]); + insta::assert_snapshot!(stderr, @r" + Changes to push to origin: + Move forward bookmark foo from 8ee5f4bfcc8f to 2333511334a0 + Dry-run requested, not pushing. + "); + + // Try with --bookmark=NAME, which is an error. Git users might expect that + // the remote to push to is chosen by the tracking bookmark. + let stderr = test_env.jj_cmd_failure( + &workspace_root, + &["git", "push", "--bookmark=bar", "--dry-run"], + ); + insta::assert_snapshot!(stderr, @r" + Error: Refusing to track new remote origin by bookmark bar + Hint: Use --allow-new-tracking to push bookmark to new remote. Use --remote to specify the remote to push to. + "); +} + #[test] // TODO: This test fails with libgit2 v1.8.1 on Windows. #[cfg(not(target_os = "windows"))] diff --git a/cli/tests/test_undo.rs b/cli/tests/test_undo.rs index 1d1d1666e6..cbb6537dec 100644 --- a/cli/tests/test_undo.rs +++ b/cli/tests/test_undo.rs @@ -58,7 +58,7 @@ fn test_git_push_undo() { test_env.advance_test_rng_seed_to_multiple_of(100_000); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "main"]); test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "AA"]); - test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push"]); test_env.advance_test_rng_seed_to_multiple_of(100_000); test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "BB"]); // Refs at this point look as follows (-- means no ref) @@ -131,7 +131,7 @@ fn test_git_push_undo_with_import() { test_env.advance_test_rng_seed_to_multiple_of(100_000); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "main"]); test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "AA"]); - test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push"]); test_env.advance_test_rng_seed_to_multiple_of(100_000); test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "BB"]); // Refs at this point look as follows (-- means no ref) @@ -211,7 +211,7 @@ fn test_git_push_undo_colocated() { test_env.advance_test_rng_seed_to_multiple_of(100_000); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "main"]); test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "AA"]); - test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push"]); test_env.advance_test_rng_seed_to_multiple_of(100_000); test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "BB"]); // Refs at this point look as follows (-- means no ref) @@ -291,7 +291,7 @@ fn test_git_push_undo_repo_only() { test_env.advance_test_rng_seed_to_multiple_of(100_000); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "main"]); test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "AA"]); - test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push"]); insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" main: qpvuntsm 2080bdb8 (empty) AA @origin: qpvuntsm 2080bdb8 (empty) AA @@ -335,7 +335,7 @@ fn test_bookmark_track_untrack_undo() { test_env.jj_cmd_ok(&repo_path, &["describe", "-mcommit"]); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "feature1", "feature2"]); - test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push"]); test_env.jj_cmd_ok(&repo_path, &["bookmark", "delete", "feature2"]); insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" feature1: qpvuntsm 8da1cfc8 (empty) commit diff --git a/docs/bookmarks.md b/docs/bookmarks.md index 4c16e25c94..c71fd8bf22 100644 --- a/docs/bookmarks.md +++ b/docs/bookmarks.md @@ -194,8 +194,8 @@ makes several safety checks. 3. If the remote bookmark already exists on the remote, it must be [tracked](#remotes-and-tracked-bookmarks). If the bookmark does not already - exist on the remote, there is no problem; `jj git push --allow-new` will - create the remote bookmark and mark it as tracked. + exist on the remote, there is no problem; `jj git push` will create the + remote bookmark and mark it as tracked. [^known-issue]: See "A general note on safety" in diff --git a/docs/github.md b/docs/github.md index a572750c85..3532137e1d 100644 --- a/docs/github.md +++ b/docs/github.md @@ -48,7 +48,7 @@ $ jj commit -m 'feat(bar): add support for bar' # on the working-copy commit's *parent* because the working copy itself is empty. $ jj bookmark create bar -r @- # `bar` now contains the previous two commits. # Push the bookmark to GitHub (pushes only `bar`) -$ jj git push --allow-new +$ jj git push ``` While it's possible to create a bookmark in advance and commit on top of it in a @@ -80,7 +80,7 @@ $ # Do some more work. $ jj commit -m "Update tutorial" # Create a bookmark on the working-copy commit's parent $ jj bookmark create doc-update -r @- -$ jj git push --allow-new +$ jj git push ``` ## Working in a Jujutsu repository