Document preferred way to add opt only compile flags with rules based toolchain

[keith]

A common pattern in the previous CC toolchain was to have some compile flags split on dbg/fastbuild/opt. I don't see any docs or examples of how to do that with the rule based toolchain. My first attempt was this:

cc_feature_constraint(
    name = "opt_cfg",
    all_of = ["@rules_cc//cc/toolchains/features:opt"],
)

cc_args(
    name = "opt_compile_flags",
    actions = ["@rules_cc//cc/toolchains/actions:compile_actions"],
    args = [
        "-DNDEBUG",
        "-D_FORTIFY_SOURCE=1",
        "-O2",
    ],
    requires_any_of = [":opt_cfg"],
)

But the flags aren't added, so I assume that's not the right move. I found another example where the user just overrode the feature instead:

https://github.com/lowRISC/opentitan/blob/7704c7a75d9fff2fdfb42e40d5bd311ac9f94f9b/toolchain/BUILD#L193-L202

[AustinSchuh]

You are prodding in eerily similar code right now to me.

I found it super handy to put a print inside src/main/starlark/builtins_bzl/common/cc/cc_toolchain_provider_helper.bzl and print out toolchain_config_info. That shows the exact crosstool protocol buffer being fed into bazel, if I'm reading it right. In theory, we should be able to get that to match with both APIs.

The code you have above looks right. That generates:

  flag_set {
    action: "assemble"
    action: "c++-compile"
    action: "c++-header-parsing"
    action: "c++-module-codegen"
    action: "c++-module-compile"
    action: "c++-module-deps-scanning"
    action: "c++20-module-codegen"
    action: "c++20-module-compile"
    action: "c-compile"
    action: "clif-match"
    action: "linkstamp-compile"
    action: "lto-backend"
    action: "preprocess-assemble"                                                                                                       
    flag_group {                                                                                                                                      
      flag: "-DNDEBUG"                                                                                  
      flag: "-D_FORTIFY_SOURCE=1"                                                                                                                                   
      flag: "-O2"                                                                                                                                                                                                               
    }                                                                                                                                           
    with_feature {                                                                                                      
      feature: "opt"                                                                                                               
    }     
  } 

For unix_cc_toolchain_config, the old toolchain definition when adjusted to have the same flags, generates an identical crosstool protocol buffer.

If you change it to requires_any_of = ["@rules_cc//cc/toolchains/features:opt"],, that generates an equivalent crosstool protocol buffer.

I'm trying to convert unix_cc_toolchain_config over to the new rules based toolchain. When comparing the original crosstool with the new one, I see the following only in the working crosstool:

feature {                      
  name: "opt"                                                                       
  enabled: false                                               
} 

This makes me think there is something wrong with the definitions for the external features for "opt" and the others that is somehow preventing them from being created automatically.

[keith]

thanks for the debugging tip! that's very useful. with working on the unix toolchain conversion are you removing the reliance on the legacy features? im considering looking into that but if you're already nearly done with that I wouldn't have to!

[AustinSchuh]

Keith, I'm starting with "Port, line for line, the old syntax over to rules". I don't have any legacy features converted right now. Now I know to check in with you before starting so we don't duplicate each other :)

[trybka]

I found it super handy to put a print inside src/main/starlark/builtins_bzl/common/cc/cc_toolchain_provider_helper.bzl and print out toolchain_config_info. That shows the exact crosstool protocol buffer being fed into bazel, if I'm reading it right. In theory, we should be able to get that to match with both APIs.

Similar to this but without needing to modify builtins, etc. you can accomplish this with cquery.

e.g.

bazel cquery --output=starlark --starlark:expr 'providers(target).get("CcToolchainConfigInfo").proto' $CC_TOOLCHAIN

If your goals involve a straight port, in theory you could resurrect the comparator tests: https://github.com/bazelbuild/rules_cc/blob/0.0.1/tools/migration/ctoolchain_comparator.py

[keith]

that's a great tip! note that for use with the rule based toolchain if you have a toolchain like:

cc_toolchain(
    name = "clang_toolchain",
...

The target you need to use is :_clang_toolchain_config

[keith]

It seems like in the proto representation from this i get 3 duplicate entries 1 after the other, any idea how to eliminate that? it just adds some noise to diffs

[TroyKomodo]

Was there a consensus on the preferred way to handle opt cc args and by extension coverage cc args too.

[cerisier]

The way I do it for now is using the legacy opt family of features.

cc_feature(
    name = "opt",
    overrides = "@rules_cc//cc/toolchains/features:opt",
    args = [
        ":opt_compile_flags",
        ":opt_link_flags",
    ],
)

then

cc_toolchain(
    name = "cc_toolchain",
    args = [":toolchain_args"],
    known_features = [
       ":opt",
    ],
    // ...
)

[armandomontanez]

Sorry this got lost in time, @cerisier's suggestion is the intended approach. We should definitely document this, though!

[eldjh3]

Wouldn’t it be cleaner to make these external features fully fledged and thus able to be used directly in requires_any_of rather than requiring them to be overridden?

Firstly it means that anyone using them has to define their own versions overriding the ones in rules_cc//cc/toolchains/features. Secondly, you now have two features, the original one and the one that overrides it. Which should be used to match on in cc_args etc. From the above it sounds like it has to be the custom one.

It feels like the whole situation would be simpler if bazel’s internally defined external features could just be used as-is.