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

…mediately push bookmark

Fixes #5472

CHANGELOG.md

@@ -63,6 +63,8 @@ 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.
 * The description of commits backed out by `jj backout` can now be configured
   using `templates.backout_description`.
 

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;
@@ -47,7 +48,9 @@ use crate::cli_util::CommandHelper;
 use crate::cli_util::RevisionArg;
 use crate::cli_util::WorkspaceCommandHelper;
 use crate::cli_util::WorkspaceCommandTransaction;
+use crate::command_error::cli_error;
 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 +81,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 +170,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 +265,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 +280,30 @@ pub fn cmd_git_push(
             }
         }
 
+        for name_revision in &args.named {
+            let (name, target) =
+                create_explicitly_named_bookmarks(ui, &remote, &mut tx, name_revision)?;
+
+            let allow_new = true;
+            match classify_bookmark_update(
+                &name,
+                &remote,
+                LocalAndRemoteRef {
+                    local_target: &target,
+                    remote_ref: &RemoteRef::absent(),
+                },
+                allow_new,
+            ) {
+                Ok(Some(update)) => bookmark_updates.push((name, 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 +320,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(),
@@ -672,6 +707,68 @@ fn classify_bookmark_update(
     }
 }
 
+// Creates the bookmars for `--named` arguments
+//
+// The logic is not identical to that of `jj bookmark create` since we need to
+// make sure the new bookmark is safe to push.
+fn create_explicitly_named_bookmarks(
+    ui: &mut Ui,
+    remote: &String,
+    tx: &mut WorkspaceCommandTransaction<'_>,
+    name_revision: &String,
+) -> Result<(String, RefTarget), CommandError> {
+    let Some((name, revision_str)) = name_revision.split_once('=') else {
+        return Err(cli_error(format!(
+            "Argument '{name_revision}' must include '=' and have the form NAME=REVISION"
+        ))
+        .hinted("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(cli_error(format!(
+            "Argument '{name_revision}' must have the form NAME=REVISION, with both NAME and \
+             REVISION non-empty."
+        ))
+        .hinted("For example, `--named myfeature=@` is valid syntax"));
+    }
+    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()
+    {
+        // This is slightly different from classify_bookmark_update since we
+        // error even in the case where the remote branch is not tracked. This can
+        // happen if the local branch is deleted by the (untracked) remote-tracking
+        // bookmark still exists.
+        //
+        // This may not be strictly necessary, but ensuring that the bookmark is always
+        // created on the remote from nothing simplifies both the UI and the logic.
+        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());
+    Ok((name.to_string(), ref_target))
+}
+
 /// Creates or moves bookmarks based on the change IDs.
 fn update_change_bookmarks(
     ui: &Ui,

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

@@ -1249,6 +1249,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,180 @@ 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", "pushed"]);
+    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 3e677c129c1d
+    ");
+    }
+    // test pushing a change with an empty name
+    let stderr = test_env.jj_cmd_cli_error(&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_cli_error(&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_cli_error(&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 4ab4513e b1* | pushed
+      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", "pushed_to_remote"]);
+    std::fs::write(workspace_root.join("file"), "contents").unwrap();
+    test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "child", "--no-edit"]);
+    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 = test_env.jj_cmd_success(&workspace_root, &["bookmark", "list", "--all", "b1"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stdout, @r#"b1@origin: yqosqzyt 6adf7da7 pushed_to_remote"#);
+    }
+    let stdout = test_env.jj_cmd_success(
+        &workspace_root,
+        &[
+            "log",
+            "-r=::@+",
+            r#"-T=separate(" ", commit_id.shortest(3), description)"#,
+        ],
+    );
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stdout, @r"
+    ○  237 child
+    @  6ad pushed_to_remote
+    ◆  000
+    ");    }
+
+    // The error would be the same whether we pushed `@` or `@+`. In principle, we
+    // could allow both, but it would create confusing corner cases.
+    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 make sure it still errors if we try to push a bookmark with the same name
+    // to a different location.
+    test_env.jj_cmd_ok(&workspace_root, &["bookmark", "forget", "b1"]);
+    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 237b1f3f2dda
+    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.
+    ");
+    }
+
+    // The bookmark is still forgotten
+    let stdout = test_env.jj_cmd_success(&workspace_root, &["bookmark", "list", "--all", "b1"]);
+    insta::allow_duplicates! {
+    insta::assert_snapshot!(stdout, @"");
+    }
+    // In this case, pushing the bookmark to the same location where it already is
+    // succeeds. TODO: This seems pretty safe, but perhaps it should still show
+    // an error or some sort of warning?
+    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 6adf7da7aa07
+    ");
+    }
+}
+
 #[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) {