Skip to content

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Jun 4, 2024

This is a partial cherry-pick of 417c6b8 which excludes the changes to oneversion itself, which is built and released for Bazel 7 as part of java_tools:

  • Avoid passing --whitelist to one_version if no allowlist is configured in the toolchain as it isn't supported by the Bazel version of oneversion yet.
  • Document the one_version flags.
  • Clean up tests not updated after recent rules_java releases.

Fixes #22576

This is a partial cherry-pick of 417c6b8 which excludes the changes to `oneversion` itself, which is built and released for Bazel 7 as part of java_tools:

* Avoid passing --whitelist to one_version if no allowlist is configured in the toolchain as it isn't supported by the Bazel version of oneversion yet.
* Document the one_version flags.
* Clean up tests not updated after recent rules_java releases.
@fmeum fmeum requested a review from a team as a code owner June 4, 2024 21:17
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Rules-Java Issues for Java rules labels Jun 4, 2024
@fmeum fmeum requested a review from cushon June 4, 2024 21:18
@keertk keertk changed the title Make Bazel changes for oneversion support [7.2.0] Make Bazel changes for oneversion support Jun 4, 2024
@Wyverald
Copy link
Member

Wyverald commented Jun 4, 2024

@fmeum, how urgent is this change? And how would you characterize the risk for breakage if we include it? I'm asking because even though we're doing an rc3, I'd very much like to avoid any further delays on the release. Ideally we shouldn't be cherry-picking anything that's not an rc1/rc2 regression at this point.

@fmeum
Copy link
Collaborator Author

fmeum commented Jun 4, 2024

I fully understand your concerns, happy to provide the context for you to make a call:

  • It doesn't fix a regression.
  • It's the minimal change to Bazel necessary to allow for independent development and delivery of oneversion support via a future rules_java release.
  • It only touches code that so far hasn't been supported in Bazel (it actually didn't run at all unless you used an undocumented experimental flag), so the risk of this breaking anything is very low.

@meteorcloudy
Copy link
Member

Well, turned out we need 193b114 to make sure Bazel still builds in our trusted environment where there is an older macOS version.

@meteorcloudy
Copy link
Member

Oh, just realized this is a partial cherry-pick, which doesn't touch abseil-cpp version, then we don't need the fix on the main branch.

@meteorcloudy
Copy link
Member

I'm fine with backporting this one if it's low risk. In general, we can require such justification for all non-regression-fix PRs after the first rc and make decision case by case. @Wyverald @keertk Do you think that's a reasonable balance?

@keertk
Copy link
Member

keertk commented Jun 5, 2024

Sounds good! @cushon could you approve the PR please?

@keertk keertk enabled auto-merge June 5, 2024 17:40
@keertk keertk added this pull request to the merge queue Jun 5, 2024
Merged via the queue into bazelbuild:release-7.2.0 with commit b11d9c9 Jun 5, 2024
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Jun 5, 2024
@fmeum fmeum deleted the 22576 branch June 5, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Rules-Java Issues for Java rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants