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

[Bug]: //tools:add_module should default to MODULE.bazel in the source tree #2781

Open
davidben opened this issue Sep 14, 2024 · 0 comments
Open
Labels
bug Something isn't working

Comments

@davidben
Copy link
Contributor

What happened?

When using the //tools:add_module tool in the bazel-central-registry, it asks if I would like to supply a MODULE.bazel file. Since the project I work on is trying to be bzlmod-aware, I would not need to supply one, since the repository has the correct one.

However, leaving that blank seems to cause the tool to synthesize one, rather than picking up the one that already exists. It then complains the synthesized one does not match the one in the archive, so the tool seems able to extract it. It just does not use it for some reason.

Writing our tooling around this (unfortunately necessary because the recommended automation doesn't meet security requirements; see bazel-contrib/publish-to-bcr#157) would be simpler if the script could just use the existing one when available.

Version

n/a, issue is in BCR tooling, not Bazel

How to reproduce

Run `//tools:add_module` and do not supply a `MODULE.bazel` file, when the archive already has one.

Any other information?

No response

@davidben davidben added the bug Something isn't working label Sep 14, 2024
davidben added a commit to google/boringssl that referenced this issue Sep 16, 2024
I had hoped to use Publish to BCR, but there are a few issues with it.

First, a security issue: Publish to BCR requires granting a third-party
app write access to the GitHub repository, even though it only reads
from the repository, which requires no special privileges to read a
repository: bazel-contrib/publish-to-bcr#157

Second, merely cutting a release is not sufficient to satisfy
https://blog.bazel.build/2023/02/15/github-archive-checksum.html
One needs to manually upload a release tarball that GitHub then stores
explicitly. (Perhaps someone should define a deterministic tarball
creation process for git revisions and end this silliness.) Since that
tarball is added by an individual developer, it seems poor that nothing
checks it against the git repository.

The BCR repository itself has some tooling for making a release. It
works by interactively asking questions (not automatable), but then
saves an undocumented JSON file with the answers. I've written a script
that generates the JSON file we need from a git tag. These JSON files
need to reference file paths, so they cannot be made standalone. (See
bazelbuild/bazel-central-registry#2781)
Instead, the script drops everything into a temporary directory.

Since BCR's limitations force us to do a lot of custom processing
anyway, I made the script check that:

1. The release tarball matches the archive tarball, which are stable
   enough in practice. This allows anyone to perform an easy
   (still GitHub-dependent) check that they match, unless GitHub
   changes the hash.

2. The tarball's contents match the git tag in the local repository, so
   we verify GitHub against the developer's workstation.

The script then prints a command to run in a local fork of the
bazel-central-registry repository to make a PR. Alas, even downloading
the tarball from GitHub takes a few seconds, so I had a bit of fun with
the script output.

Change-Id: I2a748309f63848ff097ee3c3e93e11751ef65cd7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71307
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant