Skip to content

Commit

Permalink
git fetch: do a git export of deleted branches before fetch
Browse files Browse the repository at this point in the history
  • Loading branch information
ilyagr committed Jul 1, 2023
1 parent 4b2958d commit 7ef607c
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 18 deletions.
41 changes: 41 additions & 0 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,47 @@ pub fn fetch(
callbacks: RemoteCallbacks<'_>,
git_settings: &GitSettings,
) -> Result<Option<String>, 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)
Expand Down
28 changes: 10 additions & 18 deletions tests/test_branch_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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
"###);
}

Expand Down

0 comments on commit 7ef607c

Please sign in to comment.