diff --git a/lib/src/git.rs b/lib/src/git.rs index 13e2e3914bb..49f0cd56df5 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -507,6 +507,47 @@ pub fn fetch( callbacks: RemoteCallbacks<'_>, git_settings: &GitSettings, ) -> Result, GitFetchError> { + // In non-colocated repositories, it's possible that `jj branch forget` was run + // at some point and no `jj git export` happened since. + // + // This would mean that remote-tracking branches, forgotten in the jj repo, + // still exist in the git repo. If the branches didn't move on the remote, and + // we fetched them, jj would think that they are unmodified and wouldn't + // resurrect them. + // + // Export will delete the remote-tracking branches in the git repo, so it's + // possible to fetch them again. + // + // For more details, see the `test_branch_forget_fetched_branch` test, and PRs + // #1714 and #1771 + // + // Apart from `jj branch forget`, jj doesn't provide commands to manipulate + // remote-tracking branches, and local git branches don't affect fetch + // behaviors. So, it's unnecessary to export anything else. + // + // 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`. + let nonempty_branches = mut_repo + .base_repo() + .view() + .branches() + .iter() + .filter_map(|(branch, target)| target.local_target.as_ref().map(|_| branch.to_owned())) + .collect_vec(); + // TODO: Inform the user if the export failed? In most cases, export is not + // essential for fetch to work. + let _ = export_some_refs(mut_repo, git_repo, |ref_name| { + matches!( + ref_name, + RefName::RemoteBranch { branch, remote:_ } + if !nonempty_branches.contains(branch) + ) + }); + let mut remote = git_repo .find_remote(remote_name) diff --git a/tests/test_branch_command.rs b/tests/test_branch_command.rs index 5ba31288d36..4ba00c95fe6 100644 --- a/tests/test_branch_command.rs +++ b/tests/test_branch_command.rs @@ -238,9 +238,8 @@ fn test_branch_forget_export() { insta::assert_snapshot!(stdout, @""); 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 does not delete its local-git tracking branch. The + // git-tracking branch is kept. let stdout = test_env.jj_cmd_success(&repo_path, &["branch", "list"]); insta::assert_snapshot!(stdout, @r###" foo (forgotten) @@ -346,17 +345,14 @@ fn test_branch_forget_fetched_branch() { "###); // TEST 2: No export/import (otherwise the same as test 1) - // Short-term TODO: While it looks like the bug above is fixed, correct behavior - // now actually depends on export/import. test_env.jj_cmd_success(&repo_path, &["branch", "forget", "feature1"]); insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @""); - // Short-term TODO: Fix this BUG. It should be possible to fetch `feature1` - // again. + // Fetch works even without the export-import 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 @@ -371,18 +367,14 @@ fn test_branch_forget_fetched_branch() { &[&git_repo.find_commit(first_git_repo_commit).unwrap()], ) .unwrap(); - let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "forget", "feature1"]); - insta::assert_snapshot!(stderr, @r###" - Error: No such branch: feature1 - "###); + 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 "###); }