Set a default value for --host_crosstool_top #12093
Conversation
katre
force-pushed
the
default-host-crosstool-top
branch
from
493fd7d to
86b4928
Compare
Previously it was unset and defaulted to --crosstool_top, but that doesn't make sense for android/apple/etc builds. Ideally we would set --host_crosstool_top to the value of --crosstool_top at the beginning of the build, but that isn't currently possible.
katre
force-pushed
the
default-host-crosstool-top
branch
from
86b4928 to
cdb1f0f
Compare
katre
changed the title
| changed = true; | ||
| } | ||
| if (hostCrosstoolTop != null | ||
| && hostCrosstoolTop.getName().equals(HOST_CROSSTOOL_TOP_NOT_YET_SET)) { |
There was a problem hiding this comment.
Do we actually need a special marker value or does null still work here? As the code stands now, what case does the != null check catch?
It took me a lot of time to follow the logic from 6720ff3, which set this basic pattern. Even with the crucial comments at
bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
Lines 653 to 656 in 6720ff3
My recollection now is that null default doesn't work for targetLibcTopLabel because the similarly named libcTopLabel does default to null. So if you build in a default target configuration but with --android_grte_top=<something non-null>, the top-level version of getNormalized would set targetLibcTopLabel = libcTopLabel = null, and then the Android version would re-trigger because it thought null meant "never set".
host_crosstool_top isn't quite the same, is it? Because the equivalent to libcTopLabel is crosstoolTop, which itself doesn't default to null?
Whatever logic we want to use, can we document these obscure subtleties super-precisely?
There was a problem hiding this comment.
You're right, using "null" is fine for host_crosstool_top. Let me know if the current documentation is insufficient.
|
Also, did a particular use case catch this issue? |
|
This was caught when I was testing the change to use toolchain transitions for Java toolchains. Specifically, building an android binary (which transitions the crosstool_top to an android-specific crosstool) stopped working, because Bazel was using the android crosstool to try and build tools. This wasn't a problem previously because the single host transition was created before the android transition had happened, and so saw the original crosstool_top. |
Previously it was unset and defaulted to --crosstool_top in whatever configuration was being used, but that doesn't make sense for android/apple/etc builds. Instead we use getNormalized to set --host_crosstool_top in the target configuration. Closes bazelbuild#12093. PiperOrigin-RevId: 332256211
Previously it was unset and defaulted to --crosstool_top in whatever configuration was being used, but that
doesn't make sense for android/apple/etc builds.
Instead we use getNormalized to set --host_crosstool_top in the target configuration.