Skip to content

Commit

Permalink
git: on import_refs(), don't abandon ancestors of newly fetched refs
Browse files Browse the repository at this point in the history
I made import_refs() not preserve commits referenced by remote branches at
520f692 "git: on import_refs(), don't preserve old branches referenced by
remote refs." The idea is that remote branches are weak, and commits referenced
by these refs can be freely rewritten by future local changes without moving
the refs. I don't think that's wrong, but 520f692 also made "new" remote
changes be abandoned by old remote refs. This problem occurs only when
git.auto-local-branch is off.

I think there are two ways to fix the problem:
 a. pin non-tracking remote branches just like local refs
 b. pin newly fetched refs in addition to local refs
This patch implements (b) because it's simpler and more obvious that the
fetched commits would never be abandoned immediately.
  • Loading branch information
yuja committed Oct 17, 2023
1 parent c3b45b6 commit 57167ce
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 1 deletion.
5 changes: 4 additions & 1 deletion lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,10 @@ pub fn import_some_refs(
};
return Ok(stats);
}
let pinned_heads = itertools::chain(
let pinned_heads = itertools::chain!(
changed_remote_refs
.values()
.flat_map(|(_, new_target)| new_target.added_ids()),
pinned_commit_ids(mut_repo.view()),
iter::once(mut_repo.store().root_commit_id()),
)
Expand Down
54 changes: 54 additions & 0 deletions lib/tests/test_git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,60 @@ fn test_import_refs_reimport_with_moved_remote_ref() {
assert_eq!(*view.heads(), expected_heads);
}

#[test]
fn test_import_refs_reimport_with_moved_untracked_remote_ref() {
let settings = testutils::user_settings();
let git_settings = GitSettings {
auto_local_branch: false,
};
let test_workspace = TestRepo::init_with_backend(TestRepoBackend::Git);
let repo = &test_workspace.repo;
let git_repo = get_git_repo(repo);

// The base commit doesn't have a reference.
let remote_ref_name = "refs/remotes/origin/feature";
let commit_base = empty_git_commit(&git_repo, remote_ref_name, &[]);
let commit_remote_t0 = empty_git_commit(&git_repo, remote_ref_name, &[&commit_base]);
let mut tx = repo.start_transaction(&settings, "test");
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
let repo = tx.commit();
let view = repo.view();

assert_eq!(*view.heads(), hashset! { jj_id(&commit_remote_t0) });
assert_eq!(view.local_branches().count(), 0);
assert_eq!(view.all_remote_branches().count(), 1);
assert_eq!(
view.get_remote_branch("feature", "origin"),
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_t0)),
state: RemoteRefState::New,
},
);

// Move the reference remotely and fetch the changes.
delete_git_ref(&git_repo, remote_ref_name);
let commit_remote_t1 = empty_git_commit(&git_repo, remote_ref_name, &[&commit_base]);
let mut tx = repo.start_transaction(&settings, "test");
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
let repo = tx.commit();
let view = repo.view();

// commit_remote_t0 should be abandoned, but commit_base shouldn't because
// it's the ancestor of commit_remote_t1.
assert_eq!(*view.heads(), hashset! { jj_id(&commit_remote_t1) });
assert_eq!(view.local_branches().count(), 0);
assert_eq!(view.all_remote_branches().count(), 1);
assert_eq!(
view.get_remote_branch("feature", "origin"),
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_t1)),
state: RemoteRefState::New,
},
);
}

#[test]
fn test_import_refs_reimport_git_head_with_fixed_ref() {
// Simulate external `git checkout` in colocated repo, from named branch.
Expand Down

0 comments on commit 57167ce

Please sign in to comment.