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

Can we generate a toolchain config without a hardcoded toolchain target? #901

Open
sunjayBhatia opened this issue Jul 27, 2020 · 2 comments
Assignees

Comments

@sunjayBhatia
Copy link
Contributor

bazel-toolchains generates a hard coded toolchain identifier:

toolchain = "//configs/ubuntu16_04_clang/11.0.0/bazel_3.4.1/cc:cc-compiler-k8",

This works for Linux toolchains as when the toolchain config BUILD file is always generated (from here: https://github.com/bazelbuild/bazel/blob/e4e06c0293bda20ad8c2b8db131ce821316b8d12/tools/cpp/BUILD.tpl#L47-L55) with an appropriate cpu and compiler combination only the target compiler:

On Windows however, the BUILD file that is generated (from here: https://github.com/bazelbuild/bazel/blob/e4e06c0293bda20ad8c2b8db131ce821316b8d12/tools/cpp/BUILD.windows.tpl#L47-L59) is not structured specifically for the target compiler, rather it has a toolchain suite for all supported toolchain configs: https://github.com/envoyproxy/envoy-build-tools/blob/c4d2fe1a46a1e173c3dad245b4bbc772a75b5db7/toolchains/configs/windows/msvc-cl/bazel_3.4.1/cc/BUILD#L47-L59

It seems like this might just an issue of Bazel fixing the structure of the BUILD file template and how it is generated on Windows to match Linux, but is there anything we can do in bazel-toolchains in the mean time/instead? If we could take in the cpu and target compiler and use that to determine the toolchain config target that would help. Currently we are using the Bazel hack to set USE_CLANG_CL=1 in the environment to make the msvc-cl toolchain use clang-cl tools but we would rather use a properly named toolchain config etc.

Just looking for ideas/thoughts, and if you think we should focus on getting Bazel to structure its builtin C++ toolchain BUILD files differently

Also see:

def create_export_platform(ctx, exec_properties, exec_compatible_with, target_compatible_with, image_name, name, toolchain_config_spec_name, use_legacy_platform_definition):
"""Creates a BUILD file (to be exported to output_base) with the cc_toolchain and platform targets.
Args:
ctx: the Bazel context object.
exec_properties: A string->string dict containing execution properties to
be used when creating the platform. Will be used only when
use_legacy_platform_definition == False. This dict must not contain
"container-image".
exec_compatible_with: List of constraints to add to the produced
toolchain/platform targets (e.g., ["@bazel_tools//platforms:linux"] in the
exec_compatible_with/constraint_values attrs, respectively.
target_compatible_with: List of constraints to add to the produced
toolchain target (e.g., ["@bazel_tools//platforms:linux"]) in the
target_compatible_with attr.
image_name: the name of the image.
name: name of rbe_autoconfig repo rule.
toolchain_config_spec_name: name of the toolchain config spec
use_legacy_platform_definition: Whether to create a platform with
remote_execution_properties (legacy) or with exec_properties.
"""
cc_toolchain_target = "//" + ctx.attr.toolchain_config_suite_spec["output_base"]
if toolchain_config_spec_name:
cc_toolchain_target += "/" + toolchain_config_spec_name
cc_toolchain_target += "/bazel_" + ctx.attr.bazel_version
cc_toolchain_target += "/cc" + _CC_TOOLCHAIN[os_family(ctx)]
_create_platform(ctx, exec_properties, exec_compatible_with, target_compatible_with, image_name, name, cc_toolchain_target, use_legacy_platform_definition)

_CC_TOOLCHAIN = {
"Linux": ":cc-compiler-k8",
"Windows": ":cc-compiler-x64_windows",
}

@sunjayBhatia
Copy link
Contributor Author

cc @wrowe

@smukherj1
Copy link
Collaborator

My suggestion would be to point this out to the Bazel team and see if they're interested in fixing it or accepting a contribution that restructures the Windows toolchain to look like the Linux toolchain. If Bazel refuses, I would be open to considering patching up the Windows case here but I really hope it won't be necessary.

In the meantime, does continuing to use your workaround work for you? Otherwise, maybe a Skylark macro that takes the Windows toolchain target and extracts the compiler specific toolchain target from it in your repo? I'm not sure how the compiler is auto-determined/specified on Linux so I'm not sure how this macro would look like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants