cli git push: new --named NAME=REVISION argument to create and im…

…mediately push bookmark

Fixes #5472

CHANGELOG.md

@@ -51,6 +51,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   useful for making machine-readable templates by escaping problematic
   characters like `\n`.
 
+* `jj git push` now accepts a `--named NAME=REVISION` argument to create a named
+  bookmark and immediately push it.
+
 ### Fixed bugs
 
 * `jj status` now shows untracked files under untracked directories.

cli/src/commands/git/push.rs

@@ -30,6 +30,7 @@ use jj_lib::git;
 use jj_lib::git::GitBranchPushTargets;
 use jj_lib::object_id::ObjectId;
 use jj_lib::op_store::RefTarget;
+use jj_lib::op_store::RemoteRef;
 use jj_lib::refs::classify_bookmark_push_action;
 use jj_lib::refs::BookmarkPushAction;
 use jj_lib::refs::BookmarkPushUpdate;
@@ -48,6 +49,7 @@ use crate::cli_util::RevisionArg;
 use crate::cli_util::WorkspaceCommandHelper;
 use crate::cli_util::WorkspaceCommandTransaction;
 use crate::command_error::user_error;
+use crate::command_error::user_error_with_hint;
 use crate::command_error::CommandError;
 use crate::commands::git::get_single_remote;
 use crate::complete;
@@ -78,7 +80,7 @@ use crate::ui::Ui;
 ///     https://jj-vcs.github.io/jj/latest/bookmarks/#conflicts
 
 #[derive(clap::Args, Clone, Debug)]
-#[command(group(ArgGroup::new("specific").args(&["bookmark", "change", "revisions"]).multiple(true)))]
+#[command(group(ArgGroup::new("specific").args(&["bookmark", "change", "revisions", "named"]).multiple(true)))]
 #[command(group(ArgGroup::new("what").args(&["all", "deleted", "tracked"]).conflicts_with("specific")))]
 pub struct GitPushArgs {
     /// The remote to push to (only named remotes are supported)
@@ -167,6 +169,13 @@ pub struct GitPushArgs {
         add = ArgValueCandidates::new(complete::mutable_revisions)
     )]
     change: Vec<RevisionArg>,
+    /// Specify a new bookmark name and a revision to push under that name, e.g.
+    /// '--named myfeature=@'.
+    ///
+    /// Does not require --allow-new
+    // TODO: Add arg completer
+    #[arg(long, value_name = "NAME=REVISION")]
+    named: Vec<String>,
     /// Only display what will change on the remote
     #[arg(long)]
     dry_run: bool,
@@ -255,7 +264,6 @@ 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;
@@ -271,6 +279,77 @@ pub fn cmd_git_push(
             }
         }
 
+        for name_revision in &args.named {
+            let Some((name, revision_str)) = name_revision.split_once('=') else {
+                return Err(user_error_with_hint(
+                    format!(
+                        "Argument '{name_revision}' must include '=' and have the form \
+                         NAME=REVISION"
+                    ),
+                    "For example, `--named myfeature=@` is valid syntax",
+                ));
+            };
+            // Revisions are lenient towards spaces, so let's make the name lenient as well
+            // Git does not allow bookmark names that include spaces anyway.
+            let name = name.trim();
+            if name.is_empty() || revision_str.is_empty() {
+                return Err(user_error_with_hint(
+                    format!(
+                        "Argument '{name_revision}' must have the form NAME=REVISION, with both \
+                         NAME and REVISION non-empty."
+                    ),
+                    "For example, `--named myfeature=@` is valid syntax",
+                ));
+            }
+            // The logic here is similar to but not quite identical to the logic in `jj
+            // bookmark create`
+            if tx.repo().view().get_local_bookmark(name).is_present() {
+                return Err(user_error_with_hint(
+                    format!("Bookmark already exists: '{name}'"),
+                    format!(
+                        "Use 'jj bookmark move' to move it, and 'jj git push -b {name} \
+                         [--allow-new]' to push it."
+                    ),
+                ));
+            }
+            if tx
+                .repo()
+                .view()
+                .get_remote_bookmark(name, &remote)
+                .is_present()
+            {
+                return Err(user_error_with_hint(
+                    format!("Remote bookmark already exists: '{name}@{remote}'"),
+                    "Use 'jj bookmark track' to track it.",
+                ));
+            }
+            let revision = tx
+                .base_workspace_helper()
+                .resolve_single_rev(ui, &revision_str.to_string().into())?;
+            let ref_target = RefTarget::normal(revision.id().clone());
+            tx.repo_mut()
+                .set_local_bookmark_target(name, ref_target.clone());
+
+            let allow_new = true;
+            match classify_bookmark_update(
+                name,
+                &remote,
+                LocalAndRemoteRef {
+                    local_target: &ref_target,
+                    remote_ref: &RemoteRef::absent(),
+                },
+                allow_new,
+            ) {
+                Ok(Some(update)) => bookmark_updates.push((name.to_owned(), update)),
+                Ok(None) => writeln!(
+                    ui.status(),
+                    "Bookmark {name}@{remote} already matches the desired revision",
+                )?,
+                Err(reason) => return Err(reason.into()),
+            }
+        }
+
+        let view = tx.repo().view();
         let allow_new = args.allow_new || tx.settings().get("git.push-new-bookmarks")?;
         let bookmarks_by_name = find_bookmarks_to_push(view, &args.bookmark, &remote)?;
         for &(bookmark_name, targets) in &bookmarks_by_name {
@@ -287,8 +366,10 @@ pub fn cmd_git_push(
             }
         }
 
