diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d97601187..8a76b0dcf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * Default operation log template now shows end times of operations instead of start times. +* `jj git push` no longer pushes tracking bookmarks to new remotes. Specify + `--remote=NAME` to bypass this restriction. + ### Deprecations ### New features diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index 45e956eb5f..1729e7077b 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -155,6 +155,7 @@ pub fn cmd_git_push( } else { get_default_push_remote(ui, command.settings(), &git_repo)? }; + let track_new = args.remote.is_some(); let mut tx = workspace_command.start_transaction(); let view = tx.repo().view(); @@ -162,7 +163,7 @@ 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) { + match classify_bookmark_update(view, bookmark_name, &remote, targets, track_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -174,7 +175,7 @@ pub fn cmd_git_push( if !targets.remote_ref.is_tracking() { continue; } - match classify_bookmark_update(bookmark_name, &remote, targets) { + match classify_bookmark_update(view, bookmark_name, &remote, targets, track_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -186,7 +187,7 @@ pub fn cmd_git_push( if targets.local_target.is_present() { continue; } - match classify_bookmark_update(bookmark_name, &remote, targets) { + match classify_bookmark_update(view, bookmark_name, &remote, targets, track_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -223,7 +224,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(view, bookmark_name, &remote, targets, track_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => writeln!( ui.status(), @@ -246,7 +247,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(view, bookmark_name, &remote, targets, track_new) { Ok(Some(update)) => bookmark_updates.push((bookmark_name.to_owned(), update)), Ok(None) => {} Err(reason) => reason.print(ui)?, @@ -491,9 +492,11 @@ impl From for CommandError { } fn classify_bookmark_update( + view: &View, bookmark_name: &str, remote_name: &str, targets: LocalAndRemoteRef, + track_new: bool, ) -> Result, RejectedBookmarkUpdateReason> { let push_action = classify_bookmark_push_action(targets); match push_action { @@ -516,7 +519,44 @@ fn classify_bookmark_update( bookmark." )), }), - BookmarkPushAction::Update(update) => Ok(Some(update)), + BookmarkPushAction::Update(update) => { + if !targets.remote_ref.is_tracking() && !track_new { + reject_tracking_bookmark(view, bookmark_name, remote_name)?; + } + Ok(Some(update)) + } + } +} + +fn reject_tracking_bookmark( + view: &View, + bookmark_name: &str, + remote_name: &str, +) -> Result<(), RejectedBookmarkUpdateReason> { + let tracking_remote_names = view + .remote_bookmarks_matching( + &StringPattern::exact(bookmark_name), + &StringPattern::everything(), + ) + .filter(|&((_, remote), _)| remote != git::REMOTE_NAME_FOR_LOCAL_GIT_REPO) + .filter(|(_, remote_ref)| remote_ref.is_tracking()) + .map(|((_, remote), _)| remote) + .collect_vec(); + match &*tracking_remote_names { + [] => Ok(()), + [tracking_remote, ..] => { + let remote_bookmarks = tracking_remote_names + .iter() + .map(|remote| format!("{bookmark_name}@{remote}")) + .join(", "); + Err(RejectedBookmarkUpdateReason { + message: format!("Bookmark {bookmark_name} is already tracking {remote_bookmarks}"), + hint: Some(format!( + "Use --remote={tracking_remote} to update the remote bookmark. Use \ + --remote={remote_name} to push to new remote." + )), + }) + } } } diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index 4a34c22358..2142c7af33 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -1289,6 +1289,105 @@ fn test_git_push_moved_sideways_untracked() { "###); } +#[test] +fn test_git_push_already_tracking() { + let test_env = TestEnvironment::default(); + // Set up colocated local workspace, which has the @git remote. + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "--colocate", "local"]); + let workspace_root = test_env.env_root().join("local"); + for remote in ["origin", "other"] { + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", remote]); + let path = test_env + .env_root() + .join(PathBuf::from_iter([remote, ".jj", "repo", "store", "git"])); + test_env.jj_cmd_ok( + &workspace_root, + &["git", "remote", "add", remote, path.to_str().unwrap()], + ); + } + + // Push newly-created bookmark to default + test_env.jj_cmd_ok(&workspace_root, &["commit", "-m1"]); + test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "-r@-", "foo"]); + let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::assert_snapshot!(stderr, @r#" + Changes to push to origin: + Add bookmark foo to 8ee5f4bfcc8f + "#); + + // Push newly-created bookmark to other + test_env.jj_cmd_ok(&workspace_root, &["commit", "-m2"]); + test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "-r@-", "bar"]); + let (_stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--remote=other", "--bookmark=bar"], + ); + insta::assert_snapshot!(stderr, @r#" + Changes to push to other: + Add bookmark bar to 6836627dd409 + "#); + + // Try to push tracking and newly-created bookmarks to default + test_env.jj_cmd_ok(&workspace_root, &["commit", "-m2"]); + test_env.jj_cmd_ok(&workspace_root, &["bookmark", "move", "--to=@-", "foo"]); + test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "-r@-", "baz"]); + let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--dry-run"]); + insta::assert_snapshot!(stderr, @r#" + Warning: Bookmark bar is already tracking bar@other + Hint: Use --remote=other to update the remote bookmark. Use --remote=origin to push to new remote. + Changes to push to origin: + Add bookmark baz to 8a0af1306e25 + Move forward bookmark foo from 8ee5f4bfcc8f to 8a0af1306e25 + Dry-run requested, not pushing. + "#); + + // Try with explicit --remote argument, which bypasses the protection. + let (_stdout, stderr) = test_env.jj_cmd_ok( + &workspace_root, + &["git", "push", "--remote=origin", "--dry-run"], + ); + insta::assert_snapshot!(stderr, @r#" + Changes to push to origin: + Add bookmark bar to 6836627dd409 + Add bookmark baz to 8a0af1306e25 + Move forward bookmark foo from 8ee5f4bfcc8f to 8a0af1306e25 + Dry-run requested, not pushing. + "#); + + // Try with --all, which doesn't bypass the protection right now. + // TODO: Maybe okay to push all bookmarks because --all is an explicit flag? + let (_stdout, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all", "--dry-run"]); + insta::assert_snapshot!(stderr, @r#" + Warning: Bookmark bar is already tracking bar@other + Hint: Use --remote=other to update the remote bookmark. Use --remote=origin to push to new remote. + Changes to push to origin: + Add bookmark baz to 8a0af1306e25 + Move forward bookmark foo from 8ee5f4bfcc8f to 8a0af1306e25 + Dry-run requested, not pushing. + "#); + + // Try with --tracked, which effectively silences the warning. + let (_stdout, stderr) = + test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--tracked", "--dry-run"]); + insta::assert_snapshot!(stderr, @r#" + Changes to push to origin: + Move forward bookmark foo from 8ee5f4bfcc8f to 8a0af1306e25 + Dry-run requested, not pushing. + "#); + + // Try with --bookmark=NAME, which is an error. Git users might expect that + // the remote to push to is chosen by the tracking bookmark. + let stderr = test_env.jj_cmd_failure( + &workspace_root, + &["git", "push", "--bookmark=bar", "--dry-run"], + ); + insta::assert_snapshot!(stderr, @r#" + Error: Bookmark bar is already tracking bar@other + Hint: Use --remote=other to update the remote bookmark. Use --remote=origin to push to new remote. + "#); +} + #[test] // TODO: This test fails with libgit2 v1.8.1 on Windows. #[cfg(not(target_os = "windows"))]