Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bazel@HEAD broke toolchains #198

Closed
hlopko opened this issue Oct 12, 2018 · 11 comments
Closed

Bazel@HEAD broke toolchains #198

hlopko opened this issue Oct 12, 2018 · 11 comments
Assignees

Comments

@hlopko
Copy link
Member

hlopko commented Oct 12, 2018

Hi all,

tonight downstream pipeline was red because of toolchain_identifier error: https://buildkite.com/bazel/bazel-with-downstream-projects-bazel/builds/498#d982b428-5036-41e2-9edc-1107b286d850

This might have been caused by bazelbuild/bazel@c3fb1db or by (now rolledback) bazelbuild/bazel@3aedb2f (leaving for the plane in 3 minutes, cannot investigate more).

Can you help me understand what caused the failure, and whether you think the new behavior should be guarded with an incompatible flag (https://docs.bazel.build/versions/master/skylark/backward-compatibility.html#disallow-using-crosstool-to-select-the-cc_toolchain-label).

@xingao267 xingao267 self-assigned this Oct 12, 2018
@xingao267
Copy link
Member

It's probably due to bazelbuild/bazel@c3fb1db. The error message is "ERROR: Toolchain identifier 'local' was not found, valid identifiers are [stub_armeabi-v7a, linux_gnu_x86, msys_x64_mingw, msvc_x64]".

The tests that are failing are doing a simple cc autoconfig using Bazel@HEAD in a container (doing bazel build --all_incompatible_changes @local_config_cc//... using bazel@HEAD in a container).

I'm trying to understand why "local" is no longer a valid toolchain identifier.

@hlopko
Copy link
Member Author

hlopko commented Oct 16, 2018

Cc @meteorcloudy. Ha I finally know what's going on. I'm rolling back the bazelbuild/bazel@c3fb1db, but the fix has to be made in bazel-toolchains. The problem is that you're building @local_config_cc//..., but with a different CROSSTOOL (because you specify your own toolchain using --crosstool_top). That results in @local_config_cc//tools/cpp:cc-compiler-k8 to be used with a different CROSSTOOL, and failing. I consider building cc_toolchain with a different CROSSTOOL to be unsupported use case and therefore outside of backwards incompatible policy. And FYI this would just work as expected if local_config_cc used (cc_toolchain_suite|cc_toolchain).proto attribute, or when we'll migrate to crosstool in skylark (bazelbuild/bazel#5380).

Today it works just because toolchain selection is super benevolent and will accept anything that even remotely looks like could be used. I'm fixing that. I'm sure that's not what you intended to test.

I guess the fix is not to set --crosstool_top when testing local_config_cc.

meteorcloudy added a commit to bazelbuild/continuous-integration that referenced this issue Oct 16, 2018
bazel-toolchains need to adapt incompatible changes and fix their bug.
See bazelbuild/bazel-toolchains#198
@philwo @mhlopko
@hlopko
Copy link
Member Author

hlopko commented Oct 16, 2018

I discussed this with colleagues and decided to temporarily remove bazel-toolchains from Bazel with downstream projects pipeline. The problem is that to roll out my change I'd have to create incompatible environment variable, and duplicate all the affected files, and there's no infrastructure to handle incompatible environment variables in place. And since we consider this behavior a bug, we'll not follow the policy (also because the policy is not yet set in stone :)
Thanks for understanding.

@nlopezgi
Copy link
Contributor

Hi Marcel,
Sorry for these issue. I should be able to fix these tests by not using the --all_incompatible_changes flag for these builds. Will follow up soon with a PR.

@xingao267
Copy link
Member

@mhlopko we tried removed the --all_incompatible_changes flag in #203, but it looks like the tests are still failing due to the same reason. https://source.cloud.google.com/results/invocations/169760d3-f703-4f0a-9bfd-d8f853042c99/targets/%2F%2Ftest%2Fconfigs:debian-jessie-bazel-head-autoconfig_test/log

I'm going to temporarily disable the two failing tests in the presubmit.

@hlopko
Copy link
Member Author

hlopko commented Oct 16, 2018

The real issue is that you build @local_config_cc//..., that builds all cc_toolchain_suite and cc_toolchain targets inside, but without using their corresponding CROSSTOOL (that is specified by --crosstool_top, and I assume you specify it to point to your own toolchain suite). Am I making any sense?

@xingao267
Copy link
Member

I'm a little confused. In this test, we don't specify --crosstool_top. We just run bazel build @local_config_cc//... in a container to let Bazel autoconfig itself, and then we extract the generated CROSSTOOL and BUILD files. These configs will later be used for remote execution together with that same container.

@xingao267
Copy link
Member

Or maybe something is changed for how we should obtain those cc toolchain files?

@hlopko
Copy link
Member Author

hlopko commented Oct 16, 2018

And there's no --crosstool_top anywhere (e.g. bazelrc)? Then I misunderstood the issue. But I do observe cc_toolchain/CROSSTOOL mismatch. Is it possible that you patch or manipulate with the @local_config_cc//tools/cpp:CROSSTOOL somehow?

@nlopezgi
Copy link
Contributor

Thanks for all the comments Marcel.
I was able to debug further, and it seems this error is occurring because we have some env variables set in our containers. Specifically, if I remove the following env variable, the tests pass:
export CC_TOOLCHAIN_NAME="linux_gnu_x86"
Is it wrong to set this env variable now (it used to work?)?
How can we set this env variable to some value so that our toolchain identifier will not come out as "local" (which does not make much sense for rbe) in here (https://github.com/bazelbuild/bazel-toolchains/blob/master/configs/ubuntu16_04_clang/1.1/bazel_0.17.1/default/CROSSTOOL#L89) (but in a way that avoids this error).

@hlopko
Copy link
Member Author

hlopko commented Oct 18, 2018

Thanks Nick, that was valuable debugging! This is exactly the problem. We use CC_TOOLCHAIN_NAME to set the toolchain_identifier in the CROSSTOOL, but not in the BUILD file. I'll fix it today. I apologise for the overhead you had to have for our bug.

katre pushed a commit to bazelbuild/bazel that referenced this issue Nov 13, 2018
…is set

Since we specify toolchain_identifier in cc_toolchain now, it is much more
strict in selecting a CROSSTOOL toolchain. This broke when CC_TOOLCHAIN_NAME
environment variable was set, as that only renamed toolchain identifier in the
CROSSTOOL, but not in the cc_toolchain. This cl fixes that.

This fixed bazelbuild/bazel-toolchains#198.

RELNOTES: None
PiperOrigin-RevId: 217666695
katre pushed a commit to bazelbuild/bazel that referenced this issue Nov 14, 2018
…is set

Since we specify toolchain_identifier in cc_toolchain now, it is much more
strict in selecting a CROSSTOOL toolchain. This broke when CC_TOOLCHAIN_NAME
environment variable was set, as that only renamed toolchain identifier in the
CROSSTOOL, but not in the cc_toolchain. This cl fixes that.

This fixed bazelbuild/bazel-toolchains#198.

RELNOTES: None
PiperOrigin-RevId: 217666695
freeformstu pushed a commit to freeformstu/bazel-toolchains that referenced this issue Nov 11, 2023
This is a fairly straightforward change that adds support for using `bazel-toolchain` with `bzlmod`.

The `llvm.toolchain` extension is only a wrapper around the existing `llvm_toolchain` repository rule and supports the same attributes.

For trying it out, add the following to your `MODULE.bazel`:

```
bazel_dep(name = "com_grail_bazel_toolchain", version = "0.8")
git_override(module_name = "com_grail_bazel_toolchain", remote = "https://github.com/grailbio/bazel-toolchain", commit = "cf915e5c7cfcd19a3e71de54e385e01240b865dc")

llvm = use_extension("@com_grail_bazel_toolchain//toolchain:extensions.bzl", "llvm")
llvm.toolchain(
    llvm_version = "15.0.0"
)

use_repo(llvm, "llvm_toolchain")

register_toolchains("@llvm_toolchain//:all")
```

---------

Co-authored-by: James Sharpe <james.sharpe@zenotech.com>
Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Co-authored-by: Gabriel Féron <g@leirbagl.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants