Platform/toolchain support for C++ rules #6516

gregestren · 2018-10-25T19:37:55Z

Tracking issue on Bazel Configurability Roadmap

Goal:

C++ toolchains can be defined with Bazel's generic toolchain machinery
C++ rules read these toolchains
C++ rules resolve properly against platforms

hlopko · 2018-10-26T13:04:51Z

Before I leave for 3 weeks of vacation, let me say I'm not aware of any work needed now that 683c302 is submitted. John, do you know about anything?

katre · 2018-10-26T13:48:22Z

Testing, that's all. Enjoy your vacation!

hlopko · 2018-11-28T08:47:09Z

I'm back! :)

Could you give me a list of behaviors/features that I should test? I don't know where to start :)

katre · 2018-11-28T15:10:12Z

The big areas we've had problems with were around finding the correct CROSSTOOL file for a cc_toolchain. @nlopezgi might have more ideas, but honestly I'd just start taking any cc rules integration tests, and try running them with toolchain resolution enabled for cc toolchains, to see what happens.

nlopezgi · 2018-11-29T18:43:14Z

I'd like to see tests that run with custom values for some of the ENV variables that cc_config uses. Most of the times we have seen issues is because we set custom values for some of these ENV variables in our rules (https://github.com/bazelbuild/bazel-toolchains/blob/master/rules/environments.bzl#L30).

nlopezgi · 2018-11-29T18:46:28Z

This is a bigger ask: I would like to see changes that impact crosstool/c_toolchain compatibility to be guarded behind --incompatible flags. Let me expand that: Currently, when there are changes to the CROSSTOOL / c_toolchain, there is no guarding them behind an --incopatible flag (afaik). For users that have hand built or checked in versions of a CROSSTOOL / c_toolchain, they have no way of preventing Bazel changes in release X+1 from breaking them.
For example, a recent change to how toolchains are selected using as key only e.g., "k8" instead of e.g., "k8|clang" could have been guarded behind an --incompatible flag for ~1 bazel release. That way, I could have tested that with version X my c_toolchain that did not have the "k8|clang" would work, but if I use --incompatible_someting_something it would break.

nlopezgi · 2018-11-29T18:51:30Z

