Removed rust_toolchain.os in favor of rust_toolchain.exec_triple.
#1960
Conversation
UebelAndre
force-pushed
the
toolchain-triple
branch
2 times, most recently
from
c156ca8
to
76946eb
Compare
| @@ -1822,10 +1818,10 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, crate | |||
|
|
|||
| use_pic = _should_use_pic(cc_toolchain, feature_configuration, crate_type, compilation_mode) | |||
|
|
|||
| if toolchain.os == "windows": | |||
| if toolchain.target_os == "windows": | |||
There was a problem hiding this comment.
I could be wrong but I think this section has a mixed use of the "os". The formatting of the link flags would be dependent on what tools are being used (MSVC's link.exe vs gcc). This may be something to address in #1958 but worth considering in this change as well.
| exec_os = exec_triple.system | ||
|
|
||
| if not exec_os: | ||
| if not exec_triple.system: | ||
| fail("No system was provided for the execution platform. Please update {}".format( |
There was a problem hiding this comment.
I think this fail could probably be removed but not entirely sure.
UebelAndre
force-pushed
the
toolchain-triple
branch
from
76946eb
to
3781a54
Compare
UebelAndre
requested review from
krasimirgg and
illicitonion
and removed request for
krasimirgg
|
What's the benefit to introducing |
|
…bazelbuild#1960) * Removed `rust_toolchain.os` in favor of `rust_toolchain.exec_triple`. * Regenerate documentation
…bazelbuild#1960) * Removed `rust_toolchain.os` in favor of `rust_toolchain.exec_triple`. * Regenerate documentation
rust_toolchain.exec_triplehas been a mandatory field for years (f1b8fd3) but by maintaining legacy behavior, there's now an opportunity for users to define misleading toolchains whererust_toolchain.osis ambiguous and/or doesn't matchrust_toolchain.exec_triple.Changes:
osattribute fromrust_toolchaintarget_osto the toolchain provider which is derived from thetarget_tripleortarget_jsonattributes.osfield on the toolchain provider withtarget_osas it's clearer and more matches prior art with Cargo custom targets.Note that all uses of
toolchain.oshave been replaced withtoolchain.target_osto avoid a change in behavior.rust_toolchain.oswas near universally set to the target triple (see repository_utils.bzl) unless users defined their own toolchains without usingrules_rustrepository rules. That said, the use oftarget_osmay not be correct in all it's uses today.