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

Config setting with msvc compiler constraint appears to be never selected #7730

Closed
oakad opened this issue Mar 15, 2019 · 14 comments
Closed
Labels
area-Windows Windows-specific issues and feature requests P3 We're not considering working on this, but happy to review a PR. (No assignee) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: documentation (cleanup)

Comments

@oakad
Copy link

oakad commented Mar 15, 2019

Tested with bazel 0.23.1

I'm trying to use the following construct to select compiler specific code:

config_setting(
	name = "windows_msvc",
	constraint_values = [
		"@bazel_tools//platforms:windows",
		"@bazel_tools//tools/cpp:msvc",
	],
	visibility = ["//visibility:public"],
)

Unfortunately, it appears that select clauses predicated with ":windows_msvc" are never selected. No errors are being reported and I have tried several compiler/cpu command line flag combinations as well as some custom ...cpp:cc_compiler constraint values to no avail.

If I comment out the "msvc" line everything works as expected, apart from the fact that the build becomes unsuitable for msys.

It appears to me that a minor bug of some sort may exist within the msvc toolchain definitions.

@oakad oakad changed the title Config setting with msvc compailer constraint appears to be never selected Config setting with msvc compiler constraint appears to be never selected Mar 15, 2019
@aiuto aiuto added area-Windows Windows-specific issues and feature requests untriaged labels Mar 18, 2019
@laszlocsomor
Copy link
Contributor

@katre : Could you please help triaging?

@katre
Copy link
Member

katre commented Mar 18, 2019

The config_setting.constraint_values are matched against the target platform, not the selected toolchain. I suspect that this is happening because no platform is setting the :msvc constraint.

How are you selecting the msvc compiler in the first place? If you are using the --compiler flag, then your constraint_setting should also be using that.

@oakad
Copy link
Author

oakad commented Mar 19, 2019

@katre Of course I tested various variants.

bazel build --compiler msvc ...
in cc_toolchain_suite rule @local_config_cc//:toolchain: cc_toolchain_suite '@local_config_cc//:toolchain' does not contain a toolchain for cpu 'x64_windows' and compiler 'msvc'.

Ok, another try:

build:msvc --compiler msvc-cl ...

Works, but there's no constraint to select msvc-cl.

Let's try to create a custom constraint value:

constraint_value(
	name = "msvc-cl",
	constraint_setting = "@bazel_tools//tools/cpp:cc_compiler",
)

config_setting(
	name = "windows_msvc",
	constraint_values = [
		"@bazel_tools//platforms:windows",
		#"@bazel_tools//tools/cpp:msvc",
		":msvc-cl",
	],
	visibility = ["//visibility:public"],
)

No errors reported, but config is not selected - default select clause gets triggered.

Which brings me to a more general question: is there a setting or aspect to make bazel print the state of constraints and selected configs?

@katre
Copy link
Member

katre commented Mar 19, 2019

I am sorry that I was not clear: do not use the msvc constraint value as part of your config_setting.

Since you are setting the flag --compiler, use that in the config_setting:

config_setting(
    name = "windows_msvc",
    values = {
        "compiler": "msvc",
    },
)

This config_setting will match whenever the build has the flag --compiler msvc.

@laszlocsomor
Copy link
Contributor

@katre : Thanks for answering! Is there a Bazel bug here?

@katre
Copy link
Member

katre commented Mar 19, 2019

No, there is no bug.

I should probably work with @hlopko to remove the confusing compiler constraint_setting, however.

@laszlocsomor
Copy link
Contributor

Thanks!

@oakad
Copy link
Author

oakad commented Mar 20, 2019

If there's no bug, then the whole thing should be branded as "confusing feature implementation".

Allow me to repeat:

bazel build --compiler msvc ...
in cc_toolchain_suite rule @local_config_cc//:toolchain: cc_toolchain_suite '@local_config_cc//:toolchain' does not contain a toolchain for cpu 'x64_windows' and compiler 'msvc'.

But it does appears to work if compiler is specified as "msvc-cl" (this should be at least documented).

In general, though, the diagnostics offered by the Bazel regarding selected config values must be somehow improved (or documented!).

