This repository was archived by the owner on Sep 30, 2024. It is now read-only.
[Backport 5.0] Structural Search: Modify revisions values to match on Zoekt branches#51419
Merged
Conversation
…#51076) Issue: https://github.com/sourcegraph/sourcegraph/issues/50506 The issue manifests for a structural search for any revisions specified in the query that have both of the following: 1) the revision is a commit and is the latest commit of an indexed branch in Zoekt 2) the revision is not the branch name indexed in Zoekt (i.e. it is not a revision specified in `experimentalFeatures.search.index.branches`) The following fix works for all cases where the indexed branch to query is `HEAD`. Therefore this will resolve the code insights use case since code insights repo filter does not accept revisions. The fix in this PR is intended to be something we can ship quickly and therefore I hoped to avoid modifying existing logic as much as possible. For this reason we keep the `SearchJob` arguments and logic unchanged but just modify the repo revisions passed in. Note that there is still a remaining issue where if we specify two revisions for our query and both of those revisions match both criteria described above, then we do not have enough information to determine which branch to use since we'll have two branches and neither of them have a map to their latest commit. To resolve this I believe we'd have to modify [`PartitionRepos()`](https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/search/zoekt/indexed_search.go?L177:6) and/or [`IndexedRepoRevs`](https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/search/zoekt/indexed_search.go?L38) which would be a bit more involved so I am planning to spin off to a separate ticket. ## Test plan <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> New method has test cases added (cherry picked from commit 633c37b)
unknwon
approved these changes
May 3, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue: https://github.com/sourcegraph/sourcegraph/issues/50506
The issue manifests for a structural search for any revisions specified in the query that have both of the following:
experimentalFeatures.search.index.branches)The following fix works for all cases where the indexed branch to query is
HEAD. Therefore this will resolve the code insights use case since code insights repo filter does not accept revisions.The fix in this PR is intended to be something we can ship quickly and therefore I hoped to avoid modifying existing logic as much as possible. For this reason we keep the
SearchJobarguments and logic unchanged but just modify the repo revisions passed in.Note that there is still a remaining issue where if we specify two revisions for our query and both of those revisions match both criteria described above, then we do not have enough information to determine which branch to use since we'll have two branches and neither of them have a map to their latest commit. To resolve this I believe we'd have to modify
PartitionRepos()and/orIndexedRepoRevswhich would be a bit more involved so I am planning to spin off to a separate ticket.Test plan
<!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles -->
New method has test cases added
Backport 633c37b from #51076