diff --git a/src/commands/branch.rs b/src/commands/branch.rs index 9e255a91fd..55d423bc5c 100644 --- a/src/commands/branch.rs +++ b/src/commands/branch.rs @@ -3,7 +3,8 @@ use std::collections::{BTreeSet, HashSet}; use clap::builder::NonEmptyStringValueParser; use itertools::Itertools; use jujutsu_lib::backend::{CommitId, ObjectId}; -use jujutsu_lib::git::local_branch_git_tracking_refs; +use jujutsu_lib::git::{self, local_branch_git_tracking_refs}; +use jujutsu_lib::git_backend::GitBackend; use jujutsu_lib::op_store::{BranchTarget, RefTarget}; use jujutsu_lib::repo::Repo; use jujutsu_lib::revset::{self, RevsetExpression}; @@ -74,11 +75,24 @@ pub struct BranchListArgs { revisions: Vec, } -/// Forget everything about a branch, including its local and remote -/// targets. +/// Forget everything about a branch, including its local and remote targets. /// /// A forgotten branch will not impact remotes on future pushes. It will be /// recreated on future pulls if it still exists in the remote. +/// +/// jj will immediately attempt to remove this branch and the corresponding +/// remote-tracking branches from the local git repo. +/// +/// NOTE: This happens even if a repo is not colocated and `jj git export` is +/// not already applied after every command. If you are using a non-colocated +/// repo and are counting on this command not touching the git repo's branch +/// refs, please report your use case on our bug tracker. +// +// More details on the above note: The issue is that if the remote-tracking branch in the local git +// repo wasn't immediately deleted, and that branch didn't move on a remote, a subsequent `jj git +// fetch` would think the branch is unchanged and wouldn't resurrect the branch in jj (and there +// wouldn't be a good way to resurrect it for the user). If the branch did move on the remote, there +// would be a conflict. See also `test_branch_forget_fetched_branch`. #[derive(clap::Args, Clone, Debug)] pub struct BranchForgetArgs { /// The branches to forget. @@ -296,6 +310,16 @@ fn cmd_branch_forget( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let view = workspace_command.repo().view(); + // This is based on commands::git::get_git_repo, but we don't want to abort with + // a user error even if the backend is not git. + // TODO: Some abstraction that allows the backend choose what to do here and + // removes GitBackend-specific code. + let git_repo = workspace_command + .repo() + .store() + .backend_impl() + .downcast_ref::() + .map(|backend| backend.git_repo_clone()); for branch_name in args.names.iter() { if view.get_branch(branch_name).is_none() { return Err(user_error(format!("No such branch: {branch_name}"))); @@ -308,6 +332,29 @@ fn cmd_branch_forget( for branch_name in names.iter() { tx.mut_repo().remove_branch(branch_name); } + + // Immediate export of the deletion of remote-tracking branches is necessary so + // that subsequent `jj git fetch` work in non-colocated repositories. It would + // be confusing to export deletion of the remote-tracking branches and not + // of the local branch, even though the latter only exists if the user runs + // `jj git export` manually in a non-colocated repo. + // TODO: Create a command the user can use to reset jj's branch state to the git + // repo's state. In this case, `jj branch forget` doesn't work as it tries + // to delete the latter. One possible name is `jj git import --reset + // BRANCH`. + // TODO: Once the command described above exists, it should be mentioned in `jj + // help branch forget`. + if let Some(git_repo) = git_repo { + git::export_some_refs(tx.mut_repo(), &git_repo, |ref_name| match ref_name { + jujutsu_lib::view::RefName::LocalBranch(branch) => names.contains(branch), + jujutsu_lib::view::RefName::RemoteBranch { branch, remote: _ } => { + names.contains(branch) + } + _ => false, + })?; + } else { + // We don't know how to export to other backends. + } tx.finish(ui)?; if names.len() > 1 { writeln!(ui, "Forgot {} branches.", names.len())?; diff --git a/tests/test_branch_command.rs b/tests/test_branch_command.rs index 8a71aef7ce..c33623af5d 100644 --- a/tests/test_branch_command.rs +++ b/tests/test_branch_command.rs @@ -236,46 +236,24 @@ fn test_branch_forget_export() { // Exporting the branch to git creates a local-git tracking branch let stdout = test_env.jj_cmd_success(&repo_path, &["git", "export"]); insta::assert_snapshot!(stdout, @""); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r=foo@git", "--no-graph"]); + insta::assert_snapshot!(stdout, @r###" + rlvkpnrzqnoo test.user@example.com 2001-02-03 04:05:08.000 +07:00 foo 65b6b74e0897 + (empty) (no description set) + "###); let stdout = test_env.jj_cmd_success(&repo_path, &["branch", "forget", "foo"]); insta::assert_snapshot!(stdout, @""); - // Forgetting a branch does not delete its local-git tracking branch. This is - // the opposite of what happens to remote-tracking branches. - // TODO: Consider allowing forgetting local-git tracking branches as an option + // Forgetting a branch deletes its local-git tracking branch immediately let stdout = test_env.jj_cmd_success(&repo_path, &["branch", "list"]); - insta::assert_snapshot!(stdout, @r###" - foo (forgotten) - @git: 65b6b74e0897 (no description set) - (this branch will be deleted from the underlying Git repo on the next `jj git export`) + insta::assert_snapshot!(stdout, @""); + let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r=foo@git", "--no-graph"]); + insta::assert_snapshot!(stderr, @r###" + Error: Revision "foo@git" doesn't exist "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r=foo", "--no-graph"]); insta::assert_snapshot!(stderr, @r###" Error: Revision "foo" doesn't exist "###); - let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r=foo@git", "--no-graph"]); - insta::assert_snapshot!(stdout, @r###" - rlvkpnrzqnoo test.user@example.com 2001-02-03 04:05:08.000 +07:00 65b6b74e0897 - (empty) (no description set) - "###); - - // The presence of the @git branch means that a `jj git import` is a no-op... - let stdout = test_env.jj_cmd_success(&repo_path, &["git", "import"]); - insta::assert_snapshot!(stdout, @r###" - Nothing changed. - "###); - // ... and a `jj git export` will delete the branch from git and will delete the - // git-tracking branch. In a colocated repo, this will happen automatically - // immediately after a `jj branch forget`. This is demonstrated in - // `test_git_colocated_branch_forget` in test_git_colocated.rs - let stdout = test_env.jj_cmd_success(&repo_path, &["git", "export"]); - insta::assert_snapshot!(stdout, @""); - let stdout = test_env.jj_cmd_success(&repo_path, &["branch", "list"]); - insta::assert_snapshot!(stdout, @""); - - // Note that if `jj branch forget` *did* delete foo@git, a subsequent `jj - // git export` would be a no-op and a `jj git import` would resurrect - // the branch. In a normal repo, that might be OK. In a colocated repo, - // this would automatically happen before the next command, making `jj - // branch forget` useless. } #[test] @@ -333,10 +311,6 @@ fn test_branch_forget_fetched_branch() { // the ref in jj view's `git_refs` tracking the local git repo's remote-tracking // branch. // TODO: Show that jj git push is also a no-op - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "export"]), @""); - insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "import"]), @r###" - Nothing changed. - "###); insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @""); // We can fetch feature1 again. @@ -354,10 +328,10 @@ fn test_branch_forget_fetched_branch() { // Short-term TODO: Fix this BUG. It should be possible to fetch `feature1` // again. let stdout = test_env.jj_cmd_success(&repo_path, &["git", "fetch", "--remote=origin"]); - insta::assert_snapshot!(stdout, @r###" - Nothing changed. + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" + feature1: 9f01a0e04879 message "###); - insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @""); // TEST 3: fetch branch that was moved & forgotten @@ -377,13 +351,11 @@ fn test_branch_forget_fetched_branch() { Error: No such branch: feature1 "###); - // BUG: fetching a moved branch creates a move-deletion conflict + // Fetching a moved branch does not create a conflict let stdout = test_env.jj_cmd_success(&repo_path, &["git", "fetch", "--remote=origin"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" - feature1 (conflicted): - - 9f01a0e04879 message - + 38aefb173976 another message + feature1: 38aefb173976 another message "###); }