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

Allow setting platform constraints with --exec_constraints #968

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

coeuvre
Copy link
Member

@coeuvre coeuvre commented Aug 17, 2021

By default, clang is detected for cpp configs. While --cpp_env_json can be used to override the default environment variables during toolchain generation, there isn't a way to override platform constraints.

This PR adds flag --exec_constraints which accept a comma separated list of constraint values e.g. --exec_constraints=@bazel_tools//platforms:linux,@bazel_tools//platforms:x86_64,@bazel_tools//tools/cpp:gcc to override the default values.

@crackcomm
Copy link

I would be delighted to have this feature.

@coeuvre
Copy link
Member Author

coeuvre commented Oct 18, 2023

I have rebased the PR. Please review and let me know whether it's feasible to merge!

@jjmaestro
Copy link

@coeuvre I'm trying your PR and it's not overriding any of the env flags :-? When I run with

--exec_constraints=@bazel_tools//platforms:linux,@bazel_tools//platforms:x86_64,@bazel_tools//tools/cpp:gcc

I still see x86 and clang everywhere:

(...)
2024/10/03 10:54:23 Running: 'docker exec -w /workdir/cpp_configs_project \
-e USE_BAZEL_VERSION=7.3.1 \
-e ABI_LIBC_VERSION=glibc_2.19 \
-e BAZEL_TARGET_LIBC=glibc_2.19 \
-e BAZEL_TARGET_SYSTEM=x86_64-unknown-linux-gnu \
-e CC_TOOLCHAIN_NAME=linux_gnu_x86 \
-e ABI_VERSION=clang \
-e BAZEL_COMPILER=clang \
-e BAZEL_HOST_SYSTEM=i686-unknown-linux-gnu \
-e BAZEL_TARGET_CPU=k8 \
-e CC=clang \
cf841a5922ed2a0fe4772303e04280a3dff41c9eeb25facd00d05635c373afd0 \
/workdir/bazelisk build @local_config_cc//...'

Also, I can't get your PR and bazel-toolchains to work with another arch

--exec_constraints=@bazel_tools//platforms:linux,@bazel_tools//platforms:arm64,@bazel_tools//tools/cpp:gcc

it keeps trying to force x86:

(...)
2024/10/03 11:41:22 Output: 2024/10/03 10:41:08 Downloading https://releases.bazel.build/7.3.1/release/bazel-7.3.1-linux-x86_64...
rosetta error: failed to open elf at /lib64/ld-linux-x86-64.so.2

I checked the code and, if I'm not mistaken, we still need to use --cpp_env_json and make sure "everything matches" (e.g. compiler, architecture, etc)?

Maybe there's something I'm doing wrong... :-? Anyway, it would be great if you have a working example that uses --exec_constraints.

Thanks!

@jjmaestro
Copy link

OK, I checked further and... there's so many things hardcoded that I don't think this PR helps, sorry :-/

Here's how I got bazel-toolchains to work with my image:

diff --git a/pkg/rbeconfigsgen/options.go b/pkg/rbeconfigsgen/options.go
index b32965a..be90748 100644
--- a/pkg/rbeconfigsgen/options.go
+++ b/pkg/rbeconfigsgen/options.go
@@ -134,12 +134,12 @@ var (
 			PlatformParams: PlatformToolchainsTemplateParams{
 				ExecConstraints: []string{
 					"@platforms//os:linux",
-					"@platforms//cpu:x86_64",
-					"@bazel_tools//tools/cpp:clang",
+					"@platforms//cpu:arm64",
+					"@bazel_tools//tools/cpp:gcc",
 				},
 				TargetConstraints: []string{
 					"@platforms//os:linux",
-					"@platforms//cpu:x86_64",
+					"@platforms//cpu:arm64",
 				},
 				OSFamily: "Linux",
 			},
@@ -147,15 +147,15 @@ var (
 			CPPConfigRepo:    "local_config_cc",
 			CppBazelCmd:      "build",
 			CppGenEnv: map[string]string{
-				"ABI_LIBC_VERSION":    "glibc_2.19",
-				"ABI_VERSION":         "clang",
-				"BAZEL_COMPILER":      "clang",
+				"ABI_LIBC_VERSION":    "glibc_2.36",
+				"ABI_VERSION":         "gcc",
+				"BAZEL_COMPILER":      "gcc",
 				"BAZEL_HOST_SYSTEM":   "i686-unknown-linux-gnu",
 				"BAZEL_TARGET_CPU":    "k8",
-				"BAZEL_TARGET_LIBC":   "glibc_2.19",
+				"BAZEL_TARGET_LIBC":   "glibc_2.36",
 				"BAZEL_TARGET_SYSTEM": "x86_64-unknown-linux-gnu",
-				"CC":                  "clang",
-				"CC_TOOLCHAIN_NAME":   "linux_gnu_x86",
+				"CC":                  "gcc",
+				"CC_TOOLCHAIN_NAME":   "linux_gnu_arm64",
 			},
 			CPPToolchainTargetName: "cc-compiler-k8",
 		},
diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go
index b7815f9..1c9799b 100644
--- a/pkg/rbeconfigsgen/rbeconfigsgen.go
+++ b/pkg/rbeconfigsgen/rbeconfigsgen.go
@@ -262,7 +262,7 @@ func workdir(os string) string {
 func BazeliskDownloadInfo(os string) (string, string, error) {
 	switch os {
 	case OSLinux:
-		return "https://github.com/bazelbuild/bazelisk/releases/download/v1.19.0/bazelisk-linux-amd64", "bazelisk", nil
+		return "https://github.com/bazelbuild/bazelisk/releases/download/v1.19.0/bazelisk-linux-arm64", "bazelisk", nil
 	case OSWindows:
 		return "https://github.com/bazelbuild/bazelisk/releases/download/v1.19.0/bazelisk-windows-amd64.exe", "bazelisk.exe", nil
 	}

I think we should have a good way to override all of the parameters, including the arch, and avoid hardcoding it / using bazelisk if e.g. there's already Bazel in the image, etc.

@jjmaestro
Copy link

OK, I've managed to use your PR and avoid the hardcoding of x86 by having Bazel in the image and specifying bazel_path, thus avoiding the installation of Bazelisk which is hardcoded to x86.

The following works with this PR:

#!/bin/bash

build() {
    go build \
        -o rbe_configs_gen \
        ./cmd/rbe_configs_gen/rbe_configs_gen.go
}

generate() {
    local docker_image="$1"
    local bazel_version="$2"
    local target_arch="$3"
    local repo_path="$4"

    local exec_constraints=(
        "@bazel_tools//platforms:linux"
        "@bazel_tools//platforms:$target_arch"
        "@bazel_tools//tools/cpp:gcc"
    )

    GLIBC_VERSION="2.36"  # ldd --version

    cat <<- EOF >/tmp/cpp_env.json
{
    "ABI_LIBC_VERSION": "glibc_$GLIBC_VERSION",
    "ABI_VERSION": "gcc",
    "BAZEL_COMPILER": "gcc",
    "BAZEL_HOST_SYSTEM": "i686-unknown-linux-gnu",
    "BAZEL_TARGET_CPU": "k8",
    "BAZEL_TARGET_LIBC": "glibc_$GLIBC_VERSION",
    "BAZEL_TARGET_SYSTEM": "${target_arch}-unknown-linux-gnu",
    "CC": "gcc",
    "CC_TOOLCHAIN_NAME": "linux_gnu_$target_arch"
}
EOF

    # NOTE: bazel_path is needed to avoid downloading Bazelisk inside the
    # image because the tool HARDCODES THE ARCH to x86 regardless of the
    # docker platform we use...

    IFS=','
    ./rbe_configs_gen \
        --generate_cpp_configs=true \
        --generate_java_configs=false \
        --exec_os="linux" \
        --target_os="linux" \
        --toolchain_container="$docker_image" \
        --docker_platform="linux/$target_arch" \
        --bazel_version="$bazel_version" \
        --bazel_path="/usr/bin/bazel" \
        --exec_constraints="${exec_constraints[*]}" \
        --cpp_env_json=/tmp/cpp_env.json \
        --output_src_root="$repo_path"
}

"$@"

Then, using the script, I (1) build the tool locally (in my case, MacOS arm64) and (2) run rbe_configs_gen with --cpp_env_json and --exec_constraints:

$ ./runner.sh build
$ ./runner.sh generate MY_CUSTOM_IMAGE 7.3.2 arm64 /tmp/repo

@jjmaestro
Copy link

@coeuvre Also, it would be awesome if you could rebase your branch, I've tried and it rebases cleanly on top of the current HEAD :) Tkanks!!

@jjmaestro
Copy link

jjmaestro commented Oct 4, 2024

I've also noticed that TargetConstraints is hardcoded to x86_64... so I guess that would also have to be changed to truly support platform selection, e.g. via a similar flag --target_constraints :-?

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

Successfully merging this pull request may close these issues.

3 participants