Skip to content

Fixing release workflow #1736

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

Merged
merged 1 commit into from
May 9, 2025

Conversation

simuons
Copy link
Collaborator

@simuons simuons commented May 9, 2025

Motivation

Release attempt failed https://github.com/bazel-contrib/rules_scala/actions/runs/14926599712

release_prep_command was removed in bazel-contrib/.github#32

workspace_snippet.sh was renabed to release_prep.sh this is a hardcoded path that release workflow expects

Release attempt failed https://github.com/bazel-contrib/rules_scala/actions/runs/14926599712

release_prep_command was removed in bazel-contrib/.github#32

workspace_snippet.sh was renabed to release_prep.sh
this is a hardcoded path that release workflow expects
Copy link
Contributor

@mbland mbland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, FWIW. Sorry I missed this earlier in #1731.

@alexeagle alexeagle merged commit 999fa48 into bazel-contrib:master May 9, 2025
1 check passed
@simuons
Copy link
Collaborator Author

simuons commented May 9, 2025

@mbland no worries, I missed that too. I'm grateful for all the work you did!

mbland added a commit to mbland/rules_scala that referenced this pull request May 17, 2025
Explicitly loads all symbols affected by
`--incompatible_disable_autoloads_in_main_repo` to fix `last_green`
Bazel builds. Adds `rules_shell` as a dev dependency and updates files
to fix `test_bzlmod_macros.sh` and `test_dependency_versions.sh`.

As a result of using Buildifier to add the missing `load` statements,
Buildifier reordered the `load` statements as well.

Fixes the following breakage when using the `last_green` build from
bazelbuild/bazel@f08d561 (and other
breakages blocked by this one):

```sh
$ bazel run //tools:lint_check

ERROR: scala/private/rules/scala_test.bzl:130:21:
  name 'JavaInfo' is not defined

ERROR: .../external/+dev_deps+com_github_bazelbuild_buildtools/buildifier/BUILD.bazel:3:10:
  While resolving toolchains for target
  @@+dev_deps+com_github_bazelbuild_buildtools//buildifier:buildifier
  (b5bbebe): invalid registered toolchain
  '//test/proto:scalapb_toolchain':
  error loading package 'test/proto':
  at /Users/mbland/src/bazel-contrib/rules_scala/scala/scala.bzl:35:5:
  compilation of module 'scala/private/rules/scala_test.bzl' failed

ERROR: Analysis of target '//tools:lint_check' failed; build aborted:
  Analysis failed
```

Fixed using Buildifier 8.2.0 by first running the following to get the
list of appropriate warnings:

```txt
buildifier --lint=warn -r . 2>&1 | grep -- native- |
  awk '{print $2}' | sort | uniq
```

then running:

```txt
buildifier --lint=fix \
  --warnings=native-java-common,native-java-info,native-sh-test,native-sh-binary \
  -r .
```

I also ran it separately on the changed files in `deps/test` and
`scala/private/macros/test`, since Buildifier didn't automatically find
them.

---

bazelbuild/bazel@d87eaf5 from
2025-05-08 at 8:03am EDT flipped
`--incompatible_disable_autoloads_in_main_repo` to `true` to prepare for
Bazel 9. I did confirm that setting
`--noincompatible_disable_autoloads_in_main_repo` resolved the breakage
before using Buildifier to add the necessary `load` statements.

Pull request bazel-contrib#1735 was the last to pass the `last_green` CI build using
bazelbuild/bazel@94e0b44 from
2025-05-08 at 7:39am EDT. This was two commits before the commit that
flipped `--incompatible_disable_autoloads_in_main_repo`:

- https://buildkite.com/bazel/rules-scala-scala/builds/5596#0196afc2-4af3-4202-8232-d5f5fe113349

Pull request bazel-contrib#1736 failed the `last_green` CI build using
bazelbuild/bazel@cac54c4 from
2025-05-09 at 6:14am EDT. That job ran on 2025-05-09 at 6:47am EDT. The
`last_green` configuration in `.bazelci/presubmit.yml` emits a warning
if the build fails. This warning was present on the build page, but
we've overlooked it until now:

- https://buildkite.com/bazel/rules-scala-scala/builds/5598#0196b4a8-e593-431a-ab4c-8d8471d9b08b

Part of this change copies more files from `test/jmh` into the
`test_dependency_versions.sh` test directory. It also replicates targets
from `//test/jmh` in `deps/test/BUILD.bazel.test` to replace
the previous dependency on `@rules_scala//test/jmh:add_numbers`.

This is because `//test/jmh` now has a `load` dependency on
`@rules_shell`, which is a `dev_dependency` of `@rules_scala`. Since
`@rules_shell` isn't visible to `@rules_scala` when it isn't the root
module, this broke the build within `test_dependency_versions.sh`.
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.

3 participants