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 CROSSTOOL in Starlark #5380

Open
scentini opened this Issue Jun 12, 2018 · 0 comments

Comments

Projects
None yet
2 participants
@scentini
Copy link
Contributor

scentini commented Jun 12, 2018

Description of the problem / feature request:

Change the file format used to configure C++ toolchain from text proto to Skylark

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

  • It will make maintenance of the C++ toolchain easier, it will allow getting rid of the duplication and boilerplate using standard programming practices, e.g. we will be able to provide a library of common features.
  • We will benefit from the tooling being developed for Skylark (e.g. testing, debugger).
  • It will make future C++ rules consistent with all the other Skylark rules. It will unblock rewriting of cc_toolchain rule in Skylark.

More info on this effort can be found in @hlopko 's doc: https://docs.google.com/document/d/1Nqf16jqDGWSrPp4VuRxh0iNnVBoAXsO0meDH69J9xoc/edit#heading=h.r30au8wdo4dh

@scentini scentini self-assigned this Jun 12, 2018

bazel-io pushed a commit that referenced this issue Jun 13, 2018

Add "toolchain-identifier" attribute to cc_toolchain rule
Currently the selection of C++ toolchain configuration depends on 2 different things:
1. CROSSTOOL file: a proto text file that contains multiple CrosstoolConfig.CToolchain messages, out of which we want to select the appropriate CToolchain;
2. cc_toolchain_suite rule, specified by --crosstool_top option: this rule contains "toolchains" map attribute. The map's keys are of type <cpu>|<compiler>, or just <cpu>, and the values are labels of cc_toolchain rules (which contain additional C++ information).

If there is an entry in cc_toolchain_suite.toolchains that corresponds to the --cpu and --compiler options, we use it, otherwise we loop through all the CToolchains in the CROSSTOOL file, select the one that corresponds to the --cpu and --compiler options, and then get the cc_toolchain label from cc_toolchain_suite.toolchains[toolchain.targetCpu|toolchain.compiler].
In both cases we read the CROSSTOOL file and pass all its information forward, to be used in creation of CppConfiguration and CcToolchainProvider creation.

As part of the efforts of rewriting CROSSTOOL in Skylark, we need to make obtaining the cc_toolchain label independent of the CROSSTOOL file and toolchain selection. As a step towards that goal, we add a new "toolchain_identifier" attribute to cc_toolchain, which uniquely identifies a CToolchain in the CROSSTOOL file.

Now the process of getting the CToolchain goes as follows:
Check for existence of cc_toolchain_suite.toolchains[<cpu>|<compiler>], if --compiler is specified, otherwise check for cc_toolchain_suite.toolchains[<cpu>].

1. if a value is found, load the cc_toolchain rule and look for the toolchain_identifier attribute.
    1.a if the attribute exists, loop through all the CToolchains in CROSSTOOL and select the one with the matching toolchain identifier.
    1.b otherwise fall back to selecting the CToolchain from CROSSTOOL by matching the --cpu and --compiler values.

2. If a value is not found, select the CToolchain from CROSSTOOL by matching the --cpu and --compiler values, and construct the key as follows: <toolchain.cpu>|<toolchain.compiler>.

In the future we will get rid of 2. by making sure that cc_toolchain_suite.toolchains[<cpu>|<compiler>] and cc_toolchain_suite.toolchains[<cpu>] are always defined.
1.b will be removed by making the cc_toolchain.toolchain_identifier attribute mandatory
After this deriving the cc_toolchain label will be independent of the CROSSTOOL file

1.a will ultimately be replaced by an attribute that points to a skylark_crosstool rule, that will contain all the information that CROSSTOOL contains, allowing us to remove CROSSTOOL altogether.

Work towards issue #5380
RELNOTES: None.
PiperOrigin-RevId: 200388550

bazel-io pushed a commit that referenced this issue Jun 19, 2018

Add fields to CcToolchainFeatures.Feature and CcToolchainFeatures.Act…
…ionConfig that are needed for FeatureConfiguration creation

In the future CcToolchainFeatures will not be created from a CToolchain but from a CrosstoolInfo provider that will encapsule all relevant information from the CROSSTOOL. Feature and ActionConfig classes need to carry all the information that is currently obtained from CToolchain.

Work towards issue #5380

RELNOTES: None.
PiperOrigin-RevId: 201147336

ArielleA added a commit to ArielleA/bazel that referenced this issue Jun 19, 2018

Add "toolchain-identifier" attribute to cc_toolchain rule
Currently the selection of C++ toolchain configuration depends on 2 different things:
1. CROSSTOOL file: a proto text file that contains multiple CrosstoolConfig.CToolchain messages, out of which we want to select the appropriate CToolchain;
2. cc_toolchain_suite rule, specified by --crosstool_top option: this rule contains "toolchains" map attribute. The map's keys are of type <cpu>|<compiler>, or just <cpu>, and the values are labels of cc_toolchain rules (which contain additional C++ information).

If there is an entry in cc_toolchain_suite.toolchains that corresponds to the --cpu and --compiler options, we use it, otherwise we loop through all the CToolchains in the CROSSTOOL file, select the one that corresponds to the --cpu and --compiler options, and then get the cc_toolchain label from cc_toolchain_suite.toolchains[toolchain.targetCpu|toolchain.compiler].
In both cases we read the CROSSTOOL file and pass all its information forward, to be used in creation of CppConfiguration and CcToolchainProvider creation.

As part of the efforts of rewriting CROSSTOOL in Skylark, we need to make obtaining the cc_toolchain label independent of the CROSSTOOL file and toolchain selection. As a step towards that goal, we add a new "toolchain_identifier" attribute to cc_toolchain, which uniquely identifies a CToolchain in the CROSSTOOL file.

Now the process of getting the CToolchain goes as follows:
Check for existence of cc_toolchain_suite.toolchains[<cpu>|<compiler>], if --compiler is specified, otherwise check for cc_toolchain_suite.toolchains[<cpu>].

1. if a value is found, load the cc_toolchain rule and look for the toolchain_identifier attribute.
    1.a if the attribute exists, loop through all the CToolchains in CROSSTOOL and select the one with the matching toolchain identifier.
    1.b otherwise fall back to selecting the CToolchain from CROSSTOOL by matching the --cpu and --compiler values.

2. If a value is not found, select the CToolchain from CROSSTOOL by matching the --cpu and --compiler values, and construct the key as follows: <toolchain.cpu>|<toolchain.compiler>.

In the future we will get rid of 2. by making sure that cc_toolchain_suite.toolchains[<cpu>|<compiler>] and cc_toolchain_suite.toolchains[<cpu>] are always defined.
1.b will be removed by making the cc_toolchain.toolchain_identifier attribute mandatory
After this deriving the cc_toolchain label will be independent of the CROSSTOOL file

1.a will ultimately be replaced by an attribute that points to a skylark_crosstool rule, that will contain all the information that CROSSTOOL contains, allowing us to remove CROSSTOOL altogether.

Work towards issue bazelbuild#5380
RELNOTES: None.
PiperOrigin-RevId: 200388550

ArielleA added a commit to ArielleA/bazel that referenced this issue Jun 19, 2018

Add fields to CcToolchainFeatures.Feature and CcToolchainFeatures.Act…
…ionConfig that are needed for FeatureConfiguration creation

In the future CcToolchainFeatures will not be created from a CToolchain but from a CrosstoolInfo provider that will encapsule all relevant information from the CROSSTOOL. Feature and ActionConfig classes need to carry all the information that is currently obtained from CToolchain.

Work towards issue bazelbuild#5380

RELNOTES: None.
PiperOrigin-RevId: 201147336

bazel-io pushed a commit that referenced this issue Jun 21, 2018

Extract logic from CROSSTOOL in CrosstoolInfo provider
CrosstoolInfo carries the necessary information from the CROSSTOOL text proto. Later on, instead from the CROSSTOOL file and the corresponding CToolchain, CrosstoolInfo will be derived from a skylark_crosstool rule implemented in Skylark. Therefore CToolchain involvement in CppConfiguration and CcToolchain creation needs to be eliminated.

Work towards issue #5380

RELNOTES: None.
PiperOrigin-RevId: 201491207

werkt added a commit to werkt/bazel that referenced this issue Aug 2, 2018

Add "toolchain-identifier" attribute to cc_toolchain rule
Currently the selection of C++ toolchain configuration depends on 2 different things:
1. CROSSTOOL file: a proto text file that contains multiple CrosstoolConfig.CToolchain messages, out of which we want to select the appropriate CToolchain;
2. cc_toolchain_suite rule, specified by --crosstool_top option: this rule contains "toolchains" map attribute. The map's keys are of type <cpu>|<compiler>, or just <cpu>, and the values are labels of cc_toolchain rules (which contain additional C++ information).

If there is an entry in cc_toolchain_suite.toolchains that corresponds to the --cpu and --compiler options, we use it, otherwise we loop through all the CToolchains in the CROSSTOOL file, select the one that corresponds to the --cpu and --compiler options, and then get the cc_toolchain label from cc_toolchain_suite.toolchains[toolchain.targetCpu|toolchain.compiler].
In both cases we read the CROSSTOOL file and pass all its information forward, to be used in creation of CppConfiguration and CcToolchainProvider creation.

As part of the efforts of rewriting CROSSTOOL in Skylark, we need to make obtaining the cc_toolchain label independent of the CROSSTOOL file and toolchain selection. As a step towards that goal, we add a new "toolchain_identifier" attribute to cc_toolchain, which uniquely identifies a CToolchain in the CROSSTOOL file.

Now the process of getting the CToolchain goes as follows:
Check for existence of cc_toolchain_suite.toolchains[<cpu>|<compiler>], if --compiler is specified, otherwise check for cc_toolchain_suite.toolchains[<cpu>].

1. if a value is found, load the cc_toolchain rule and look for the toolchain_identifier attribute.
    1.a if the attribute exists, loop through all the CToolchains in CROSSTOOL and select the one with the matching toolchain identifier.
    1.b otherwise fall back to selecting the CToolchain from CROSSTOOL by matching the --cpu and --compiler values.

2. If a value is not found, select the CToolchain from CROSSTOOL by matching the --cpu and --compiler values, and construct the key as follows: <toolchain.cpu>|<toolchain.compiler>.

In the future we will get rid of 2. by making sure that cc_toolchain_suite.toolchains[<cpu>|<compiler>] and cc_toolchain_suite.toolchains[<cpu>] are always defined.
1.b will be removed by making the cc_toolchain.toolchain_identifier attribute mandatory
After this deriving the cc_toolchain label will be independent of the CROSSTOOL file

1.a will ultimately be replaced by an attribute that points to a skylark_crosstool rule, that will contain all the information that CROSSTOOL contains, allowing us to remove CROSSTOOL altogether.

Work towards issue bazelbuild#5380
RELNOTES: None.
PiperOrigin-RevId: 200388550

werkt added a commit to werkt/bazel that referenced this issue Aug 2, 2018

Add fields to CcToolchainFeatures.Feature and CcToolchainFeatures.Act…
…ionConfig that are needed for FeatureConfiguration creation

In the future CcToolchainFeatures will not be created from a CToolchain but from a CrosstoolInfo provider that will encapsule all relevant information from the CROSSTOOL. Feature and ActionConfig classes need to carry all the information that is currently obtained from CToolchain.

Work towards issue bazelbuild#5380

RELNOTES: None.
PiperOrigin-RevId: 201147336

werkt added a commit to werkt/bazel that referenced this issue Aug 2, 2018

Extract logic from CROSSTOOL in CrosstoolInfo provider
CrosstoolInfo carries the necessary information from the CROSSTOOL text proto. Later on, instead from the CROSSTOOL file and the corresponding CToolchain, CrosstoolInfo will be derived from a skylark_crosstool rule implemented in Skylark. Therefore CToolchain involvement in CppConfiguration and CcToolchain creation needs to be eliminated.

Work towards issue bazelbuild#5380

RELNOTES: None.
PiperOrigin-RevId: 201491207

bazel-io pushed a commit that referenced this issue Aug 21, 2018

Make creating a CcToolchainConfigInfo from Skylark possible
Work towards issue #5380.