-        let use_default_revset =
-            args.bookmark.is_empty() && args.change.is_empty() && args.revisions.is_empty();
+        let use_default_revset = args.bookmark.is_empty()
+            && args.change.is_empty()
+            && args.revisions.is_empty()
+            && args.named.is_empty();
         let bookmarks_targeted = find_bookmarks_targeted_by_revisions(
             ui,
             tx.base_workspace_helper(),

cli/tests/cli-reference@.md.snap

@@ -1238,6 +1238,9 @@ Before the command actually moves, creates, or deletes a remote bookmark, it mak
 * `-c`, `--change <REVSETS>` — Push this commit by creating a bookmark based on its change ID (can be repeated)
 
    The created bookmark will be tracked automatically. Use the `git.push-bookmark-prefix` setting to change the prefix for generated names.
+* `--named <NAME=REVISION>` — Specify a new bookmark name and a revision to push under that name, e.g. '--named myfeature=@'.
+
+   Does not require --allow-new
 * `--dry-run` — Only display what will change on the remote
 
 

cli/tests/test_git_push.rs

@@ -954,6 +954,165 @@ fn test_git_push_changes(subprocess: bool) {
     }
 }
 
+#[test_case(false; "use git2 for remote calls")]
+#[test_case(true; "spawn a git subprocess for remote calls")]
+fn test_git_push_changes_with_name(subprocess: bool) {
+    let (test_env, workspace_root) = set_up();
+    if subprocess {
+        test_env.add_config("git.subprocess = true");
+    }
+    test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]);
+    std::fs::write(workspace_root.join("file"), "contents").unwrap();
+    test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "bar"]);
+    std::fs::write(workspace_root.join("file"), "modified").unwrap();
+
+    // Normal behavior. Spaces around the = sign are OK.
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--named", "b1 = @"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stdout, @"");
+    }
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stderr, @r"
+    Changes to push to origin:
+      Add bookmark b1 to cf1a53a8800a
+    ");
+    }
+    // test pushing a change with an empty name
+    let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--named", "=@"]);
+    insta::allow_duplicates! {
+        insta::assert_snapshot!(stderr, @r"
+    Error: Argument '=@' must have the form NAME=REVISION, with both NAME and REVISION non-empty.
+    Hint: For example, `--named myfeature=@` is valid syntax
+    ");    
+    }
+    // test pushing a change with an empty revision
+    let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--named", "b2="]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stderr, @r"
+    Error: Argument 'b2=' must have the form NAME=REVISION, with both NAME and REVISION non-empty.
+    Hint: For example, `--named myfeature=@` is valid syntax
+    ");
+    }
+    // test pushing a change with no equals sign
+    let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--named", "b2"]);
+    insta::allow_duplicates! {
+        insta::assert_snapshot!(stderr, @r"
+    Error: Argument 'b2' must include '=' and have the form NAME=REVISION
+    Hint: For example, `--named myfeature=@` is valid syntax
+    ");
+    }
+
+    // test pushing the same change with the same name again
+    let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--named", "b1=@"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stderr, @r"
+    Error: Bookmark already exists: 'b1'
+    Hint: Use 'jj bookmark move' to move it, and 'jj git push -b b1 [--allow-new]' to push it.
+    ");
+    }
+    // test pushing two changes at once
+    std::fs::write(workspace_root.join("file"), "modified2").unwrap();
+    let stderr =
+        test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--named=b2=all:(@|@-)"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stderr, @r"
+    Error: Revset `all:(@|@-)` resolved to more than one revision
+    Hint: The revset `all:(@|@-)` resolved to these revisions:
+      yostqsxw e5fc831c b1* | bar
+      yqosqzyt a050abf4 foo
+    ");
+    }
+
+    // specifying the same bookmark with --named/--bookmark
+    std::fs::write(workspace_root.join("file"), "modified4").unwrap();
+    let stderr =
+        test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--named=b2=@", "-b=b2"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stderr, @r"
+    Error: Refusing to create new remote bookmark b2@origin
+    Hint: Use --allow-new to push new bookmark. Use --remote to specify the remote to push to.
+    ");
+    }
+}
+
+#[test_case(false; "use git2 for remote calls")]
+#[test_case(true; "spawn a git subprocess for remote calls")]
+fn test_git_push_changes_with_name_untracked_or_forgotten(subprocess: bool) {
+    let (test_env, workspace_root) = set_up();
+    if subprocess {
+        test_env.add_config("git.subprocess = true");
+    }
+    test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]);
+    std::fs::write(workspace_root.join("file"), "contents").unwrap();
+    test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "bar"]);
+    std::fs::write(workspace_root.join("file"), "modified").unwrap();
+    // Unset immutable_heads so that untracking branches does not move the working
+    // copy
+    test_env.jj_cmd_ok(
+        &workspace_root,
+        &[
+            "config",
+            "set",
+            "--repo",
+            r#"revset-aliases."immutable_heads()""#,
+            r#""none()""#,
+        ],
+    );
+
+    // Push a branch to a remote, but forget the local branch
+    test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--named", "b1=@"]);
+    test_env.jj_cmd_ok(&workspace_root, &["bookmark", "untrack", "b1@origin"]);
+    test_env.jj_cmd_ok(&workspace_root, &["bookmark", "delete", "b1"]);
+
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&workspace_root, &["bookmark", "list", "--all", "b1"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stdout, @"b1@origin: yostqsxw 5f432a85 bar");
+    }
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stderr, @"");
+    }
+
+    let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--named", "b1=@"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stderr, @r"
+    Error: Remote bookmark already exists: 'b1@origin'
+    Hint: Use 'jj bookmark track' to track it.
+    ");
+    }
+
+    // Now, the bookmarked is pushed to the remote. Let's entirely forget it locally
+    // and try pushing it again to the same location it was.
+    // TODO: This seems pretty safe, but perhaps it should still show an error or
+    // some sort of warning?
+    test_env.jj_cmd_ok(&workspace_root, &["bookmark", "forget", "b1"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--named", "b1=@"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stdout, @"");
+    }
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stderr, @r"
+    Changes to push to origin:
+      Add bookmark b1 to 5f432a855e59
+    ");
+    }
+
+    // Forget the bookmark once more
+    test_env.jj_cmd_ok(&workspace_root, &["bookmark", "forget", "b1"]);
+    // Now, we'll push a new commit
+    test_env.jj_cmd_ok(&workspace_root, &["new", "-m=new commit"]);
+    let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--named", "b1=@"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stderr, @r"
+    Changes to push to origin:
+      Add bookmark b1 to 97505688dff3
+    Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/b1
+    Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again.
+    ");
+    }
+}
+
 #[test_case(false; "use git2 for remote calls")]
 #[test_case(true; "spawn a git subprocess for remote calls")]
 fn test_git_push_revisions(subprocess: bool) {