lib id_prefix: look for divergent changes outside short prefix set

Fixes #2476.

Previously, if there was a change id match within the short prefix
lookup set, `jj` would not look for commits with that same change id
outside the short prefix set. So, it wouldn't find the conflicted
commits for a commit with a divergent (AKA conflicted) change id.
This commit is contained in:
Ilya Grigoriev 2024-06-28 15:35:13 -07:00
parent 360a2b6a8b
commit 1b658ea80e
3 changed files with 21 additions and 11 deletions

View file

@ -82,6 +82,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
to-be-pushed commit has conflicts, or has no description / committer / author to-be-pushed commit has conflicts, or has no description / committer / author
set. [#3029](https://github.com/martinvonz/jj/issues/3029) set. [#3029](https://github.com/martinvonz/jj/issues/3029)
* `jj` will look for divergent changes outside the short prefix set even if it finds the change id inside the short prefix set. [#2476](https://github.com/martinvonz/jj/issues/2476)
## [0.18.0] - 2024-06-05 ## [0.18.0] - 2024-06-05
### Breaking changes ### Breaking changes

View file

@ -168,13 +168,14 @@ impl IdPrefixContext {
prefix: &HexPrefix, prefix: &HexPrefix,
) -> PrefixResolution<Vec<CommitId>> { ) -> PrefixResolution<Vec<CommitId>> {
if let Some(indexes) = self.disambiguation_indexes(repo) { if let Some(indexes) = self.disambiguation_indexes(repo) {
let resolution = indexes.change_index.resolve_prefix_with( let resolution = indexes
&*indexes.commit_change_ids, .change_index
prefix, .resolve_prefix_to_key(&*indexes.commit_change_ids, prefix);
|(commit_id, _)| commit_id.clone(), if let PrefixResolution::SingleMatch(change_id) = resolution {
); // There may be more commits with this change id outside the narrower sets.
if let PrefixResolution::SingleMatch((_, ids)) = resolution { return PrefixResolution::SingleMatch(repo.resolve_change_id(&change_id).expect(
return PrefixResolution::SingleMatch(ids); "Change ids present in narrower search set should be present globally.",
));
} }
} }
repo.resolve_change_id_prefix(prefix) repo.resolve_change_id_prefix(prefix)

View file

@ -365,12 +365,19 @@ fn test_id_prefix_divergent() {
c.shortest_change_prefix_len(repo.as_ref(), second_commit.change_id()), c.shortest_change_prefix_len(repo.as_ref(), second_commit.change_id()),
1 1
); );
// Short prefix does not find the first commit (as intended). // This tests two issues, both important:
// TODO(#2476): BUG: Looking up the divergent commits by their change id prefix // - We find both commits with the same change id, even though
// only finds the id within the lookup set. // `third_commit_divergent_with_second` is not in the short prefix set
// (#2476).
// - The short prefix set still works: we do *not* find the first commit and the
// match is not ambiguous, even though the first commit's change id would also
// match the prefix.
assert_eq!( assert_eq!(
c.resolve_change_prefix(repo.as_ref(), &prefix("a")), c.resolve_change_prefix(repo.as_ref(), &prefix("a")),
SingleMatch(vec![second_commit.id().clone()]) SingleMatch(vec![
second_commit.id().clone(),
third_commit_divergent_with_second.id().clone()
])
); );
// We can still resolve commits outside the set // We can still resolve commits outside the set