//tools/cpp/cc_toolchain_config_lib.bzl contains all the necessary functionality for
one to be able to create a CcToolchainConfigInfo provider from Skylark. CcToolchainConfigInfo currently encapsulates the info read from the CROSSTOOL file. We are moving away from using the CROSSTOOL files, in the future we will reroute the logic
through an attribute of cc_toolchain that will point to a Skylark rule.
A CcToolchainConfigInfo provider can be created with the cc_common.create_cc_toolchain_config_info()
method.

e.g:

crosstool_rule = rule{
    implementation = _impl,
    provides = [CcToolchainConfigInfo],
)

def _impl(ctx):
    feature1 = feature(
        name = "feature_one",
        flag_sets = [
            flag_set(actions = ["action1"]),
            flag_set(
                actions = ["action1"],
                flag_groups = [
                    flag_group(
                        flags = ["flag1"],
                    ),
                    flag_group(
                        flag_groups = [
                            flag_group(
                                flags = ["flag1"],
                            ),
                        ],
                        expand_if_equal = variable_with_value(
                            name = "variable1",
                            value = "value",
                        ),
                    ),
                ],
                expand_if_all_available = ["sysroot"],
            ),
        ],
    )
    action_config1 = action_config(action_name = "action_one", enabled = True)
    action_config2 = action_config(
        action_name = "action_two",
        enabled = True,
        tools = [tool(path = "/a/b/c")],
    )
    artifact_name_pattern1 = artifact_name_pattern(
        category_name = "static_library",
        prefix = "prefix",
        extension = ".a",
    )
    cc_target_os = "os"

    return  cc_common.create_cc_toolchain_config_info(
        features = [feature1],
        action_configs = [action_config1, action_config2],
        artifact_name_patterns = [artifact_name_pattern1],
        cc_target_os = cc_target_os,
        make_variables = [make_variable(name = "v1", value = "val")],
    )

RELNOTES: None.
PiperOrigin-RevId: 209648136

bazel-io pushed a commit that referenced this issue Sep 6, 2018

Add entries in cc_toolchain_suites so cc_toolchain label selection do…
…esn't depend on the CROSSTOOL file.

Work towards issue #5380

RELNOTES: None.
PiperOrigin-RevId: 211771190

bazel-io pushed a commit that referenced this issue Sep 6, 2018

Add 'toolchain_config' attribute to cc_toolchain to enable replacing …
…CROSSTOOL with a skylark rule

Work towards issue #5380.

RELNOTES: None.
PiperOrigin-RevId: 211793570

bazel-io pushed a commit that referenced this issue Oct 24, 2018

Fix relative path calculation for Starlark rules that provide CcToolc…
…hainConfigInfo

Work towards #5380
RELNOTES: None.
PiperOrigin-RevId: 218484449

bazel-io pushed a commit that referenced this issue Nov 15, 2018

Make cc_common.create_cc_toolchain_config_info() deal with legacy fea…
…ture behavior

