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

[7.4.0] Cherrypick --incompatible_autoload_externally implementation #23906

Conversation

comius
Copy link
Contributor

@comius comius commented Oct 9, 2024

Backporting #23043 so that users can migrate with Bazel 7.x

Fixes: #23905

Commits:
dbb9670
84c15a0
f9ed0b0
cc3ba22
7e13939
0f6d0d8
38239d9
3cb5bea
3bc5dea
7518bbb
963e7a2
ae02744
8d3bd0e (partial)

A later change will make packages.StarlarkProvider depend on
skyframe.BzlLoadValue which will be cyclic otherwise.

This requires splitting out packages:bzl_visibility,
packages:package_specification and skyframe:bzl_compile_value into their
own targets.
PiperOrigin-RevId: 625813461
Change-Id: If7a95be9e48a34afc9e3a47fa9232d672a7bbac9
@comius comius force-pushed the incompatible_autoload_externally_7_4_0 branch 2 times, most recently from 91c744d to f9062fc Compare October 9, 2024 06:35
aoeui and others added 12 commits October 9, 2024 08:45
PiperOrigin-RevId: 630371031
Change-Id: Ie598933f6a827a7fef596a6bc3ab126334547271
Issue: bazelbuild#22928
Incompatible flag issue: bazelbuild#23043

The flag supports Bazel users in migrating the rules from being embedded in Bazel to external repositories. Listing a symbol or a rule in the flag automatically adds a load to the respective repository. Listing it with a '+' keeps the symbol available in the rules_repository. Listing a symbol with "-" forcefully removes it from Bazel.

Cycles are prevented with a list of repositories where autoloads should not be used.

The new environments are injected into BzlCompile, BzlLoad and Package function.

StarlarkBuiltinsFunction with autoloads true key is used to load the external symbols.

Closes bazelbuild#23016.

Downstream test: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/4003

PiperOrigin-RevId: 666710259
Change-Id: Idaa9b6b13c9a474f700e69e22b6162ed59b7fab0
Listing 30 different symbols on the command line is not really users friendly.
Once we finish a rules set, we can set it to the whole rules repository.

PiperOrigin-RevId: 666776777
Change-Id: I6ddd8aec4daec26a427f962f6f76e496163d1108
They're not provided by rules_android.

PiperOrigin-RevId: 672607197
Change-Id: Ieb81a72c0d26b165f98d8b1f11c9bf2149f550d8
In the repositories that don't have autoloads we also expose native.legacy_symbols.
Those can be used to fallback to the native symbol, whenever it's still available in Bazel.

Fallback using a top-level symbol doesn't work, because BzlCompileFunction would throw an error when it's mentioned.

legacy_symbols aren't available when autoloads are not enabled.

The feature is intended to be used with bazel_features repository, which can correctly report native symbols on all versions of Bazel.

PiperOrigin-RevId: 673741927
Change-Id: I2334030d70cbb944b92784e32a184841ea238d51
We generally only expect people to be loading our top-level rules. At this stage all other providers, helpers etc are considered internal implementation details and we don't want to suggest otherwise. In the future this may change but for now just remove the autoloads.

PiperOrigin-RevId: 679677200
Change-Id: I50fd2dbddb0a59c12d4e5444cf1832b995bf370e
With legacy_symbols I made a typo in 2 location: protobuf repo and in bazel_feature. At this point it's easier to rename it in Bazel.

Tested manually on protobuf repository and this works with and without `--incompatible_autoload_externally=+@protobuf`

PiperOrigin-RevId: 679735770
Change-Id: I5879060404deab9656acc045da3a6fb37c1d1e5f
This changes from loading symbols as a dependency of bazel_tools to loading them as any dependency of any of the resolved modules.

In case the dependency is missing a warning is reported and Bazel only fails when the actual symbol is needed but not available.

PiperOrigin-RevId: 681347025
Change-Id: I4847a6a0420d2ae9e22d347dcd898ae9cdbdb0d6
PiperOrigin-RevId: 682997914
Change-Id: I30f5f9e752d1c91e16c838639790e90f3559b0f5
PiperOrigin-RevId: 682998564
Change-Id: I20316c27050c97a0040953b0fd5b163adf30851f
PiperOrigin-RevId: 683179562
Change-Id: I7e0885e33b65dc5e24be705a676108c2aa3c8e9a
Downstream build: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/4164
Downstream build shows that rules_android isn't quite yet ready, but I think it's better to enable the flag for Bazel 8 now, than to do an incompatible change later.

PiperOrigin-RevId: 683271234
Change-Id: I4b1a8c5fe087f8dbf0742440d6af3c436bbf26d2
@comius comius force-pushed the incompatible_autoload_externally_7_4_0 branch from f9062fc to ad69c6d Compare October 9, 2024 06:45
@comius comius marked this pull request as ready for review October 9, 2024 07:51
@comius comius requested a review from a team as a code owner October 9, 2024 07:51
@comius comius requested review from Wyverald and meteorcloudy and removed request for a team October 9, 2024 07:51
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Rules-Java Issues for Java rules labels Oct 9, 2024
@keertk keertk enabled auto-merge October 9, 2024 12:53
Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

Amazing!

@keertk keertk added this pull request to the merge queue Oct 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 9, 2024
@meteorcloudy meteorcloudy added this pull request to the merge queue Oct 9, 2024
@keertk keertk changed the title Cherrypick --incompatible_autoload_externally implementation [7.4.0] Cherrypick --incompatible_autoload_externally implementation Oct 9, 2024
Merged via the queue into bazelbuild:release-7.4.0 with commit b090c83 Oct 9, 2024
51 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Rules-Java Issues for Java rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants