cli: git push: do not push new bookmarks by default

CHANGELOG.md

@@ -10,6 +10,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Breaking changes
 
+* `jj git push` no longer pushes new bookmarks by default. Use `--allow-new` to
+  bypass this restriction.
+
 ### Deprecations
 
 ### New features

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=<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<StringPattern>,
-    /// 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<RevisionArg>,
     /// 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<Option<BookmarkPushUpdate>, 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)),
     }
 }

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=<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=<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 <remote name>` 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 <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 <REVISIONS>` — Push bookmarks pointing to these commits (can be repeated)
 * `-c`, `--change <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
 
 

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",

cli/tests/test_completion.rs

@@ -46,7 +46,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"]);

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