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

Prepare for bzlmod #73

Merged
merged 11 commits into from
Jul 10, 2024
Merged

Prepare for bzlmod #73

merged 11 commits into from
Jul 10, 2024

Conversation

manuelnaranjo
Copy link
Contributor

@manuelnaranjo manuelnaranjo commented Jun 14, 2024

This PR will be an enabler so we can adopt bzlmod, but in order to simplify the process let's first
do a bazel upgrade.

Changes:

  • adding e2e tests for all the possible conditions we support: 5, 6 and 7 all without bzlmod
  • adoption toolchain support for bazeldnf
  • refactor the external API to the tools to reduce the external dependencies required to consume from us
  • build the binary with bazel 6

Adding CI that can build into multiple bazel versions, right now focusing
on the latest of each of the LTS version
Making 6.5.0 the default bazel version
Our tests where using the old and deprecated bundled pkg_tar rule, now
we need to grab it from rules_pkg instead.

Adding the results of running buildifier over the files.
Turns out there's a prebuilt buildifier that speeds up things a lot by
not having to rebuild the binary on each checkout of the repository
We all know the joke of Bazel just got it's 1000+1 flag each time a new
feature gets released, the aspect.dev folks have condensed a few flags
that speed up things and make sure bazel is used in the right way, so
let's start adopting it.
bumping skylib so we can use the platforms repository properly now
Following conventions from other rules so that there's a directory that
can be entirely built from other workspaces with
`bazel build @bazeldnf//bazeldnf/...`. It should be self contained and
inject as many dependencies into the parent workspace as needed.
By providing a proper toolchain instead of an alias we remove the
dependency on rules_go when someone uses bazeldnf externally, but also
works better for cross compilation and other kind of processes
Now we provide an e2e test for bazel5 with no bzlmod, it shows the
minimal setup to consume from bazeldnf rules
make sure bazel6 repos can consume bazeldnf when bzlmod is off
adding one more e2e test for the rule
@manuelnaranjo manuelnaranjo marked this pull request as draft June 16, 2024 08:18
@manuelnaranjo manuelnaranjo changed the title WIP: bump to bazel6 Prepare for bzlmod Jun 16, 2024
@manuelnaranjo manuelnaranjo marked this pull request as ready for review June 16, 2024 08:26
@manuelnaranjo
Copy link
Contributor Author

manuelnaranjo commented Jun 16, 2024

@rmohr this is ready for review, I'm splitting my bzlmod migration into multiple steps, this is the first step, refactor the public API, adopt a few good practices from other rules from the rules template, make sure we have e2e test for multiple bazel versions that would consume from bazeldnf and adoption of toolchains (reusing the pre-built binaries or doing built if requested).

The next step is for bazel 6 and 7 to add bzlmod, we will start with simple module extension reusing the current rpm rules and consuming rpm dependencies from WORKSPACE file (to avoid getting blocked by MODULE.bazel update process) and then on a follow up state figure out the API for MODULE.bazel

@manuelnaranjo
Copy link
Contributor Author

FYI this is what the bzlmod with toolchains support starts looking like: https://github.com/rmohr/bazeldnf/compare/main...bookingcom:bazeldnf:mnaranjo/bzlmod-toolchain?expand=1

@manuelnaranjo
Copy link
Contributor Author

FYI this is what the bzlmod with toolchains support starts looking like: https://github.com/rmohr/bazeldnf/compare/main...bookingcom:bazeldnf:mnaranjo/bzlmod-toolchain?expand=1

For simplicity I'm starting to consider we need to split the repo in 2 bazel repos, the first one is either prebuilt or build on demand bazeldnf binary (TBH maybe if we move to prebuilt protobuf it may be way faster to build than right now, but that only works with bazel7) and then another one that exposes the bzl rules, I'm having some trouble when consuming the bzlmod setup and I want to redirect the preloads to the upstream repo or to another repo

@rmohr
Copy link
Owner

rmohr commented Jul 10, 2024

I really don't like having two repos. But I love this PR! LGTM.

@rmohr rmohr merged commit 6e8f82f into rmohr:main Jul 10, 2024
4 checks passed
@rmohr
Copy link
Owner

rmohr commented Jul 10, 2024

I really don't like having two repos.

But if necessary we can do it, but I think I need a little bit more background on the issues you are facing.

@manuelnaranjo
Copy link
Contributor Author

I'm not saying 2 git repos, but 2 Bazel repos, so 2 separate MODULE.bazel, the biggest challenge is actually the release process where we need to build the binaries and then have a file that gives us the sha per release. I worked in another rules after doing this and I got it to work just fine, need to check how it's different it may not be needed to split.
The template for prebuilt binaries is made for cases like buildifier or some other external tool, where one repo provides the standalone binary and another one gives rules to consume those. While in this case both the rules and the binary are tight together.
Anyhow, let me see how we take it from here and I come with another PR

@rmohr
Copy link
Owner

rmohr commented Jul 11, 2024

Sounds good! Thanks for doing that!

What you describe sounds pretty similiar what the release process is doing today: Once a release is tagged, the binaries get built and then updated references are pushed to the branch directly.

@manuelnaranjo
Copy link
Contributor Author

manuelnaranjo commented Jul 11, 2024

So what others are doing is they keep a reference in the code to some valid state, so when using it in dev mode it works, and then during the release process after the binaries got built they push the new values into that file, but the file never gets checked in, which makes the repo a bit simpler to follow IMO.

I would say the biggest advantage of that approach is that you don't need github actions to be able to write into the repo, maybe somehow reducing how a public repo CI can be exploited?

@rmohr
Copy link
Owner

rmohr commented Jul 11, 2024

Any approach which allows us to stay within one git repo is fine by me. If we don't have to write from an action to the repo, even better :)

@rmohr rmohr mentioned this pull request Jul 11, 2024
5 tasks
@manuelnaranjo manuelnaranjo deleted the mnaranjo/bazel-6 branch July 18, 2024 02:53
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.

2 participants