The other problem I had in the past is that adding the
enabled_toolchain_types=@bazel_tools//tools/cpp:toolchain_type
made it so that the OPT toolchain was always selected and linkopt flags were not passed (#5291)
(not sure if this is relevant to this thread or if it has been fixed already)

hlopko · 2018-11-30T15:04:17Z

Hi Nick!

tests for cc_configure, absolutely, but since we plan a big rewrite of cc_configure because Rewrite CROSSTOOL in Starlark #5380, we will only add tests during the rewrite. I think it's independent from this issue.
we (C++ rules) already do that, and we did that before it was cool. This is the flag for the change that broke you: Disallow using CROSSTOOL to select the cc_toolchain label #6382. The problem was that it took us too long to polish the policy, so the upcoming incompatible changes announcement didn't receive due attention.
I pinged linkopts flags are not passed when cpp toolchains are enabled #5291, let's see what are the next steps.

Thanks for chiming in!

katre · 2018-11-30T15:10:19Z

Also, this issue is specifically for "cc rules work when --enabled_toolchain_types=@bazel_tools//tools/cpp:toolchain_type is passed". Other cc rules issues should be filed separately.

nlopezgi · 2018-12-07T16:33:22Z

Thanks for the clarifications @hlopko , sorry for tagging along other stuff here @katre

jin · 2019-01-14T23:12:34Z

@gregestren could you add a priority to this issue, please?

gregestren · 2019-01-29T19:52:16Z

See #7260

dslomov · 2019-02-15T15:18:22Z

Please do not assign issues to more than one team

Previously, we accessed that information from TransitiveInfoCollection, but when we migrate to platforms/toolchains we only have access to the toolchain provider. This cl adds a field on the `CcToolchainInfo` so we can migrate C++ rules to platforms/toolchains. #6516 RELNOTES: None. PiperOrigin-RevId: 236351968

This is needed to build Bazel when --incompatible_enable_cc_toolchain_resolution is set. #6516 #7260 RELNOTES: None. PiperOrigin-RevId: 238209238

gregestren · 2020-07-15T18:21:21Z

Update: a major reason this hasn't been toggled on yet is it'll break Android or iOS projects with C++ native deps. Configurability devs are doing a concerted focus now on adding platform/toolchain support for Android builds to unblock. We may be getting support for doing the same for iOS builds too, although I'm unclear on how much yet.

c-mita · 2020-11-23T17:36:42Z

Changing team to reflect who's been doing the work.

katre · 2020-11-23T17:44:06Z

Downgrading priority to reflect the current level of activity.

…ime_lib to Starlark This is required since existing cc_toolchain.all_files doesn't usually contain C++ runtime library. Progress towards bazelbuild/bazel#6516 RELNOTES: `cc_toolchain.static_runtime_lib` and `cc_toolchain.dynamic_runtime_lib` are now exposed to Starlark. PiperOrigin-RevId: 242863875

bazelbuild/bazel#6516 RELNOTES: None. PiperOrigin-RevId: 241523004

With this cl we will pass target libc to the host configuration and analyze it from cc_toolchain attribute. This is needed to migrate C++ rules to platforms, under which cc_toolchain is analyzed in the host configuration. bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 241043559

This way, we can compute build variables for a different configuration than the one present at cc_toolchain analysis. Do bad things in the process (such as storing BuildOptions in FeatureConfigurationForStarlark or creating AppleConfiguration inline in the AdditionalBuildVariableComputer). bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240997356

This is a preparation for moving variables away from toolchains to rules (because toolchains don't get analyzed in the same configuration as rules). bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240759048

…target configuration This is there to allow cc_toolchain to detect that FDO is active, and to preprocess profiles, while being analyzed in the host configuration. FDO related options are passed to the host configuration, they are preprocessed in the host configuration, but then used in the target configuration. Extra care is made to not enable FDO in the host configuration (as this is the existing behavior). bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240721577

Fdo related logic is complicated enough to have it in a separate class. bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240376113

This is unfortunate, confusing for the user, and less accurate than what the original behavior was, but because cc_toolchain cannot access (currently, with platforms enabled) the target configuration, there is nothing we can do other than showing the error for the target. bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240345500

This falls nicely into our quest to not use configuration from cc_toolchain :) bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240338037

Instead, compute it for each target separately using its rule context. bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240330179

Instead read it from the CppConfiguration passed from the target. bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240317365

In order to migrate C++ rules to platforms, we need the access to the C++ configuration fragment from the caller rule in Starlark APIs. All existing APIs have already access to it, but cc_common.configure_features doesn't. This incompatible change adds a ctx argument to configure_features. bazelbuild/bazel#7793 bazelbuild/bazel#6516 RELNOTES: Added `--incompatible_require_ctx_in_configure_features`, see bazelbuild/bazel#7793 for details. PiperOrigin-RevId: 240099411

Benefits: - More efficient caching for exec actions (which shouldn't integrate FDO data) - simpler, more straightforward code - resolves outdated logic from before toolchain transitions (https://github.com/bazelbuild/proposals/blob/main/designs/2020-02-07-toolchain-transition-migration.md) #6516 PiperOrigin-RevId: 478011721 Change-Id: I8df049eeb78003fa9e979a91313e45cb9de95d01

Benefits: - More efficient caching for exec actions (which shouldn't integrate FDO data) - simpler, more straightforward code - resolves outdated logic from before toolchain transitions (https://github.com/bazelbuild/proposals/blob/main/designs/2020-02-07-toolchain-transition-migration.md) bazelbuild#6516 PiperOrigin-RevId: 478011721 Change-Id: I8df049eeb78003fa9e979a91313e45cb9de95d01

github-actions · 2023-04-12T01:32:29Z

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

github-actions · 2023-04-27T01:32:49Z

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

In order to migrate C++ rules to platforms, we need the access to the C++ configuration fragment in Starlark APIs. All existing APIs have already access to it, but cc_common.configure_features doesn't. This change adds a ctx argument to configure_features. This is the migration needed for bazelbuild/bazel#7793, and is part of the effort for bazelbuild/bazel#6516. If the rule doesn't depend on cpp fragment yet, you will have to add `fragments =['cpp']` argument to the rule() call. Note that this behavior is only available in Bazel 0.25 (to be released this month). RELNOTES: None. PiperOrigin-RevId: 247206987

In order to migrate C++ rules to platforms, we need the access to the C++ configuration fragment in Starlark APIs. All existing APIs have already access to it, but cc_common.configure_features doesn't. This change adds a ctx argument to configure_features. This is the migration needed for bazelbuild/bazel#7793, and is part of the effort for bazelbuild/bazel#6516. If the rule doesn't depend on cpp fragment yet, you will have to add `fragments =['cpp']` argument to the rule() call. Note that this behavior is only available in Bazel 0.25 (to be released this month). RELNOTES: None. PiperOrigin-RevId: 248534309

gregestren assigned katre and hlopko Oct 25, 2018

gregestren added team-Configurability platforms, toolchains, cquery, select(), config transitions team-Rules-CPP Issues for C++ rules labels Oct 25, 2018

hlopko added the Bazel 1.0 label Nov 28, 2018

hlopko mentioned this issue Dec 17, 2018

incompatible_disable_runtimes_filegroups: Remove cc_toolchain.static_runtime_libs and cc_toolchain.dynamic_runtime_libs #6942

Closed

jin added untriaged and removed untriaged labels Jan 14, 2019

katre added the P1 I'll work on this now. (Assignee required) label Jan 15, 2019

katre mentioned this issue Feb 12, 2019

incompatible_enable_cc_toolchain_resolution: Turn on toolchain resolution for cc rules #7260

Open

excitoon mentioned this issue Feb 15, 2019

Have some concept of platform that can be used from WORKSPACE and BUILD files #1485

Closed

dslomov removed the team-Configurability platforms, toolchains, cquery, select(), config transitions label Feb 15, 2019

bazel-io pushed a commit that referenced this issue Mar 13, 2019

Add toolchains attribute to compiler_flag

5af2cbf

This is needed to build Bazel when --incompatible_enable_cc_toolchain_resolution is set. #6516 #7260 RELNOTES: None. PiperOrigin-RevId: 238209238

gregestren mentioned this issue Jul 10, 2020

Platform/toolchain support for Android rules #11749

Open

c-mita added team-Configurability platforms, toolchains, cquery, select(), config transitions and removed team-Rules-CPP Issues for C++ rules labels Nov 23, 2020

katre added P2 We'll consider working on this in future. (Assignee optional) type: feature request and removed P1 I'll work on this now. (Assignee required) labels Nov 23, 2020

zhlhahaha mentioned this issue Jan 21, 2021

cross compilation error when import both c toolchain and rules_go #12859

Closed

luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022

Do not try to unzip fdo profile in the host configuration

1b07f74

bazelbuild/bazel#6516 RELNOTES: None. PiperOrigin-RevId: 241523004

luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022

Introduce FdoHelper

c580bf3

Fdo related logic is complicated enough to have it in a separate class. bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240376113

luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022

Remove unused arguments

b5ed979

This falls nicely into our quest to not use configuration from cc_toolchain :) bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240338037

github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Apr 12, 2023

github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Platform/toolchain support for C++ rules #6516

Platform/toolchain support for C++ rules #6516

gregestren commented Oct 25, 2018

hlopko commented Oct 26, 2018

katre commented Oct 26, 2018

hlopko commented Nov 28, 2018

katre commented Nov 28, 2018

nlopezgi commented Nov 29, 2018

nlopezgi commented Nov 29, 2018

nlopezgi commented Nov 29, 2018 •

edited

Loading

hlopko commented Nov 30, 2018

katre commented Nov 30, 2018

nlopezgi commented Dec 7, 2018 •

edited by hlopko

Loading

jin commented Jan 14, 2019

gregestren commented Jan 29, 2019

dslomov commented Feb 15, 2019

gregestren commented Jul 15, 2020

c-mita commented Nov 23, 2020

katre commented Nov 23, 2020

github-actions bot commented Apr 12, 2023

github-actions bot commented Apr 27, 2023

Platform/toolchain support for C++ rules #6516

Platform/toolchain support for C++ rules #6516

Comments

gregestren commented Oct 25, 2018

hlopko commented Oct 26, 2018

katre commented Oct 26, 2018

hlopko commented Nov 28, 2018

katre commented Nov 28, 2018

nlopezgi commented Nov 29, 2018

nlopezgi commented Nov 29, 2018

nlopezgi commented Nov 29, 2018 • edited Loading

hlopko commented Nov 30, 2018

katre commented Nov 30, 2018

nlopezgi commented Dec 7, 2018 • edited by hlopko Loading

jin commented Jan 14, 2019

gregestren commented Jan 29, 2019

dslomov commented Feb 15, 2019

gregestren commented Jul 15, 2020

c-mita commented Nov 23, 2020

katre commented Nov 23, 2020

github-actions bot commented Apr 12, 2023

github-actions bot commented Apr 27, 2023

nlopezgi commented Nov 29, 2018 •

edited

Loading

nlopezgi commented Dec 7, 2018 •

edited by hlopko

Loading