@katre
Copy link
Member

katre commented Mar 20, 2019

Re-opening and assigning to @laszlocsomor to figure out why the --compiler flag isn't working as expected.

@hlopko
Copy link
Member

hlopko commented Mar 20, 2019

@oakad I think you have to use bazel build --compiler msvc-cl, that's the compiler used by our autoconfigured toolchain.

If you want to decouple your select from whether we use --crosstool_top, --cpu, and --compiler to select the C++ toolchain, or platforms/toolchains, you can use https://source.bazel.build/bazel/+/fc69ecd1824d1f318a7a18d5a955040eed23c6ea:tools/cpp/compiler_flag.bzl.

Does that help?

@laszlocsomor
Copy link
Contributor

@hlopko : Please help triaging this bug. Shall we improve documentation? If so, how?

@laszlocsomor laszlocsomor added type: documentation (cleanup) P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Mar 26, 2019
@laszlocsomor
Copy link
Contributor

@hlopko, What shall we do here? Is "type: documentation" accurate?

@meisterT
Copy link
Member

@oakad anything needs to be done here?

@oakad
Copy link
Author

oakad commented May 13, 2020

Don't know. I'm avoiding msvc now (clang on msys will be much welcome, though, as mentioned in my other ticket).

@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 21, 2021
The macro utilises bazel "transitions" to set the `make` toolchain to
be the preinstalled `nmake` tool.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 21, 2021
The macro utilises bazel "transitions" to set the `make` toolchain to
be the preinstalled `nmake` tool.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 21, 2021
Note that the msvc constraint was removed from the
exec_compatible_with attribute of preinstalled_nmake_toolchain as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR bazel-contrib#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 23, 2021
The macro utilises bazel "transitions" to set the `make` toolchain to
be the preinstalled `nmake` tool.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 23, 2021
The macro utilises bazel "transitions" to set the `make` toolchain to
a given make variant toolchain, e.g. preinstalled_nmake.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 23, 2021
The macro utilises bazel "transitions" to set the `make` toolchain used
in the configure_make() or make() rules to
a given make variant toolchain, e.g. preinstalled_nmake.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 23, 2021
The macro utilises bazel "transitions" to set the `make` toolchain used
in the configure_make() or make() rules to
a given make variant toolchain, e.g. preinstalled_nmake.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 23, 2021
The macros utilise bazel "transitions" to set the `make` toolchain used
in the configure_make(), cmake() or make() rules to
a given make variant toolchain, e.g. preinstalled_nmake.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 23, 2021
The macros utilise bazel "transitions" to set the `make` toolchain used
in the configure_make(), cmake() or make() rules to
a given make variant toolchain, e.g. preinstalled_nmake.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 23, 2021
The macros utilise bazel "transitions" to set the `make` toolchain used
in the configure_make(), cmake() or make() rules to
a given make variant toolchain, e.g. preinstalled_nmake.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 25, 2021
The macros utilise bazel "transitions" to set the `make` toolchain used
in the configure_make(), cmake() or make() rules to
a given make variant toolchain, e.g. preinstalled_nmake.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jheaff1 added a commit to jheaff1/rules_foreign_cc that referenced this issue Jul 28, 2021
The macros utilise bazel "transitions" to set the `make` toolchain used
in the configure_make(), cmake() or make() rules to
a given make variant toolchain, e.g. preinstalled_nmake.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
jsharpe pushed a commit to bazel-contrib/rules_foreign_cc that referenced this issue Jul 28, 2021
The macros utilise bazel "transitions" to set the `make` toolchain used
in the configure_make(), cmake() or make() rules to
a given make variant toolchain, e.g. preinstalled_nmake.

Note that the msvc constraint was removed from the
`exec_compatible_with` attribute of `preinstalled_nmake_toolchain` as
the condition is not actually met even when building with msvc. See
bazelbuild/bazel#7730.

This will be tested in PR#729
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Windows Windows-specific issues and feature requests P3 We're not considering working on this, but happy to review a PR. (No assignee) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: documentation (cleanup)
Projects
None yet
Development

No branches or pull requests

7 participants