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

Rewrite cc_configure #6926

Closed
hlopko opened this issue Dec 14, 2018 · 14 comments
Closed

Rewrite cc_configure #6926

hlopko opened this issue Dec 14, 2018 · 14 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@hlopko
Copy link
Member

hlopko commented Dec 14, 2018

Current cc_configure is greatly suboptimal:

@PiotrSikora
Copy link
Contributor

It should also include the ability to easily switch between GNU libstdc++ and LLVM libc++.

Currently, this requires either:

I think this problem falls into overall theme of this issue, but please let me know if I should open a separate issue to track this. Thanks!

@htuch
Copy link

htuch commented Jan 16, 2019

Some earlier context on libc++ and Linux at #4524

@hlopko
Copy link
Member Author

hlopko commented Feb 4, 2019

@PiotrSikora and @htuch - is passing BAZEL_LINKOPTS="--stdlib=libc++:-lc++" not enough to use libc++? I think it works, at least on linux.

@PiotrSikora
Copy link
Contributor

@hlopko you need to pass -stdlib=libc++ to the compiler (hence BAZEL_CXXOPTS), otherwise you'll include C++ headers from libstdc++ and not libc++.

@hlopko
Copy link
Member Author

hlopko commented Feb 5, 2019

Yup. you're right. All of this is doable with Bazel today. Is this enough for you, or you'd want an ability to switch it more easily than that?

@PiotrSikora
Copy link
Contributor

Yup. you're right. All of this is doable with Bazel today.

I wanted to verify that BAZEL_CXXOPTS can be set in .bazelrc using --action_env, so I updated Bazel to 0.23rc1, and even though local_config_cc/cc_toolchain_config.bzl is generated correctly and includes proper cxx_builtin_include_directories, cxx_flags and link_flags, it appears that libc++ builds are completely broken in 0.23rc1.

Perhaps cxx_builtin_include_directories are not included in the sandbox?

fatal error: 'string' file not found
#include <string>

Tests to catch any regressions there would be great.

Is this enough for you, or you'd want an ability to switch it more easily than that?

Personally, I'm fine with adding a few lines to .bazelrc to enable --config=libc++ myself, though it would be great if it just worked out of the box :)

@htuch @jmillikin-stripe Do you guys have any opinion on that? The two of you are a bit more pedantic, when it comes to the build system, than I am.

@hlopko
Copy link
Member Author

hlopko commented Feb 7, 2019

cxx_builtin_include_directories are only for .d file validation, if you need to add those directories and their contents to the sandbox, you have to update filegroups in cc_toolchain target. Does that help?

@PiotrSikora
Copy link
Contributor

@hlopko the same build works fine with Bazel 0.22, so I doubt it's something that fundamental.

Any chance you could take a look at it? The build instructions are pretty easy.

@hlopko
Copy link
Member Author

hlopko commented Feb 13, 2019

Hi Piotr,

could you pls check if its #7397? I'd build bazel at 71bc38f and then one commit before and see if it's the culprit. If so, pls comment on the issue. Thanks so much!

@PiotrSikora
Copy link
Contributor

@hlopko sorry, it appears that there was a change that broke local builds with libc++ in our build system committed at the time when I was testing 0.23rc1, and after reverting it 0.23rc1 seems to work fine. Sorry for the noise!

@keith
Copy link
Member

keith commented Apr 17, 2019

Here's another use case that would be nice to cover so that users can override some pieces of the toolchain, without having to create their own crosstool #7883

@hlopko
Copy link
Member Author

hlopko commented May 9, 2019

From email threads:

  • MSVC: SDK version selection - have an ability specify which version of the sdk to use.
  • ARM64 on Windows - cover this platform as well.

bazel-io pushed a commit that referenced this issue May 14, 2019
Tested with Bazel CI downstream projects. These fail for unrelated reasons. Both together show that everything is green with my change:
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/971
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/969

This change makes it unnecessary to patch most CppActionConfig features only in Linux for now.

The build_interface_libraries feature is still patched. Once Bazel is released with this change: 4eb7683
we can stop patching it.

Also a separate change should do the same for Windows and Mac.

This also unblocks flipping --incompatible_do_not_split_linking_cmdline

#8303, #6926, #7687

RELNOTES:none
PiperOrigin-RevId: 248117783
hlopko added a commit to hlopko/bazel that referenced this issue May 29, 2019
Fixes bazelbuild#8479.

