zoekt-indexserver: Per-Entry Branch Config#1011
zoekt-indexserver: Per-Entry Branch Config#1011dharesign wants to merge 1 commit intosourcegraph:mainfrom
Conversation
Allow specifying which branches to index per config entry via the Branches field (comma-separated, e.g. "main,release-*", default HEAD) and BranchPrefix (e.g. "refs/tags/" to index tags instead of branches). These values are persisted as zoekt.branches-to-index and zoekt.branch-prefix in each repo's git config by the mirror commands, then read at index time and passed to zoekt-git-index as -branches and -prefix flags. Fixes sourcegraph#432.
keegancsmith
left a comment
keegancsmith
left a comment
There was a problem hiding this comment.
Requesting changes, but not sure if my comments are feasible.
Not the biggest fan of needing to update so many mirror commands for this, but currently this is the most obvious way to do this. I don't think we have a nice way to map indexserver config <-> cloned repo.
Maybe a more general field getting into the zoekt config for just zoekt-indexserver to read would make this code a bit cleaner / easier to add future features like this to zoekt-indexserver?
| // Read zoekt.branches-to-index from the repo's git config. If set, pass | ||
| // it to zoekt-git-index as -branches along with -allow_missing_branches. | ||
| // The value is a comma-separated list of branch names (default: HEAD). | ||
| if branchOut, err := exec.Command("git", "--git-dir", dir, "config", "zoekt.branches-to-index").Output(); err == nil { | ||
| if branches := strings.TrimSpace(string(branchOut)); branches != "" { | ||
| args = append(args, "-branches", branches, "-allow_missing_branches") | ||
| } | ||
| } | ||
| if prefixOut, err := exec.Command("git", "--git-dir", dir, "config", "zoekt.branch-prefix").Output(); err == nil { | ||
| if prefix := strings.TrimSpace(string(prefixOut)); prefix != "" { | ||
| args = append(args, "-prefix", prefix) | ||
| } | ||
| } |
There was a problem hiding this comment.
this feels like it violates a layer we had before. Only zoekt-git-index would read the git config, not indexserver. Would it be possible to instead read these values in in the git indexer, and if they are set override branches-to-index or branch-prefix
|
How about modifying the |
This is easy enough to do initially, however once the repos have been cloned the Additionally, it looks like if you remove items from |
|
https://github.com/sourcegraph/zoekt/compare/main...dharesign:zoekt:indexserver-branch-support-take2?expand=1 shows how the code would look for the initial clone. TBD how to handle |
|
@dharesign Yeah I see how the current architecture causes issues now. It's quite nice that periodicFetch can just rely on git state. Unclear what a good architecture is here now. My initial thought is this, what do you think: zoekt-git-index decides on which branches to index in this order. The first one it finds it uses, no merging/etc.
Then it does make sense for indexserver to persist this state. Finally, git already has the concept of branch specific config. I wonder if we should instead mark each branch we should index that way? That might be complicating things too much though. |
2 participants
Allow specifying which branches to index per config entry via the Branches field (comma-separated, e.g. "main,release-*", default HEAD) and BranchPrefix (e.g. "refs/tags/" to index tags instead of branches).
These values are persisted as zoekt.branches-to-index and zoekt.branch-prefix in each repo's git config by the mirror commands, then read at index time and passed to zoekt-git-index as -branches and -prefix flags.
Fixes #432.