build_support: add g++ cross-compilation wrappers for musl targets#3208
Open
benhillis wants to merge 2 commits intomicrosoft:mainfrom
Open
build_support: add g++ cross-compilation wrappers for musl targets#3208benhillis wants to merge 2 commits intomicrosoft:mainfrom
benhillis wants to merge 2 commits intomicrosoft:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds C++-aware cross-compilation support for Underhill musl targets by introducing g++ wrapper scripts and wiring them into the build environment so cc-rs can compile .cxx sources against the musl sysroot.
Changes:
- Add
*-underhill-musl-g++wrapper scripts that applymusl-gcc.specsand inject sysroot libstdc++ header include paths. - Configure Cargo environment variables (
CXX_*) socc-rsdiscovers the new wrappers for all Underhill musl targets. - Extend the Nix dev shell to provide
*-linux-gnu-g++symlinks alongside existinggccsymlinks for consistent wrapper resolution.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
shell.nix |
Adds g++ symlinks into the temporary wrapper dir so the new scripts can find prefixed compilers in nix shells. |
build_support/underhill_cross/x86_64-underhill-musl-g++ |
New x86_64 musl g++ wrapper with sysroot C++ header auto-detection. |
build_support/underhill_cross/aarch64-underhill-musl-g++ |
New aarch64 musl g++ wrapper with sysroot C++ header auto-detection. |
.cargo/config.toml |
Adds CXX_* env entries for musl targets so cc-rs uses the new wrappers. |
Contributor
|
What C++ code are we compiling? Ideally we'd be reducing our non-Rust code, not adding more. |
The existing cross-compilation setup only had C compiler wrappers (*-underhill-musl-gcc). When cc-rs needed to compile .cxx files, it fell back to using that gcc wrapper as the C++ compiler, which failed because: 1. gcc doesn't search C++ standard library headers (<cassert>, etc.) 2. The musl-gcc.specs file uses -nostdinc, stripping all system include paths -- so even g++ can't find its C++ headers unless they're explicitly added back via -isystem Add matching g++ wrappers for both x86_64 and aarch64 that invoke g++ with the same musl specs, plus -isystem flags pointing at the sysroot's C++ headers. The C++ header version directory is auto-detected via shell glob rather than hardcoded. Also wire up the wrappers: - Add CXX_* env vars to .cargo/config.toml so cc-rs discovers them - Add g++ symlinks to shell.nix alongside the existing gcc symlinks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review feedback: - Validate that the sysroot env var is set, fail early with a clear error - Use -d check on glob results to handle missing C++ headers gracefully - Pass -isystem flags as properly quoted arguments instead of via an unquoted intermediate string to avoid accidental glob expansion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1b18dda to
599b0c8
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.
The existing cross-compilation setup only had C compiler wrappers
(
*-underhill-musl-gcc). When cc-rs needed to compile.cxxfiles, it fellback to using that gcc wrapper as the C++ compiler, which failed because:
<cassert>,<cstdint>, etc.)musl-gcc.specsfile uses-nostdinc, stripping all system includepaths — so even g++ can't find its C++ headers unless they're explicitly
added back via
-isystemThis PR adds matching g++ wrappers for both x86_64 and aarch64 that invoke g++
with the same musl specs, plus
-isystemflags pointing at the sysroot's C++headers. The C++ header version directory is auto-detected via shell glob rather
than hardcoded.
Changes
build_support/underhill_cross/{x86_64,aarch64}-underhill-musl-g++— new g++wrappers mirroring the existing gcc wrapper pattern
.cargo/config.toml— addedCXX_*env vars for all 4 musl targets socc-rs discovers the wrappers
shell.nix— added g++ symlinks alongside the existing gcc symlinks