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

Add gazelle plugin to CI and distribution mechanism #424

Merged
merged 10 commits into from
Jan 20, 2023

Conversation

tetromino
Copy link
Collaborator

@tetromino tetromino commented Jan 17, 2023

After #400, the gazelle plugin has been cleanly separated out into its own bazel workspace, which will soon finally allow us to mark it stable. But this means:

  • we need to change our bazelci config to explicitly build and test it, since bazel build //... no longer includes the plugin;
  • we need to add proper distribution rules for it;
  • we need to update release instructions, since now we will have two distribution tarballs

We now have 3 versions: the tarball version (version.bzl), the skylib
module version (MODULE.bazel), and the gazelle plugin module version
(gazelle/MODULE.bazel)
@tetromino
Copy link
Collaborator Author

Note: as a followup, I will try to see if we can clean up distro BUILD files to not include distro-only targets. I think this is feasible by updating to a newer version of rules_pkg, but the change is big enough that it would be best done as a separate followup PR.

@achew22
Copy link
Member

achew22 commented Jan 17, 2023

This LGTM. Obviously gotta fixup CI but conceptually this seems like the right direction. Thanks @tetromino!

Needed to allow building/testing gazelle plugin from bazelci and
generating release tarballs when in bzlmod mode.

Note that the local_path_override will be stripped out in the
tarball release, so users of stable skylib releases will not be
affected.

Users of a live/unreleased version of skylib who depend on it via
git_override will need to add a patch to remove or comment out
the local_path_override. I expect the number of such users to be
tiny, but I have added a comment in MODULE.bazel to guide them.
Needed to cleanly strip ./external/bazel_skylib_gazelle_plugin~* dir
prefixes in the tarball.

Also allows us to clean up fixed target name vs. versioned tarball
file name without needing aliases.

Note that we are using 0.7.0 since 0.8.0 is not yet in BCR.
Provide a patch file that users can use in their git_override
@tetromino tetromino merged commit 99a6bcb into bazelbuild:main Jan 20, 2023
@tetromino tetromino deleted the gazelle-fixes branch January 20, 2023 21:48
@alexeagle
Copy link
Contributor

This module also needs to be added to the Bazel Central Registry in order to use it - @tetromino is that on your list? Want a PR for it?

@tetromino
Copy link
Collaborator Author

@alexeagle - yes, it's my next todo; I just need to generate the tarball

@alexeagle
Copy link
Contributor

I guess it's a wider discussion, but I think we could chat about distribution of the entire repo instead, it's tremendously simpler.

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

Successfully merging this pull request may close these issues.

4 participants