feat: inherit path remap rules from cargo trim-paths - #1794
Conversation
cargo with `-Ztrim-paths` nightly feature would set
`CARGO_TRIM_PATHS_{SCOPE,REMAP}` for build scripts,
so they can forward path remaps to C/C++ compilers.
https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#profile-trim-paths-option
For other env vars, unfortunately we don't have any pass through, if there's any affecting support if flags I'll add it |
NobodyXu
left a comment
There was a problem hiding this comment.
Thank you!
I have some feedbacks on code, but overall looks good to me!
61520b6 to
cb96bd5
Compare
NobodyXu
left a comment
There was a problem hiding this comment.
Thanks, let's just add a probe and I'll cut a release if the code looks good
| // | ||
| // So we may assume `-fdebug-prefix-map` is always available, | ||
| // and only probe `-fmacro-prefix-map`. | ||
| if macro_scope { |
There was a problem hiding this comment.
Well I think we should still have s probe for debug, and I think it can be extracted as a function and only some minor differences in the string used
There was a problem hiding this comment.
Done the probe enhancement and extraction. Thanks for the suggestion. Looks better now.
BTW I am thinking about the support of clang-cl.exe though I guess we can leave this as a follow-up?
There was a problem hiding this comment.
Yeah for clang-cl.exe another PR makes sense, we have a is_like_clang_cl fn
Line 478 in e805bf3
Cargo with `-Ztrim-paths` feature would set `CARGO_TRIM_PATHS_SCOPE` and `CARGO_TRIM_PATHS_REMAP` for build scripts, so they can forward path remaps to C/C++ compilers. * `macro` scope -> `-fmacro-prefix-map` (`__FILE__` and friends) * `object` scope -> `-fdebug-prefix-map` (debug info) * `all` scope -> both * `diagnostics` and `none` scopes have no C equivalent MSVC is skipped as it has no equivalent flag family. It seems to have an undocumented `/pathmap` though, see bazelbuild/bazel 9466 This is inherited by default. Rationale: * Mirroring `inherit_rustflags` * The env vars only exist when the user opted into Cargo profile trim-paths. * It is easy to opt-out from user. Cargo only looks at your local package's profile, and you can also do a per-dependency profile override. Do note this is nightly only feature from Cargo's point of view.
a58148f to
c69f330
Compare
We probe these flags and skip with a warning when the compiler rejects it. * `-fdebug-prefix-map`: supported since GCC 4.3 (2008-03), Clang 3.8 (2016-03): * <https://gcc.gnu.org/onlinedocs/gcc-4.3.0/gcc/Debugging-Options.html> * <llvm/llvm-project@436256a> * `-fmacro-prefix-map`: supported since GCC 8.1 (2018-05), Clang 10.0 (2020-03) * <https://gcc.gnu.org/onlinedocs/gcc-8.1.0/gcc/Option-Summary.html> * <https://releases.llvm.org/10.0.0/tools/clang/docs/ReleaseNotes.html>
NobodyXu
left a comment
There was a problem hiding this comment.
Thank you LGTM! I will merge and cut a release
### What does this PR try to resolve? With this we get remap for free when building in rustc bootstrap: See * rust-lang/rust#161049 * rust-lang#17309 * rust-lang/cc-rs#1794 ### How to test and review this PR?
With this, we get C dep remap for free when building in rustc bootstrap: See * rust-lang/rust#161049 * rust-lang/cargo#17309 * rust-lang/cc-rs#1794
With this, we get C dep remap for free when building in rustc bootstrap: See * rust-lang#161049 * rust-lang/cargo#17309 * rust-lang/cc-rs#1794
What this is for
Fixes #593
Cargo with
-Ztrim-pathsfeature would setCARGO_TRIM_PATHS_SCOPEandCARGO_TRIM_PATHS_REMAPfor build scripts,so they can forward path remaps to C/C++ compilers.
macroscope ->-fmacro-prefix-map(__FILE__and friends)objectscope ->-fdebug-prefix-map(debug info)allscope -> bothdiagnosticsandnonescopes have no C equivalentMSVC is skipped as it has no equivalent flag family.
It seems to have an undocumented
/pathmapthough,see bazelbuild/bazel#9466
This is inherited by default. Rationale:
inherit_rustflagspackage's profile, and you can also do a per-dependency profile
override.
Do note again this is nightly only feature from Cargo's point of view,
so I put a warning in the doc comment.
See also https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#profile-trim-paths-option
How to review
Commit by commit.
The first one capture the current behavior.
Subsequent commits show the behavior change through the diff.
I have a cargo
-Zscriptrepro you can run against master or this PR:repro script
Note
I don't know if this is a known issue,but the flag-support probe (is_flag_supported_inner) doesn't inherit env vars from parent orBuild::env.This would result in a different invocation from the real compile.For example you may setCC="zig cc",andzig ccmay support less flags than latest gcc/clang.See #1794 (comment)