Skip to content

Add compiler source download integrity values #1749

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mbland
Copy link
Contributor

@mbland mbland commented Jun 15, 2025

Description

Eliminates the "canonical reproducible form" warnings that often appear for users that enable compiler dependency tracking. Fixes #1743.

Emits a detailed error message if the configured Scala version is unknown to rules_scala, and isn't provided by a
scala_deps.compiler_srcjar object. This message provides guidance on how to create a suitable scala_deps.compiler_srcjar object.

Details:

  • Adds scripts/update_compiler_sources_integrity.py to generate scala/private/macros/compiler_sources_integrity.bzl. Refactored code from scripts/update_protoc_integrity.py into scripts/lib/update_integrity.py for reuse by the new script.

  • Adds test/shell/test_compiler_sources_integrity.sh and adds it to test_rules_scala.sh. Adds the test/compiler_sources_integrity repo for use by the new test suite. The test suite is fast due to the test repo only containing an alias to @scala_compiler_sources//:src.

  • Fixes a problem with the regex in run_tests from test_helper.sh that prevented the test skipping mechanism from working.

Motivation

This eliminates an annoying warning for users, while providing improved build integrity. Win/win!

The refactoring of common code into scripts/lib eliminated a lot of potential duplication. It's also exposed a common pattern for generating download integrity files for different artifacts.

Eliminates the "canonical reproducible form" warnings that often appear
for users that enable compiler dependency tracking. Fixes bazel-contrib#1743.

Emits a detailed error message if the configured Scala version is
unknown to rules_scala, and isn't provided by a
`scala_deps.compiler_srcjar` object. This message provides guidance
on how to create a suitable `scala_deps.compiler_srcjar` object.

Details:

- Adds `scripts/update_compiler_sources_integrity.py` to generate
  `scala/private/macros/compiler_sources_integrity.bzl`. Refactored code
  from `scripts/update_protoc_integrity.py` into
  `scripts/lib/update_integrity.py` for reuse by the new script.

- Adds `test/shell/test_compiler_sources_integrity.sh` and adds it to
  `test_rules_scala.sh`. Adds the `test/compiler_sources_integrity` repo
  for use by the new test suite. The test suite is fast due to the test
  repo only containing an alias to `@scala_compiler_sources//:src`.

- Fixes a problem with the regex in `run_tests` from `test_helper.sh`
  that prevented the test skipping mechanism from working.

---

This eliminates an annoying warning for users, while providing improved
build integrity. Win/win!

The refactoring of common code into `scripts/lib` eliminated a lot of
potential duplication. It's also exposed a common pattern for generating
download integrity files for different artifacts.
@mbland mbland requested review from liucijus and simuons as code owners June 15, 2025 22:09
Fixes breakages in build:

- https://buildkite.com/bazel/rules-scala-scala/builds/5652

The lint fix is for `test/compiler_sources_integrity/BUILD`.

The unbound variable error on macOS was in
`test/shell/test_compiler_sources_integrity.sh`, even though the
variable should've been bound. The script sets `-euo pipefail` anyway.

Took the missing config setup from `test/shell/test_bzlmod_macros.sh`
and added it to `test/shell/test_compiler_sources_integrity.sh`.
@mbland
Copy link
Contributor Author

mbland commented Jun 15, 2025

@simuons @liucijus As always, I'm happy to split this into separate pull requests, or at least separate commits, if you'd prefer.

Either way, the recommended breakdown would be to review the parts in this order:

  • The refactoring of parts of scripts/update_protoc_integrity.py into scripts/lib/update_integrity.py
  • The new scripts/update_compiler_sources_integrity.py and scala/private/macros/compiler_sources_integrity.bzl files
  • The integration of scala/private/macros/compiler_sources_integrity.bzl into scala/private/macros/scala_repositories.bzl to fix the problem, and the fail() message providing guidance if an integrity value is missing
  • The new test/shell/test_compiler_sources_integrity.sh test suite and test/compiler_sources_integrity test repo

Also, @WojciechMazur, if this goes through, scripts/update_compiler_sources_integrity.py would be another place to update for Scala version bumps. Open to any suggestions for improvements in this regard. (I couldn't bring myself to write out all those version strings by hand, but I could update it to do so if you'd strongly prefer that.)

The build failed again with an unbound variable error on macOS:

- https://buildkite.com/bazel/rules-scala-scala/builds/5653#019775ba-c5ff-4bff-b877-4bb22d22ec9b

```
running test test_emit_no_canonical_reproducible_form_warning_for_default_version

Log: .../test/shell/test_compiler_sources_integrity.sh: line 85:
  build_args[@]: unbound variable

 build failed
```

Today I learned that older versions of Bash consider empty arrays unset,
hence the application of `${build_args[@]+"${build_args[@]}"}`:

- https://stackoverflow.com/a/7577209
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.

[Bzlmod] Using ast-plus toolchain leads to non reproducibility warning
1 participant