Skip to content

Commit

Permalink
branch forget: force jj git export of relevant branches
Browse files Browse the repository at this point in the history 
This is a bit unwieldy; see a comment in the code for a detailed explanation.

We'll have to decide whether we prefer this, or we prefer the approach of
#1771, or we have a better idea.
  • Loading branch information
@ilyagr
ilyagr committed Jul 1, 2023
1 parent ba0d076 commit ee92b24
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 47 deletions.
53 changes: 50 additions & 3 deletions src/commands/branch.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use std::collections::BTreeSet;
use clap::builder::NonEmptyStringValueParser;
use itertools::Itertools;
use jujutsu_lib::backend::{CommitId, ObjectId};
use jujutsu_lib::git::git_tracking_branches;
use jujutsu_lib::git::{self, git_tracking_branches};
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::op_store::RefTarget;
use jujutsu_lib::repo::Repo;
use jujutsu_lib::revset;
Expand Down Expand Up @@ -67,11 +68,24 @@ pub struct BranchDeleteArgs {
#[derive(clap::Args, Clone, Debug)]
pub struct BranchListArgs;

/// 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.
Expand Down Expand Up @@ -289,6 +303,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::<GitBackend>()
.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}")));
Expand All @@ -301,6 +325,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())?;
Expand Down
56 changes: 12 additions & 44 deletions tests/test_branch_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -331,17 +309,9 @@ fn test_branch_forget_fetched_branch() {
// Technically, this is because `jj branch forget` preserved
// the ref in jj view's `git_refs` tracking the local git repo's remote-tracking
// branch.
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.
// BUG/Short-term TODO: it looks like the bug is fixed, but actually this only
// works because we did `jj git export` above and that deleted the
// remote-tracking branch in the git repo. Note that the BUG below is NOT
// fixed.
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###"
Expand All @@ -362,13 +332,11 @@ fn test_branch_forget_fetched_branch() {
let stdout = test_env.jj_cmd_success(&repo_path, &["branch", "forget", "feature1"]);
insta::assert_snapshot!(stdout, @"");

// 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
"###);
}

Expand Down

0 comments on commit ee92b24

Please sign in to comment.