Open
Conversation
bazel 5 went eol in jan 2025
Collaborator
Author
|
Note that this depends on #1474 so I've set the branch target to that. |
6522815 to
d9829f0
Compare
There was a problem hiding this comment.
Pull request overview
This PR completes the ninja rule’s toolchain/environment setup to more closely match make, and updates the “simple” examples to use a shared compiler/archive wrapper so they work consistently across platforms (including Windows).
Changes:
- Add a shared
create_ninja_script()implementation that reuses the existing make env-var/toolchain plumbing. - Update
foreign_cc/ninja.bzlto use toolchain-derived tools/flags/env vars when generating build commands. - Sync simple examples to use a shared
cc_wrapper.shrather than calling PATH binaries directly.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| foreign_cc/private/ninja_script.bzl | New script generator for Ninja builds using the existing make env-var/toolchain machinery. |
| foreign_cc/private/BUILD.bazel | Exposes ninja_script.bzl as a bzl_library. |
| foreign_cc/ninja.bzl | Switches Ninja script generation to use toolchain tools/flags and the new shared script builder. |
| foreign_cc/BUILD.bazel | Wires new bzl deps for the updated Ninja rule. |
| examples/cc_wrapper.sh | Adds a shared compiler/static-link wrapper for the simple examples. |
| examples/BUILD.bazel | Exports the shared cc_wrapper.sh from the //examples package. |
| examples/ninja_simple/** | Removes the old clang wrapper and updates Ninja example to use the shared wrapper. |
| examples/make_simple/** | Removes the old wrapper and updates Makefile + rule env to use the shared wrapper. |
| examples/meson_simple/** | Removes the now-unused clang wrapper wiring from the Meson example. |
Comments suppressed due to low confidence (1)
examples/make_simple/BUILD.bazel:19
- After removing cxx_wrapper.sh, the package //make_simple/code still exports that file (see examples/make_simple/code/BUILD.bazel). That will make this example fail to load. Remove the exports_files entry (or keep/provide the exported file).
lib_source = "//make_simple/code:srcs",
out_data_dirs = ["share"],
out_static_libs = ["liba.a"],
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ninja and make fit roughly into the same category of build system, but we were only setting up the C build variables for make. This finishes the ninja implementation to match make (more or less) and then syncs the simple examples to match. The simple examples that don't already have compiler integrations (i.e. make and ninja) now use the same wrapper, which loads data from the configured toolchain instead of calling PATH binaries.
d9829f0 to
8a86fdf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ninja and make fit roughly into the same category of build system, but
we were only setting up the C build variables for make. This finishes
the ninja implementation to match make (more or less) and then syncs the
simple examples to match. The simple examples that don't already have
compiler integrations (i.e. make and ninja) now use the same wrapper,
which loads data from the configured toolchain instead of calling PATH
binaries.
All of the simple examples now work on windows, too.