Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autogazelle: Add option to pass -index=true to Gazelle #761

Closed
martinxsliu opened this issue Apr 16, 2020 · 2 comments
Closed

Autogazelle: Add option to pass -index=true to Gazelle #761

martinxsliu opened this issue Apr 16, 2020 · 2 comments

Comments

@martinxsliu
Copy link

What version of gazelle are you using?

v0.20.0

What version of rules_go are you using?

v0.22.2

What version of Bazel are you using?

v3.0.0-homebrew

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Mac OS X 10.14 Mojave

What did you do?

Use autogazelle with bazel.

What did you expect to see?

Autogazelle generates and updates build files in the same way as manually invoking Gazelle.

What did you see instead?

Autogazelle has a limitation in which it harcodes -index=false when invoking Gazelle. By not indexing, Gazelle does not know how to resolve vendored dependencies and Protobuf Go rules.

For us, we set the go_package option on all our Protobuf files to something that's not exactly the same as the protobuf package path. This means that Go packages that depend upon generated protobufs will have their rules' deps incorrectly changed from //protobuf/foo:go_default_library, which is generated by Gazelle with -index=true, to something like //go/pkg/path/foo:go_default_library where go/pkg/path/foo is the go_package option. Obviously //go/pkg/path/foo won't have a build file (and gazelle won't generate one) and so the build fails.

From the README, it sounds like the reason indexing is disabled is for performance. Anecdotally, I tried invoking Gazelle with vs without index, for both the entire workspace and for a smaller directory. In both cases, the wall clock difference between using index and without index was almost imperceptible. I wasn't very scientific about it, but there was perhaps a difference of ~100ms.

Given this, would you be opposed to adding a command line flag to Autogazelle to allow users to opt into running Gazelle with index set to true? I'm happy to submit a PR for this if there are no concerns.

@jayconrod
Copy link
Contributor

No, sorry, -index=false has to stay.

Autogazelle invokes Gazelle in individual directories that have changed at the moment a build command is issued. In order for that to be acceptably fast, Gazelle cannot index the whole repository on each of those invocations.

If you aren't seeing a performance difference here, it's either because the repository is small (in which case, you could just run Gazelle in the Bazel wrapper script instead of Autogazelle) or because something is buggy or broken.


Some context on Autogazelle: I built it a couple years ago as a demo, since Kubernetes was interested in something like that. It is acceptably fast on a repository the size of Kubernetes (still takes like 30 seconds to start though I think), but I wouldn't use it on something much bigger than that. It needs to watch for anything that could cause build file changes, so it can't scale to arbitrarily large repositories.

There's been very little interest and usage in Autogazelle since I first demoed it, so I'm happy to answer questions and take small bug fixes, but don't expect much active development here.

@eric-skydio
Copy link
Contributor

See #1181 / #1182

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants