From 296961cb194211cecaded49f272d5fbfce5de68c Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 28 Oct 2024 17:43:16 +0900 Subject: [PATCH] cli: git push: do not push new bookmarks by default If you have multiple remotes to push to, you might want to keep some changes (such as security patches) in your private fork. Git CLI has one upstream remote per branch, but jj supports multiple tracking remotes, and therefore "jj git push" can start tracking new remotes automatically. This patch makes new bookmarks not eligible for push by default. I considered adding a warning, but it's not always possible to interrupt the push shortly after a warning is emitted. --all implies --allow-new because otherwise it's equivalent to --tracked. It's also easier to write a conflict rule with --all/--deleted/--tracked than with two of them. -c/--change doesn't require --allow-new because it is the flag to create new tracking bookmark. #1278 --- CHANGELOG.md | 3 + cli/src/commands/git/push.rs | 57 +++++++-- cli/tests/cli-reference@.md.snap | 9 +- cli/tests/test_bookmark_command.rs | 7 +- cli/tests/test_completion.rs | 5 +- cli/tests/test_git_private_commits.rs | 25 +++- cli/tests/test_git_push.rs | 164 ++++++++++++++++++++------ cli/tests/test_undo.rs | 10 +- docs/bookmarks.md | 4 +- docs/github.md | 4 +- 10 files changed, 219 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5ced5c4de..2ad6f28658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * `jj move` has been removed. It was deprecated in 0.16.0. +* `jj git push` no longer pushes new bookmarks by default. Use `--allow-new` to + bypass this restriction. + ### Deprecations ### New features diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index e854f3d264..c00938605f 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -57,7 +57,7 @@ use crate::ui::Ui; /// Push to a Git remote /// -/// By default, pushes any bookmarks pointing to +/// 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. @@ -98,7 +98,7 @@ pub struct GitPushArgs { add = ArgValueCandidates::new(complete::local_bookmarks), )] bookmark: Vec, - /// Push all bookmarks (including deleted bookmarks) + /// Push all bookmarks (including new and deleted bookmarks) #[arg(long)] all: bool, /// Push all tracked bookmarks (including deleted bookmarks) @@ -115,6 +115,11 @@ pub struct GitPushArgs { /// correspond to missing local bookmarks. #[arg(long)] deleted: bool, + /// Allow pushing new bookmarks + /// + /// Newly-created remote bookmarks will be tracked automatically. + #[arg(long, conflicts_with = "what")] + allow_new: bool, /// Allow pushing commits with empty descriptions #[arg(long)] allow_empty_description: bool, @@ -127,8 +132,9 @@ pub struct GitPushArgs { /// Push this commit by creating a bookmark based on its change ID (can be /// repeated) /// - /// Use the `git.push-bookmark-prefix` setting to change the prefix for - /// generated names. + /// The created bookmark will be tracked automatically. Use the + /// `git.push-bookmark-prefix` setting to change the prefix for generated + /// names. #[arg(long, short)] change: Vec, /// Only display what will change on the remote @@ -172,7 +178,8 @@ pub fn cmd_git_push( let mut bookmark_updates = vec![]; if args.all { for (bookmark_name, targets) in view.local_remote_bookmarks(&remote) { - match classify_bookmark_update(bookmark_name, &remote, targets) { + let allow_new = true; // implied by --all + match classify_bookmark_update(bookmark_name, &remote, targets, allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -184,7 +191,8 @@ pub fn cmd_git_push( if !targets.remote_ref.is_tracking() { continue; } - match classify_bookmark_update(bookmark_name, &remote, targets) { + let allow_new = false; // doesn't matter + match classify_bookmark_update(bookmark_name, &remote, targets, allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -196,7 +204,8 @@ pub fn cmd_git_push( if targets.local_target.is_present() { continue; } - match classify_bookmark_update(bookmark_name, &remote, targets) { + let allow_new = false; // doesn't matter + match classify_bookmark_update(bookmark_name, &remote, targets, allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -228,12 +237,27 @@ pub fn cmd_git_push( (bookmark_name.as_ref(), targets) }); let view = tx.repo().view(); + for (bookmark_name, targets) in change_bookmarks { + if !seen_bookmarks.insert(bookmark_name) { + continue; + } + let allow_new = true; // --change implies creation of remote bookmark + match classify_bookmark_update(bookmark_name, &remote, targets, allow_new) { + Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), + Ok(None) => writeln!( + ui.status(), + "Bookmark {bookmark_name}@{remote} already matches {bookmark_name}", + )?, + Err(reason) => return Err(reason.into()), + } + } + let bookmarks_by_name = find_bookmarks_to_push(view, &args.bookmark, &remote)?; - for (bookmark_name, targets) in change_bookmarks.chain(bookmarks_by_name.iter().copied()) { + for &(bookmark_name, targets) in &bookmarks_by_name { if !seen_bookmarks.insert(bookmark_name) { continue; } - match classify_bookmark_update(bookmark_name, &remote, targets) { + match classify_bookmark_update(bookmark_name, &remote, targets, args.allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => writeln!( ui.status(), @@ -256,7 +280,7 @@ pub fn cmd_git_push( if !seen_bookmarks.insert(bookmark_name) { continue; } - match classify_bookmark_update(bookmark_name, &remote, targets) { + match classify_bookmark_update(bookmark_name, &remote, targets, args.allow_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -504,6 +528,7 @@ fn classify_bookmark_update( bookmark_name: &str, remote_name: &str, targets: LocalAndRemoteRef, + allow_new: bool, ) -> Result, RejectedBookmarkUpdateReason> { let push_action = classify_bookmark_push_action(targets); match push_action { @@ -526,6 +551,18 @@ fn classify_bookmark_update( bookmark." )), }), + BookmarkPushAction::Update(update) if update.old_target.is_none() && !allow_new => { + Err(RejectedBookmarkUpdateReason { + message: format!( + "Refusing to create new remote bookmark {bookmark_name}@{remote_name}" + ), + hint: Some( + "Use --allow-new to push new bookmark. Use --remote to specify the remote to \ + push to." + .to_owned(), + ), + }) + } BookmarkPushAction::Update(update) => Ok(Some(update)), } } diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 364ebf3a47..e3da2f4863 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1148,7 +1148,7 @@ Create a new Git backed repo Push to a Git remote -By default, pushes any 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 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. Before the command actually moves, creates, or deletes a remote bookmark, it makes several [safety checks]. If there is a problem, you may need to run `jj git fetch --remote ` and/or resolve some [bookmark conflicts]. @@ -1166,19 +1166,22 @@ Before the command actually moves, creates, or deletes a remote bookmark, it mak * `-b`, `--bookmark ` — Push only this bookmark, or bookmarks matching a pattern (can be repeated) By default, the specified name matches exactly. Use `glob:` prefix to select bookmarks by wildcard pattern. For details, see https://martinvonz.github.io/jj/latest/revsets#string-patterns. -* `--all` — Push all bookmarks (including deleted bookmarks) +* `--all` — Push all bookmarks (including new and deleted bookmarks) * `--tracked` — Push all tracked bookmarks (including deleted bookmarks) This usually means that the bookmark was already pushed to or fetched from the relevant remote. For details, see https://martinvonz.github.io/jj/latest/bookmarks#remotes-and-tracked-bookmarks * `--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. +* `--allow-new` — Allow pushing new bookmarks + + Newly-created remote bookmarks will be tracked automatically. * `--allow-empty-description` — Allow pushing commits with empty descriptions * `--allow-private` — Allow pushing commits that are private * `-r`, `--revisions ` — Push bookmarks pointing to these commits (can be repeated) * `-c`, `--change ` — Push this commit by creating a bookmark based on its change ID (can be repeated) - Use the `git.push-bookmark-prefix` setting to change the prefix for generated names. + The created bookmark will be tracked automatically. Use the `git.push-bookmark-prefix` setting to change the prefix for generated names. * `--dry-run` — Only display what will change on the remote diff --git a/cli/tests/test_bookmark_command.rs b/cli/tests/test_bookmark_command.rs index aace7ee17e..4f53ee9798 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", "-r@-"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new", "-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", "-b=bremote"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new", "-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", "-b", "main"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new", "-b", "main"]); test_env.jj_cmd_ok(&repo_path, &["bookmark", "untrack", "main@origin"]); test_env.jj_cmd_ok( &repo_path, @@ -1757,6 +1757,7 @@ fn test_bookmark_list_tracked() { &[ "git", "push", + "--allow-new", "--remote", "upstream", "--bookmark", diff --git a/cli/tests/test_completion.rs b/cli/tests/test_completion.rs index d7d1031c02..971222a7cf 100644 --- a/cli/tests/test_completion.rs +++ b/cli/tests/test_completion.rs @@ -49,7 +49,10 @@ 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", "--bookmark", "glob:*-tracked"]); + test_env.jj_cmd_ok( + &repo_path, + &["git", "push", "--allow-new", "--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 a70a110bcc..fadd5ca05b 100644 --- a/cli/tests/test_git_private_commits.rs +++ b/cli/tests/test_git_private_commits.rs @@ -67,7 +67,14 @@ fn set_up_remote_at_main(test_env: &TestEnvironment, workspace_root: &Path, remo ); test_env.jj_cmd_ok( workspace_root, - &["git", "push", "--remote", remote_name, "-b=main"], + &[ + "git", + "push", + "--allow-new", + "--remote", + remote_name, + "-b=main", + ], ); } @@ -155,7 +162,10 @@ 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", "-b=bookmark1"]); + let stderr = test_env.jj_cmd_failure( + &workspace_root, + &["git", "push", "--allow-new", "-b=bookmark1"], + ); insta::assert_snapshot!(stderr, @r###" Error: Won't push commit f1253a9b1ea9 since it is private "###); @@ -192,8 +202,10 @@ 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", "-b=main", "-b=bookmark1"]); + let (_, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--allow-new", "-b=main", "-b=bookmark1"], + ); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move forward bookmark main from 7eb97bf230ad to fbb352762352 @@ -223,7 +235,10 @@ 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", "-b=bookmark2"]); + let (_, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--allow-new", "-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 18522a5025..5adac498a9 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -88,7 +88,10 @@ 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", "--dry-run"]); + let (stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--allow-new", "--dry-run"], + ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -96,7 +99,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"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -232,7 +235,10 @@ fn test_git_push_other_remote_has_bookmark() { // as it is on the remote. This would also work for a descendant. // // TODO: Saner test? - let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--remote=other"]); + let (stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--allow-new", "--remote=other"], + ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to other: @@ -416,7 +422,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"]); + let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--allow-new"]); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark bookmark1 to cb17dcdc74d5 @@ -435,6 +441,12 @@ 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 @@ -443,19 +455,19 @@ fn test_git_push_locally_created_and_rewritten() { // Rewrite it and push again, which would fail if the pushed bookmark weren't // set to "tracking" test_env.jj_cmd_ok(&workspace_root, &["describe", "-mlocal 2"]); - insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" + insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r" bookmark1: xtvrqkyv d13ecdbd (empty) description 1 @origin: xtvrqkyv d13ecdbd (empty) description 1 bookmark2: rlzusymt 8476341e (empty) description 2 @origin: rlzusymt 8476341e (empty) description 2 - my: vruxwmqv bde1d2e4 (empty) local 2 + my: vruxwmqv eaf7a52c (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#" + insta::assert_snapshot!(stderr, @r" Changes to push to origin: - Move sideways bookmark my from fcc999921ce9 to bde1d2e44b2a - "#); + Move sideways bookmark my from fcc999921ce9 to eaf7a52c8906 + "); } #[test] @@ -490,7 +502,14 @@ fn test_git_push_multiple() { // Dry run requesting two specific bookmarks let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, - &["git", "push", "-b=bookmark1", "-b=my-bookmark", "--dry-run"], + &[ + "git", + "push", + "--allow-new", + "-b=bookmark1", + "-b=my-bookmark", + "--dry-run", + ], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -505,6 +524,7 @@ fn test_git_push_multiple() { &[ "git", "push", + "--allow-new", "-b=bookmark1", "-b=my-bookmark", "-b=bookmark1", @@ -714,21 +734,27 @@ 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", "-r=none()"]); + let (_stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--allow-new", "-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", "-r=@--"]); + let (stdout, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new", "-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", "-r=@-", "--dry-run"]); + let (stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--allow-new", "-r=@-", "--dry-run"], + ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -738,7 +764,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", "-r=@--", "-r=@-", "--dry-run"], + &["git", "push", "--allow-new", "-r=@--", "-r=@-", "--dry-run"], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -748,8 +774,10 @@ 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", "-r=@", "--dry-run"]); + let (stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--allow-new", "-r=@", "--dry-run"], + ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -760,7 +788,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", "-r=@-", "-r=@-", "--dry-run"], + &["git", "push", "--allow-new", "-r=@-", "-r=@-", "--dry-run"], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -783,11 +811,29 @@ 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=@", @@ -860,7 +906,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", "--bookmark", "my-bookmark"], + &["git", "push", "--allow-new", "--bookmark", "my-bookmark"], ); insta::assert_snapshot!(stderr, @r###" Error: Won't push commit 5b36783cd11c since it has no description @@ -870,6 +916,7 @@ fn test_git_push_no_description() { &[ "git", "push", + "--allow-new", "--bookmark", "my-bookmark", "--allow-empty-description", @@ -888,7 +935,13 @@ fn test_git_push_no_description_in_immutable() { let stderr = test_env.jj_cmd_failure( &workspace_root, - &["git", "push", "--bookmark=my-bookmark", "--dry-run"], + &[ + "git", + "push", + "--allow-new", + "--bookmark=my-bookmark", + "--dry-run", + ], ); insta::assert_snapshot!(stderr, @r###" Error: Won't push commit 5b36783cd11c since it has no description @@ -897,7 +950,13 @@ 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", "--bookmark=my-bookmark", "--dry-run"], + &[ + "git", + "push", + "--allow-new", + "--bookmark=my-bookmark", + "--dry-run", + ], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -921,7 +980,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", "--bookmark", "missing-name"], + &["git", "push", "--allow-new", "--bookmark", "missing-name"], ); insta::assert_snapshot!(stderr, @r###" Error: Won't push commit 944313939bbd since it has no author and/or committer set @@ -930,7 +989,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", "--bookmark=missing-email"], + &["git", "push", "--allow-new", "--bookmark=missing-email"], ); insta::assert_snapshot!(stderr, @r###" Error: Won't push commit 59354714f789 since it has no author and/or committer set @@ -956,7 +1015,13 @@ fn test_git_push_missing_author_in_immutable() { let stderr = test_env.jj_cmd_failure( &workspace_root, - &["git", "push", "--bookmark=my-bookmark", "--dry-run"], + &[ + "git", + "push", + "--allow-new", + "--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 @@ -965,7 +1030,13 @@ 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", "--bookmark=my-bookmark", "--dry-run"], + &[ + "git", + "push", + "--allow-new", + "--bookmark=my-bookmark", + "--dry-run", + ], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -987,8 +1058,10 @@ 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", "--bookmark=missing-name"]); + let stderr = test_env.jj_cmd_failure( + &workspace_root, + &["git", "push", "--allow-new", "--bookmark=missing-name"], + ); insta::assert_snapshot!(stderr, @r###" Error: Won't push commit 4fd190283d1a since it has no author and/or committer set "###); @@ -997,7 +1070,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", "--bookmark=missing-email"], + &["git", "push", "--allow-new", "--bookmark=missing-email"], ); insta::assert_snapshot!(stderr, @r###" Error: Won't push commit eab97428a6ec since it has no author and/or committer set @@ -1008,7 +1081,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", "--bookmark=missing-email"], + &["git", "push", "--allow-new", "--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 @@ -1035,7 +1108,13 @@ fn test_git_push_missing_committer_in_immutable() { let stderr = test_env.jj_cmd_failure( &workspace_root, - &["git", "push", "--bookmark=my-bookmark", "--dry-run"], + &[ + "git", + "push", + "--allow-new", + "--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 @@ -1044,7 +1123,13 @@ 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", "--bookmark=my-bookmark", "--dry-run"], + &[ + "git", + "push", + "--allow-new", + "--bookmark=my-bookmark", + "--dry-run", + ], ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" @@ -1117,7 +1202,7 @@ fn test_git_push_conflicting_bookmarks() { }; // Conflicting bookmark at @ - let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Warning: Bookmark bookmark2 is conflicted @@ -1126,8 +1211,10 @@ fn test_git_push_conflicting_bookmarks() { "###); // --bookmark should be blocked by conflicting bookmark - let stderr = - test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--bookmark", "bookmark2"]); + let stderr = test_env.jj_cmd_failure( + &workspace_root, + &["git", "push", "--allow-new", "--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. @@ -1146,7 +1233,8 @@ 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", "-rall()"]); + let (stdout, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new", "-rall()"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Warning: Bookmark bookmark2 is conflicted @@ -1260,7 +1348,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"]); + let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); 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. @@ -1281,7 +1369,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"]); + let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); 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. diff --git a/cli/tests/test_undo.rs b/cli/tests/test_undo.rs index cbb6537dec..1d1d1666e6 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"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); 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"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); 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"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); 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"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); 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"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push", "--allow-new"]); 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 a3148ec7c1..cc63f5316d 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` will create the - remote bookmark and mark it as tracked. + exist on the remote, there is no problem; `jj git push --allow-new` 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 5a1bc67f1b..b1528aecb5 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 +$ jj git push --allow-new ``` While it's possible to create a bookmark in advance and commit on top of it in a @@ -78,7 +78,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 +$ jj git push --allow-new ``` ## Working in a Jujutsu repository