Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: push new non-tracking bookmarks by default, rename --allow-new flag #5173

Closed
wants to merge 1 commit into from
Closed

cli: push new non-tracking bookmarks by default, rename --allow-new flag #5173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 26 additions & 14 deletions cli/src/commands/git/push.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ use crate::ui::Ui;

/// Push to a Git remote
///
/// By default, pushes tracking bookmarks pointing to
/// `remote_bookmarks(remote=<remote>)..@`. Use `--bookmark` to push specific
/// By default, pushes new non-tracking and existing 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.
///
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)?,
Expand All @@ -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)?,
Expand All @@ -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)?,
Expand All @@ -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(),
Expand All @@ -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(),
Expand All @@ -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)?,
Expand Down Expand Up @@ -549,11 +550,20 @@ impl From<RejectedBookmarkUpdateReason> for CommandError {
}

fn classify_bookmark_update(
view: &View,
bookmark_name: &str,
remote_name: &str,
targets: LocalAndRemoteRef,
allow_new: bool,
) -> Result<Option<BookmarkPushUpdate>, 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),
Expand All @@ -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(),
),
})
Expand Down
4 changes: 2 additions & 2 deletions cli/tests/[email protected]
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=<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=<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.

Expand All @@ -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
Expand Down
14 changes: 6 additions & 8 deletions cli/tests/test_bookmark_command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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###"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
5 changes: 1 addition & 4 deletions cli/tests/test_completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]);
Expand Down
18 changes: 5 additions & 13 deletions cli/tests/test_git_private_commits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading