Skip to content

Replace --define=EXECUTOR=remote with toolchains and selects #26019

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

Closed
wants to merge 9 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented May 7, 2025

This allows for cross-platform builds to work seamlessly without a need to manually toggle between prebuilt and from source toolchains.

As a positive side effect, this change also paves the way for dropping a dependency on an exec-configured C++ toolchain of Java and Python targets built for non-Windows platforms. Rulesets will need to migrate to using the launcher maker toolchain instead of the launcher_maker target. rules_shell wasn't affected by this as it already defined its own launcher toolchain.

Work towards #19587

@fmeum fmeum changed the title Replace --define=EXECUTOR=remote with toolchains and select Replace --define=EXECUTOR=remote with toolchains and selects May 7, 2025
@fmeum fmeum force-pushed the 19587-launcher-toolchain branch 3 times, most recently from dcff9af to fed82d4 Compare May 7, 2025 16:23
@fmeum fmeum marked this pull request as ready for review May 7, 2025 17:42
@fmeum fmeum requested review from gregestren and fweikert as code owners May 7, 2025 17:42
@fmeum fmeum requested a review from meteorcloudy May 7, 2025 17:42
@github-actions github-actions bot added team-Documentation Documentation improvements that cannot be directly linked to other team labels awaiting-review PR is awaiting review from an assigned reviewer labels May 7, 2025
@fmeum fmeum removed request for gregestren and fweikert May 7, 2025 17:42
@fmeum fmeum added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label May 7, 2025
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.

This is very smart, thanks!

/cc @pzembrod to take a look since this is affects C++ rules.

@meteorcloudy meteorcloudy requested a review from pzembrod May 8, 2025 09:43
@fmeum
Copy link
Collaborator Author

fmeum commented May 8, 2025

@bazel-io fork 8.3.0

@fmeum
Copy link
Collaborator Author

fmeum commented May 9, 2025

The way launcher_maker is handled isn't quite correct. It probably needs to be depended on as a toolchain. I'll set this to draft for now.

@fmeum fmeum force-pushed the 19587-launcher-toolchain branch from 395a5e9 to c682fca Compare May 12, 2025 10:48
@fmeum fmeum requested a review from meteorcloudy May 12, 2025 10:48
@fmeum fmeum marked this pull request as ready for review May 12, 2025 10:48
@fmeum
Copy link
Collaborator Author

fmeum commented May 12, 2025

@meteorcloudy I switched to a toolchain approach that rules_java, rules_python and rules_shell will be able to adopt via bazel_features. I hope that this can be cherry-picked into 8.3.0 to speed up adoption.

@fmeum fmeum force-pushed the 19587-launcher-toolchain branch from c682fca to 43947a5 Compare May 12, 2025 12:13
@meteorcloudy
Copy link
Member

Nice, is it possible to write a test to verify that with this approach, C++ toolchain dependency won't be propagated by depending on the launcher?

@fmeum
Copy link
Collaborator Author

fmeum commented May 12, 2025

I would like to add such a test (as a Starlark analysis test) to the individual rulesets that adopt the toolchain, where it fits better semantically by more generally verifying that cross-compilation works without a C++ toolchain.

@meteorcloudy meteorcloudy added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels May 13, 2025
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label May 14, 2025
hvadehra pushed a commit that referenced this pull request May 28, 2025
This allows for cross-platform builds to work seamlessly without a need to manually toggle between prebuilt and from source toolchains.

As a positive side effect, this change also paves the way for dropping a dependency on an exec-configured C++ toolchain of Java and Python targets built for non-Windows platforms. Rulesets will need to migrate to using the launcher maker toolchain instead of the `launcher_maker` target. rules_shell wasn't affected by this as it already defined its own launcher toolchain.

Work towards #19587

Closes #26019.

PiperOrigin-RevId: 758697189
Change-Id: Ibfa272390056963ef35301614520fd38d018210e

(cherry picked from commit e819495)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Documentation Documentation improvements that cannot be directly linked to other team labels team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants