Standard constraint_setting for C++ standard libraries

[samolisov]

Description of the problem / feature request:

Bazel's embedded_tools/tools/cpp/BUILD file contains a constraint_setting to set/detect the compiler: cc_compiler. Sometimes a build also depends on the C++ standard library and if a user wish to set some compiler flags or linked libraries according to the standard library (pass -stdlib=libc++ if libc++ is in use, for example) a config_setting where a constant that represents the standard library is can be very suitable.

Of course, a user can always define a constraint set whenever it is required but I believe it is a good idea to have a standard constraint_setting and a predefined set of standard libraries in Bazel itself.

Feature requests: what underlying problem are you trying to solve with this feature?

I propose to define a constraint_set for c++ standard libraries, for example:

constraint_setting(name = "cc_standard_library")

constraint_value(
    name = "libstdc++",
    constraint_setting = ":cc_standard_library",
)

constraint_value(
    name = "libc++",
    constraint_setting = ":cc_standard_library",
)

constraint_value(
    name = "msvc",
    constraint_setting = ":cc_standard_library",
)
...

so that a user will be able to define platforms depending on the compiler and standard library used to build a software.

[samolisov]

Maybe I've opened an issue in a wrong repo, but I see Bazel has embedded_tools as well as a separated project for C++ support: rules_cc. Should I move the issue there?

[aiuto]

This may span this repo, rules_cc and platforms. Let's let some people look and see before moving it to another placel.

[aiuto]

@katre For impact on toolchains.

[katre]

This decision should be made by @oquenchil and the C++ rules team, but our experience has been that the compiler constraint is a mistake (similarly for modelling JDK version as a constraint). This leads to a cartesian duplication of platforms, one for every compiler, and confusion for users ("I'm building for my local machine, I have clang and gcc installed, why does the platform change?").

The right model for compiler (and JDK version) is as a flag which then influences the toolchain. Whether the clang and gcc toolchains are separate cc_toolchain instances that are chosen via select, or whether it's one cc_toolchain instance is an implementation detail of C++ rules and is immaterial to toolchain resolution and to other rules.

I think the C standard library should function similarly, but I'll leave @oquenchil to weigh in on that.

[oquenchil]

I will take John's experience here and agree with the stance that this should come from a configuration option.

Once we move C++ configuration to a fully Starlark configuration we can definitely do this with a label-typed build setting.

Starlarkification of the C++ rules themselves has started, It will be a long while before we start to starlarkify the C++ configuration though.

[AustinSchuh]

You can extract the glibc version the same way the compiler is extracted today. Assuming my github starlark is up to snuff, the following should do it:

load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")

def _abi_libc_version_flag_impl(ctx):
    toolchain = find_cpp_toolchain(ctx)
    return [config_common.FeatureFlagInfo(value = toolchain.abi_libc_version)]

abi_libc_version_flag = rule(
    implementation = _abi_libc_versior_flag_impl,
    attrs = {
        "_cc_toolchain": attr.label(default = Label("//tools/cpp:current_cc_toolchain")),
    },
    toolchains = ["@bazel_tools//tools/cpp:toolchain_type"],
    incompatible_use_toolchain_transition = True,
)

abi_libc_version_flag(name = "abi_libc_version")

You can then select on the abi_libc_version the same way you can select on compiler today. Something like:

config_setting(
    name = "is_glibc",
    flag_values = {
        ":abi_libc_version": "libc++",
    },
)

...
select({":is_glibc": ["-DGLIBC"], "//conditions:default", []})

And in the next Bazel release, you should also be able to use this to declare targets incompatible and skip them using this.

[samolisov]

@AustinSchuh Thank you for the suggestion, I'm going to try it on the latest Bazel release.

[aiuto]

Austin:

I did try that yesterday in a PR I have out where I use toolchain values to name output files. https://github.com/bazelbuild/rules_pkg/pull/256 But I don't see abi_libc_version in any CC toolchain. There is 'libc', but for auto configure it is 'local', which is not that informative. Were you working from a real toolchain, or is this just a concept?

That said, the concept does actually work. It might just require adding more attributes to toolchains to precisely specify the nuances you want to select on.

On Thu, Nov 12, 2020 at 2:03 AM Pavel Samolysov notifications@github.com wrote:

@AustinSchuh https://github.com/AustinSchuh Thank you for the suggestion, I'm going to try it on the latest Bazel release.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/bazel/issues/11534#issuecomment-725883273, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXHHHCGOKKAF3TIKB6LO6DSPOCD7ANCNFSM4NQPYDMA .

[AustinSchuh]

@samolisov , Tony's reply above shows some of the subtleties of this approach. You need your toolchains to set any properties you care about to be able to select on them.

@aiuto , For sure, garbage in, garbage out :P. We only use custom C++ toolchains, which have everything filled out reasonably well.

You are spoiling the thunder on my talk tomorrow. I am going to be making the case as part of the incompatible target skipping talk that we might need to add and definately need to standardize attributes on toolchains for this exact reason. 'local' is unhelpful, and if someone uses 'GCC', and someone uses 'gcc' for the compiler field, that is also unhelpful.

[aiuto]

This of this discussion as some softball questions for the talk. :-)

As for 'GCC' vs. 'gcc', we are open to adding these nouns to the platforms repository. That will at least give us uniform spelling. We still have to leave it up to the toolchain authors to mix with other constraints to define what the toolchain represents.

On Thu, Nov 12, 2020 at 1:51 PM Austin Schuh notifications@github.com wrote:

@samolisov https://github.com/samolisov , Tony's reply above shows some of the subtleties of this approach. You need your toolchains to set any properties you care about to be able to select on them.

@aiuto https://github.com/aiuto , For sure, garbage in, garbage out :P. We only use custom C++ toolchains, which have everything filled out reasonably well.

You are spoiling the thunder on my talk tomorrow. I am going to be making the case as part of the incompatible target skipping talk that we might need to add and definately need to standardize attributes on toolchains for this exact reason. 'local' is unhelpful, and if someone uses 'GCC', and someone uses 'gcc' for the compiler field, that is also unhelpful.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/bazel/issues/11534#issuecomment-726269911, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXHHHEF4EXPRCL3PQRPKWDSPQVBRANCNFSM4NQPYDMA .

[AustinSchuh]

I forgot the smiley face. All good.

I'm not sure if we should add them somewhere or just document them with the toolchain docs for each toolchain type. They aren't really a 'platform', so @platforms isn't a great spot. Maybe in @rules_cc and the various other rules for constants?

[github-actions[bot]]

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.

[AustinSchuh]

I think this is still relevant. We keep finding standard library specific things we need to do, and this would help with that.

[aiuto]

Agreed. Not stale. But probably as part of rules_cc. That has come a long way in the last 2 years so perhaps it is time. @comius @oquenchil WDYT? Move to rules_cc?

[oquenchil]

This is already in team-Rules-CPP, for tracking I'd leave the issue here.

[github-actions[bot]]

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 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.