After selecting the CToolchain, Bazel tempers with it by adding and moving features, action configs, and artifact name patterns (see CppToolchainInfo#addLegacyFeatures() ).

What it does is :

For every ArtifactCategory defined in, if the CToolchain does not contain an equivalent ArtifactNamePattern, we add it.

if CToolchain does not contain "no_legacy_features" feature:
    - if it contains "legacy_compile_flags" feature, move it to the very top of the list of features (above both CToolchain features and legacy ones).
    - prepend a bunch of action_configs in front of the ones that CToolchain defines
    - prepend a bunch of features in front of the ones that CToolchain defines, IF they are not already present.
    - append a bunch of features to the list that CToolchain defines ,IF they are not already present.

This cl introduces the above described behavior to cc_common.create_cc_toolchain_config_info().

Work towards #5380

RELNOTES: None.
PiperOrigin-RevId: 221595847

bazel-io pushed a commit that referenced this issue Nov 26, 2018

Add a CcToolchainConfigInfo#toProto() method, to be used for comparis…
…on between data obtained from starlark rule and a CToolchain from CROSSTOOL file.

Work towards issue #5380

RELNOTES: None.
PiperOrigin-RevId: 222815904

@hlopko hlopko added the bazel 1.0 label Nov 28, 2018

@hlopko hlopko changed the title Rewrite CROSSTOOL in Skylark Rewrite CROSSTOOL in Starlark Nov 29, 2018

bazel-io pushed a commit that referenced this issue Dec 4, 2018

Allow an empty actions parameter for flag_set()
Work towards issue #5380
RELNOTES: None.
PiperOrigin-RevId: 223936920

bazel-io pushed a commit that referenced this issue Dec 4, 2018

Allow digits in action_configs' and features' names
Work towards issue #5380
RELNOTES: None.
PiperOrigin-RevId: 223946963

bazel-io pushed a commit that referenced this issue Dec 5, 2018

Propagate CppOptions.enableCcToolchainConfigInfoFromSkylark field to …
…host configuration

Work towards issue #5380.
RELNOTES: None.
PiperOrigin-RevId: 224125325

bazel-io pushed a commit that referenced this issue Dec 5, 2018

Modify process of creating 'flag_set's for action_configs in Starlark…
… to match the one of creating 'flag_set's from CToolchain.

A flag_set that is meant for an action_config should not have elements in it's actions parameter. Instead, we put the action's name in the flag_set.actions list.

Work towards issue #5380

RELNOTES: None.
PiperOrigin-RevId: 224203125

bazel-io pushed a commit that referenced this issue Dec 18, 2018

Remove legacy parameters from cc_common.create_cc_toolchain_config_in…
…fo().

Exception is the need_pic field, until we figure out its story.

Dynamic linking depends on the presence of "dynamic_linking_mode" feature.

List of removed fields:                     | What we pass to CcToolchainConfigInfo
                                            | constructor instead:

supports_gold_linker                        | check for corresponding feature
supports_start_end_lib                      | check for corresponding feature
supports_interface_shared_objects           | check for corresponding feature
supports_embedded_runtimes                  | check for corresponding feature
supports_fission                            | check for corresponding feature
supports_dsym                               | unused, pass 'false'
static_runtime_filegroup                    | pass empty string
dynamic_runtime_filegroup                   | pass empty string
compiler_flags                              | pass empty list
cxx_flags                                   | pass empty list
unfiltered_cxx_flags                        | pass empty list
linker_flags                                | pass empty list
dynamic_library_linker_flags                | pass empty list
test_only_linker_flags                      | pass empty list
objcopy_embed_flags                         | pass empty list
ld_embed_flags                              | pass empty list
compilation_mode_compiler_flags             | pass empty list
compilation_mode_cxx_flags                  | pass empty list
compilation_mode_linker_flags               | pass empty list
mostly_static_linking_mode_flags            | pass empty list
dynamic_linking_mode_flags                  | pass empty list
fully_static_linking_mode_flags             | pass empty list
mostly_static_libraries_linking_mode_flags  | pass empty list
default_libc_top                            | pass empty string

Work towards issue #5380
RELNOTES: None.
PiperOrigin-RevId: 225978244

bazel-io pushed a commit that referenced this issue Dec 18, 2018

Allow . in cc_toolchain_config feature names
We discovered version number often goes into feature names, therefore we'll allow .

Progress towards #5380

RELNOTES: None
PiperOrigin-RevId: 225995495

bazel-io pushed a commit that referenced this issue Jan 3, 2019

Add a script for diffing command lines based on bazel aquery outputs
Progress towards #5380 and #5883

RELNOTES: None.
PiperOrigin-RevId: 227646729

bazel-io pushed a commit that referenced this issue Jan 3, 2019

Make cmd_line_differ be CWD friendly
Progress towards #5380 and #5883

RELNOTES: None.
PiperOrigin-RevId: 227675648

bazel-io pushed a commit that referenced this issue Jan 3, 2019

Make cc_common.create_cc_toolchain_config_info() return default value…
…s for legacy fields

Progress towards #5380

RELNOTES: None.
PiperOrigin-RevId: 227677715

bazel-io pushed a commit that referenced this issue Jan 8, 2019

Fix CcToolchainConfigInfo.proto
In proto format, FlagSets that belong to an ActionConfig must not have elements in their 'actions' repeated field. This field is populated with the action name during construction of the Java ActionConfig object.
This cl fixes the CcToolchainConfigInfo.proto method to reflect the above, by clearing the FlagSet.actions field from CToolchain.ActionConfig objects

Progress towards #5380

RELNOTES: None.
PiperOrigin-RevId: 228302139
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment