Indexer: Add indexer for bzlmod based on the BCR registry and user defined MODULE.bazel #140
Conversation
WojciechMazur
changed the title
WojciechMazur
force-pushed
the
indexer/03-bcr-indexer-go
branch
from
150772d to
6b200d0
Compare
WojciechMazur
force-pushed
the
indexer/04-bzlmod-indexer
branch
from
36aa550 to
18d182d
Compare
WojciechMazur
force-pushed
the
indexer/04-bzlmod-indexer
branch
from
18d182d to
029cd3d
Compare
…ULE.bazel Signed-off-by: Wojciech Mazur <wmazur@virtuslab.com>
Signed-off-by: Wojciech Mazur <wmazur@virtuslab.com>
WojciechMazur
force-pushed
the
indexer/04-bzlmod-indexer
branch
from
029cd3d to
7d3eacf
Compare
| Version string | ||
| } | ||
|
|
||
| func ExtractBazelDependencies(file build.File) []BazelDependency { |
There was a problem hiding this comment.
build.File may refer to any starlark file like BUILD, MODULE or custom.bzl. Please use moduleFile as variable name to emphasize the intention.
|
|
||
| func ExtractBazelDependencies(file build.File) []BazelDependency { | ||
| result := []BazelDependency{} | ||
| for _, stmt := range file.Stmt { |
There was a problem hiding this comment.
You could extract the loop body into
func parseBazelDependency(stmt build.Expr) BazelDependency, booland then use collections.FilterMapSlice here. So we avoid too many indentation levels.
| return resolvedModules | ||
| } | ||
|
|
||
| type BazelDependency struct { |
There was a problem hiding this comment.
There's no need to use capitalized identifiers - same for functions. Nothing can be exported from an executable. I recommend not doing this, so nobody thinks this is used in other packages.
| case "bazel_dep": | ||
| dep := BazelDependency{} | ||
| parseArg: | ||
| for idx, arg := range tree.List { |
There was a problem hiding this comment.
The same thing can be done here:
func parseBazelDependencyArgs(args []Expr) BazelDependency, boolBut it's up to you. I am not a fan of deep nesting. Separate functions provide documentation advantage - their names clarify the intention behind them.
| } | ||
| log.Printf("Would run in %v", callerRoot) | ||
|
|
||
| moduleBazelPath := *moduleBzl |
There was a problem hiding this comment.
These names are too similar. Please change them to something like absModuleBazelPath and moduleBazelPath (for the pure flag value, which is moduleBzl right now).
| } | ||
| } | ||
|
|
||
| workers := runtime.GOMAXPROCS(0) |
There was a problem hiding this comment.
BTW (I am not a Go concurrency master) - is it worth implementing a thread pool like this? Isn't Go concurrency "smart" enough to just run a separate goroutine for every BazelDependency? Or is there too much resource consumption for so many goroutines?
| for bazelDep := range bazelDeps { | ||
| result := bcrClient.ResolveModuleInfo(bazelDep.Name, bazelDep.Version) | ||
| if *cli.Verbose { | ||
| switch { |
There was a problem hiding this comment.
Simple if is more readable for a single true/false condition.
| for result := range resolveResults { | ||
| switch { | ||
| case result.IsResolved() && len(result.Info.Targets) > 0: | ||
| if len(result.Info.Targets) == 0 { |
There was a problem hiding this comment.
This condition is never true. You've already assumed len(result.Info.Targets) > 0 above.
Please add a non-existing repo to MODULE.bazel in the integration test to check this.
|
|
||
| var resolvedModules []indexer.Module | ||
| var emptyModules []string | ||
| var failed int |
There was a problem hiding this comment.
You could store module names var failed []string, similarly to emptyModules, for better verbosity.
You should be able to obtain the module name from ResolveModuleInfoResult.Unresolved.Module.Name.
Extracted from #92
Adds indexer powered by BCR as provider of target definitions, allowing to create indexes in the same way as the internal BCR indexer, but takes into account only projects found in MODULE.bazel ensuring to use correct version