bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
394 stars 180 forks source link

Move gazelle plugin into a new git repo #423

Closed tetromino closed 1 year ago

tetromino commented 1 year ago

After #400, I don't understand why the gazelle plugin belongs in this git repo:

Originally posted by @tetromino in https://github.com/bazelbuild/bazel-skylib/issues/410#issuecomment-1385955577

tetromino commented 1 year ago

Potentially interested parties: @Wyverald @brandjon @shs96c @achew22 @comius

achew22 commented 1 year ago

While only speaking for myself here, I think that having the plugin live in the same place as the rules is ultimately the way to do things for most, if not all, cases.

If everything is in one place, rules authors are able to simultaneously make changes to their gazelle plugin and their rules set in the same commit. Git tags are applied at consistent points so there is no risk of drift. This is how it's done for rules_python and they seem very happy with the configuration thusfar.

At this instant bazel {build,test} //... doesn't include the gazelle plugin, but I would advocate that is incorrect. I don't know enough about bzlmod, but it feels like that should be correctable, am I mistaken?

fmeum commented 1 year ago

I agree with @achew22 here, having the canonical Gazelle plugin right next to the ruleset code seems right to me.

At this instant bazel {build,test} //... doesn't include the gazelle plugin, but I would advocate that is incorrect. I don't know enough about bzlmod, but it feels like that should be correctable, am I mistaken?

This could be achieved by using the integration test helpers used in rules_python.

tetromino commented 1 year ago

This could be achieved by using the integration test helpers used in rules_python.

@fmeum, could you point me to those helpers? As far as I can see, rules_python has its rules and gazelle plugin in one workspace.

fmeum commented 1 year ago

https://github.com/bazelbuild/rules_python/blob/main/tools/bazel_integration_test/bazel_integration_test.bzl is the trick I was referring to. But a separate CI job with working_directory set to a subdirectory also should make for a decent dev experience.

shs96c commented 1 year ago

For context, one reason why the plugin is in its own module is that it requires the use of rules_go, and that's an expensive dependency to take since it downloads an entire Go toolchain when the module is initialised. Another may be that this prevents some nasty circular dependencies (skylib is pretty foundational), but that wasn't the motivation for creating a new module.

To go on the record, I agree with @achew22's comment.

fmeum commented 1 year ago

The issue with rules_go eagerly loading toolchains has now been fixed. Depending on rules_go should now be very cheap, but still isn't right for something as foundational as bazel_skylib.

tetromino commented 1 year ago

Thank you for your comments - you have convinced me that splitting the plugin into a new repo is not the way to go. And we figured out a way to reasonably distribute both modules from the same git repo (#424, #429).