To keep this backwards compatible we have to detect xcode early in the Bazel
build (even when no C++ is being built). In cases where user knows there is
xcode, or when they know it won't be needed, I'm adding environment variable
`BAZEL_USE_XCODE_TOOLCHAIN`. When set to `1`, Bazel will not try to detect
xcode, it will assume it is there.

Makes bazelbuild#6926 a little bit more
complicated.

RELNOTES: `BAZEL_USE_XCODE_TOOLCHAIN=1` tells Bazel not to look for Xcode to
decide whether to enable toolchains for Apple rules, but to assume Xcode is
available. Can be also used when building on Darwin and no C++ or ObjC is being
built, so there is no need to detect Xcode.
bazel-io pushed a commit that referenced this issue May 29, 2019
Fixes #8479.

To keep this backwards compatible we have to detect xcode early in the Bazel
build (even when no C++ is being built). In cases where user knows there is
xcode, or when they know it won't be needed, I'm adding environment variable
`BAZEL_USE_XCODE_TOOLCHAIN`. When set to `1`, Bazel will not try to detect
xcode, it will assume it is there.

Makes #6926 a little bit more
complicated.

RELNOTES: `BAZEL_USE_XCODE_TOOLCHAIN=1` tells Bazel not to look for Xcode to
decide whether to enable toolchains for Apple rules, but to assume Xcode is
available. Can be also used when building on Darwin and no C++ or ObjC is being
built, so there is no need to detect Xcode.

Closes #8492.

PiperOrigin-RevId: 250518695
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
Tested with Bazel CI downstream projects. These fail for unrelated reasons. Both together show that everything is green with my change:
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/971
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/969

This change makes it unnecessary to patch most CppActionConfig features only in Linux for now.

The build_interface_libraries feature is still patched. Once Bazel is released with this change: bazelbuild@4eb7683
we can stop patching it.

Also a separate change should do the same for Windows and Mac.

This also unblocks flipping --incompatible_do_not_split_linking_cmdline

bazelbuild#8303, bazelbuild#6926, bazelbuild#7687

RELNOTES:none
PiperOrigin-RevId: 248117783
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
Fixes bazelbuild#8479.

To keep this backwards compatible we have to detect xcode early in the Bazel
build (even when no C++ is being built). In cases where user knows there is
xcode, or when they know it won't be needed, I'm adding environment variable
`BAZEL_USE_XCODE_TOOLCHAIN`. When set to `1`, Bazel will not try to detect
xcode, it will assume it is there.

Makes bazelbuild#6926 a little bit more
complicated.

RELNOTES: `BAZEL_USE_XCODE_TOOLCHAIN=1` tells Bazel not to look for Xcode to
decide whether to enable toolchains for Apple rules, but to assume Xcode is
available. Can be also used when building on Darwin and no C++ or ObjC is being
built, so there is no need to detect Xcode.

Closes bazelbuild#8492.

PiperOrigin-RevId: 250518695
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
Fixes bazelbuild#8479.

To keep this backwards compatible we have to detect xcode early in the Bazel
build (even when no C++ is being built). In cases where user knows there is
xcode, or when they know it won't be needed, I'm adding environment variable
`BAZEL_USE_XCODE_TOOLCHAIN`. When set to `1`, Bazel will not try to detect
xcode, it will assume it is there.

Makes bazelbuild#6926 a little bit more
complicated.

RELNOTES: `BAZEL_USE_XCODE_TOOLCHAIN=1` tells Bazel not to look for Xcode to
decide whether to enable toolchains for Apple rules, but to assume Xcode is
available. Can be also used when building on Darwin and no C++ or ObjC is being
built, so there is no need to detect Xcode.

Closes bazelbuild#8492.

PiperOrigin-RevId: 250518695
@dslomov dslomov removed the bazel 1.0 label Jul 24, 2019
@ccontavalli-j
Copy link

Also consider environments where multiple compilers may be available and necessary to build different targets. For example, having a dependency on a library that can only be built with gcc, and a different library that can only be built with clang.

A way to do this would be to extend cc_configure and/or cc_autoconf.* to take parameters, so one can specify a path for the compiler to use, for example, and the name of the toolchain to create.

Those parameters can be filled in by default from environment variables, or detection code, but you can leave the option to call cc_configure() with the correct parameters to use of bazel to simplify the detection and creation of multiple toolchains.

See also: #7020

@c-mita c-mita added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Nov 23, 2020
@sgowroji sgowroji added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 16, 2023
@sgowroji
Copy link
Member

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

@sgowroji sgowroji closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

8 participants