From b79f0ada326b79c7513d7ed0baadb627f0a561e0 Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Thu, 27 May 2021 20:17:40 +0700 Subject: [PATCH 01/10] Allow toolchain generation on MacOS --- cmd/rbe_configs_gen/rbe_configs_gen.go | 6 +- pkg/rbeconfigsgen/docker_runner.go | 202 +++++++++++++++ pkg/rbeconfigsgen/host_runner.go | 146 +++++++++++ pkg/rbeconfigsgen/options.go | 29 ++- pkg/rbeconfigsgen/rbeconfigsgen.go | 327 +++++++------------------ pkg/rbeconfigsgen/runner.go | 50 ++++ 6 files changed, 520 insertions(+), 240 deletions(-) create mode 100644 pkg/rbeconfigsgen/docker_runner.go create mode 100644 pkg/rbeconfigsgen/host_runner.go create mode 100644 pkg/rbeconfigsgen/runner.go diff --git a/cmd/rbe_configs_gen/rbe_configs_gen.go b/cmd/rbe_configs_gen/rbe_configs_gen.go index 8a440a58f..0e5d1d642 100644 --- a/cmd/rbe_configs_gen/rbe_configs_gen.go +++ b/cmd/rbe_configs_gen/rbe_configs_gen.go @@ -29,9 +29,9 @@ import ( var ( // Mandatory input arguments. - toolchainContainer = flag.String("toolchain_container", "", "Repository path to toolchain image to generate configs for. E.g., l.gcr.io/google/rbe-ubuntu16-04:latest") - execOS = flag.String("exec_os", "", "The OS (linux|windows) of the toolchain container image a.k.a, the execution platform in Bazel.") - targetOS = flag.String("target_os", "", "The OS (linux|windows) artifacts built will target a.k.a, the target platform in Bazel.") + toolchainContainer = flag.String("toolchain_container", "", "Repository path to toolchain image to generate configs for. E.g., l.gcr.io/google/rbe-ubuntu16-04:latest. Omit if configuration should be built locally") + execOS = flag.String("exec_os", "", "The OS (linux|windows|osx) of the toolchain container image a.k.a, the execution platform in Bazel.") + targetOS = flag.String("target_os", "", "The OS (linux|windows|osx) artifacts built will target a.k.a, the target platform in Bazel.") // Optional input arguments. bazelVersion = flag.String("bazel_version", "", "(Optional) Bazel release version to generate configs for. E.g., 4.0.0. If unspecified, the latest available Bazel release is picked.") diff --git a/pkg/rbeconfigsgen/docker_runner.go b/pkg/rbeconfigsgen/docker_runner.go new file mode 100644 index 000000000..9d3991ae9 --- /dev/null +++ b/pkg/rbeconfigsgen/docker_runner.go @@ -0,0 +1,202 @@ +// Copyright 2021 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +package rbeconfigsgen + +import ( + "fmt" + "log" + "strings" +) + +// workdir returns the root working directory to use inside the toolchain container for the given +// OS where the OS refers to the OS of the toolchain container. +func workdir(os string) string { + switch os { + case OSLinux: + return "/workdir" + case OSWindows: + return "C:/workdir" + } + log.Fatalf("Invalid OS: %q", os) + return "" +} + +// dockerRunner implements interface runner +// dockerRunner allows starting a container for a given docker image and subsequently running +// arbitrary commands inside the container or extracting files from it. +// dockerRunner uses the docker client to spin up & interact with containers. +type dockerRunner struct { + // Input arguments. + // containerImage is the docker image to spin up as a running container. This could be a tagged + // or floating reference to a docker image but in a format acceptable to the docker client. + containerImage string + // stopContainer determines if the running container will be deleted once we're done with it. + stopContainer bool + + // Parameters that affect how commands are executed inside the running toolchain container. + // These parameters can be changed between calls to the execCmd function. + + // workdir is the working directory to use to run commands inside the container. + workdir string + // additionalEnv is the environment variables to set when executing commands + additionalEnv map[string]string + + // Populated by the runner. + // dockerPath is the path to the docker client. + dockerPath string + // containerID is the ID of the running docker container. + containerID string + // resolvedImage is the container image referenced by its sha256 digest. + resolvedImage string +} + +// newDockerRunner creates a new running container of the given containerImage. stopContainer +// determines if the cleanup function on the dockerRunner will stop the running container when +// called. +func newDockerRunner(containerImage string, stopContainer bool, execOS string) (*dockerRunner, error) { + if containerImage == "" { + return nil, fmt.Errorf("container image was not specified") + } + d := &dockerRunner{ + containerImage: containerImage, + stopContainer: stopContainer, + dockerPath: "docker", + } + if _, err := runCmd(d.dockerPath, "pull", d.containerImage); err != nil { + return nil, fmt.Errorf("docker was unable to pull the toolchain container image %q: %w", d.containerImage, err) + } + resolvedImage, err := runCmd(d.dockerPath, "inspect", "--format={{index .RepoDigests 0}}", d.containerImage) + if err != nil { + return nil, fmt.Errorf("failed to convert toolchain container image %q into a fully qualified image name by digest: %w", d.containerImage, err) + } + resolvedImage = strings.TrimSpace(resolvedImage) + log.Printf("Resolved toolchain image %q to fully qualified reference %q.", d.containerImage, resolvedImage) + d.resolvedImage = resolvedImage + + cid, err := runCmd(d.dockerPath, "create", "--rm", d.resolvedImage, "sleep", "infinity") + if err != nil { + return nil, fmt.Errorf("failed to create a container with the toolchain container image: %w", err) + } + cid = strings.TrimSpace(cid) + if len(cid) != 64 { + return nil, fmt.Errorf("container ID %q extracted from the stdout of the container create command had unexpected length, got %d, want 64", cid, len(cid)) + } + d.containerID = cid + log.Printf("Created container ID %v for toolchain container image %v.", d.containerID, d.resolvedImage) + if _, err := runCmd(d.dockerPath, "start", d.containerID); err != nil { + return nil, fmt.Errorf("failed to run the toolchain container: %w", err) + } + if _, err := d.execCmd("mkdir", workdir(execOS)); err != nil { + d.cleanup() + return nil, fmt.Errorf("failed to create workdir in toolchain container: %w", err) + } + d.setWorkdir(workdir(execOS)) + return d, nil +} + +// execCmd runs the given command inside the docker container and returns the output with whitespace +// trimmed from the edges. +func (d *dockerRunner) execCmd(args ...string) (string, error) { + a := []string{"exec"} + if d.workdir != "" { + a = append(a, "-w", d.workdir) + } + for _, e := range d.additionalEnv { + a = append(a, "-e", e) + } + a = append(a, d.containerID) + a = append(a, args...) + o, err := runCmd(d.dockerPath, a...) + return strings.TrimSpace(o), err +} + +// cleanup stops the running container if stopContainer was true when the dockerRunner was created. +func (d *dockerRunner) cleanup() { + if !d.stopContainer { + log.Printf("Not stopping container %v of image %v because the Cleanup option was set to false.", d.containerID, d.resolvedImage) + return + } + if _, err := runCmd(d.dockerPath, "stop", "-t", "0", d.containerID); err != nil { + log.Printf("Failed to stop container %v of toolchain image %v but it's ok to ignore this error if config generation & extraction succeeded.", d.containerID, d.resolvedImage) + } +} + +// copyTo copies the local file at 'src' to the container where 'dst' is the path inside +// the container. d.workdir has no impact on this function. +func (d *dockerRunner) copyTo(src, dst string) error { + if _, err := runCmd(d.dockerPath, "cp", src, fmt.Sprintf("%s:%s", d.containerID, dst)); err != nil { + return err + } + return nil +} + +// copyFrom extracts the file at 'src' from inside the container and copies it to the path +// 'dst' locally. d.workdir has no impact on this function. +func (d *dockerRunner) copyFrom(src, dst string) error { + if _, err := runCmd(d.dockerPath, "cp", fmt.Sprintf("%s:%s", d.containerID, src), dst); err != nil { + return err + } + return nil +} + +// getEnv gets the shell environment values from the toolchain container as determined by the +// image config. Env value set or changed by running commands after starting the container aren't +// captured by the return value of this function. +// The return value of this function is a map from env keys to their values. If the image config, +// specifies the same env key multiple times, later values supercede earlier ones. +func (d *dockerRunner) getEnv() (map[string]string, error) { + result := make(map[string]string) + o, err := runCmd(d.dockerPath, "inspect", "-f", "{{range $i, $v := .Config.Env}}{{println $v}}{{end}}", d.resolvedImage) + if err != nil { + return nil, fmt.Errorf("failed to inspect the docker image to get environment variables: %w", err) + } + split := strings.Split(o, "\n") + for _, s := range split { + s = strings.TrimSpace(s) + if len(s) == 0 { + continue + } + keyVal := strings.SplitN(s, "=", 2) + key := "" + val := "" + if len(keyVal) == 2 { + key, val = keyVal[0], keyVal[1] + } else if len(keyVal) == 1 { + // Maybe something like 'KEY=' was specified. We assume value is blank. + key = keyVal[0] + } + if len(key) == 0 { + continue + } + result[key] = val + } + return result, nil +} + +func (d *dockerRunner) getWorkdir() string { + return d.workdir +} + +func (d *dockerRunner) setWorkdir(wd string) { + d.workdir = wd +} + +func (d *dockerRunner) getAdditionalEnv() map[string]string { + return d.additionalEnv +} + +func (d *dockerRunner) setAdditionalEnv(env map[string]string) { + d.additionalEnv = env +} diff --git a/pkg/rbeconfigsgen/host_runner.go b/pkg/rbeconfigsgen/host_runner.go new file mode 100644 index 000000000..c864fff08 --- /dev/null +++ b/pkg/rbeconfigsgen/host_runner.go @@ -0,0 +1,146 @@ +// Copyright 2021 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +package rbeconfigsgen + +import ( + "fmt" + "io/ioutil" + "log" + "os" + "os/exec" + "strings" +) + +// hostRunner implements interface runner +// hostRunner allows subsequently running +// arbitrary commands directly on the host. +type hostRunner struct { + // global workdir is initial workdir + globalWorkdir string + // workdir is the working directory to use to run commands. + workdir string + + // deleteWorkdir controls if global working directory is removed during cleanup. True by default + deleteWorkdir bool + + // additionalEnv is the environment variables to set when executing commands + additionalEnv map[string]string +} + +// newHostRunner creates a new runner which executes commands directly in host environment. deleteWorkdir +// determines if the cleanup function on the hostRunner will remove temporary directory +func newHostRunner(deleteWorkdir bool) (*hostRunner, error) { + + workdir, err := ioutil.TempDir("", "host_runner_") + if err != nil { + return nil, fmt.Errorf("failed to create a temporary local directory for host runner: %w", err) + } + + return &hostRunner{ + globalWorkdir: workdir, + workdir: workdir, + deleteWorkdir: deleteWorkdir, + }, nil +} + +// execCmd runs the given command and returns the output with whitespace +// trimmed from the edges. +func (r *hostRunner) execCmd(args ...string) (string, error) { + log.Printf("Running: %s", strings.Join(args, " ")) + c := exec.Command(args[0], args[1:]...) + c.Env = append(os.Environ(), convertAdditionalEnv(r)...) + c.Dir = r.workdir + o, err := c.CombinedOutput() + if err != nil { + log.Printf("Output: %s", o) + return "", err + } + return strings.TrimSpace(string(o)), err +} + +// cleanup stops the running container if stopContainer was true when the hostRunner was created. +func (r *hostRunner) cleanup() { + if !r.deleteWorkdir { + log.Printf("Not deleting workdir %v because the Cleanup option was set to false.", r.globalWorkdir) + return + } + + if err := os.RemoveAll(r.globalWorkdir); err != nil { + log.Printf("Failed to delete %v", r.globalWorkdir) + } +} + +// copyTo copies the local file at 'src' to the container where 'dst' is the path inside +// the container. d.workdir has no impact on this function. +func (r *hostRunner) copyTo(src, dst string) error { + if _, err := runCmd("cp", src, dst); err != nil { + return err + } + return nil +} + +// copyFrom extracts the file at 'src' from inside the container and copies it to the path +// 'dst' locally. d.workdir has no impact on this function. +func (r *hostRunner) copyFrom(src, dst string) error { + if _, err := runCmd("cp", src, dst); err != nil { + return err + } + return nil +} + +// getEnv gets the shell environment values from the toolchain container as determined by the +// image config. Env value set or changed by running commands after starting the container aren't +// captured by the return value of this function. +// The return value of this function is a map from env keys to their values. If the image config, +// specifies the same env key multiple times, later values supercede earlier ones. +func (r *hostRunner) getEnv() (map[string]string, error) { + result := make(map[string]string) + for _, s := range os.Environ() { + s = strings.TrimSpace(s) + if len(s) == 0 { + continue + } + keyVal := strings.SplitN(s, "=", 2) + key := "" + val := "" + if len(keyVal) == 2 { + key, val = keyVal[0], keyVal[1] + } else if len(keyVal) == 1 { + // Maybe something like 'KEY=' was specified. We assume value is blank. + key = keyVal[0] + } + if len(key) == 0 { + continue + } + result[key] = val + } + return result, nil +} + +func (r *hostRunner) getWorkdir() string { + return r.workdir +} + +func (r *hostRunner) setWorkdir(wd string) { + r.workdir = wd +} + +func (r *hostRunner) getAdditionalEnv() map[string]string { + return r.additionalEnv +} + +func (r *hostRunner) setAdditionalEnv(env map[string]string) { + r.additionalEnv = env +} diff --git a/pkg/rbeconfigsgen/options.go b/pkg/rbeconfigsgen/options.go index e3c8f1571..78e078144 100644 --- a/pkg/rbeconfigsgen/options.go +++ b/pkg/rbeconfigsgen/options.go @@ -110,12 +110,15 @@ const ( OSLinux = "linux" // OSWindows represents Windows when selecting platforms. OSWindows = "windows" + // OSWindows represents MacOS when selecting platforms. + OSMacos = "osx" ) var ( validOS = []string{ OSLinux, OSWindows, + OSMacos, } // DefaultExecOptions is a map from the ExecOS to default values for certain fields in Options @@ -167,6 +170,30 @@ var ( CppBazelCmd: "query", CPPToolchainTargetName: "cc-compiler-x64_windows", }, + OSMacos: { + PlatformParams: PlatformToolchainsTemplateParams{ + ExecConstraints: []string{ + "@bazel_tools//platforms:osx", + "@bazel_tools//platforms:x86_64", + "@bazel_tools//tools/cpp:clang", + }, + TargetConstraints: []string{ + "@bazel_tools//platforms:osx", + "@bazel_tools//platforms:x86_64", + }, + OSFamily: "MacOS", + }, + CPPConfigTargets: []string{"@local_config_cc//...", "--", "-@local_config_cc//:osx_archs.bzl"}, + CPPConfigRepo: "local_config_cc", + CppBazelCmd: "build", + CppGenEnv: map[string]string{ + "ABI_VERSION": "clang", + "BAZEL_COMPILER": "clang", + "BAZEL_TARGET_CPU": "k8", + "CC": "clang", + }, + CPPToolchainTargetName: "cc-compiler-k8", + }, } ) @@ -222,7 +249,7 @@ func (o *Options) Validate() error { } o.BazelVersion = v } - if o.ToolchainContainer == "" { + if o.ToolchainContainer == "" && (o.ExecOS == "linux" || o.ExecOS == "windows"){ return fmt.Errorf("ToolchainContainer was not specified") } if o.ExecOS == "" { diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index fa49b62ca..3206ebd78 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -152,35 +152,6 @@ type javaBuildTemplateParams struct { JavaVersion string } -// dockerRunner allows starting a container for a given docker image and subsequently running -// arbitrary commands inside the container or extracting files from it. -// dockerRunner uses the docker client to spin up & interact with containers. -type dockerRunner struct { - // Input arguments. - // containerImage is the docker image to spin up as a running container. This could be a tagged - // or floating reference to a docker image but in a format acceptable to the docker client. - containerImage string - // stopContainer determines if the running container will be deleted once we're done with it. - stopContainer bool - - // Parameters that affect how commands are executed inside the running toolchain container. - // These parameters can be changed between calls to the execCmd function. - - // workdir is the working directory to use to run commands inside the container. - workdir string - // env is the environment variables to set when executing commands specified in the given order - // as KEY=VALUE strings. - env []string - - // Populated by the runner. - // dockerPath is the path to the docker client. - dockerPath string - // containerID is the ID of the running docker container. - containerID string - // resolvedImage is the container image referenced by its sha256 digest. - resolvedImage string -} - // generatedFile represents a file part of the toolchain configs generated by the rbeconfigsgen // package. type generatedFile struct { @@ -223,19 +194,6 @@ func runCmd(cmd string, args ...string) (string, error) { return string(o), nil } -// workdir returns the root working directory to use inside the toolchain container for the given -// OS where the OS refers to the OS of the toolchain container. -func workdir(os string) string { - switch os { - case OSLinux: - return "/workdir" - case OSWindows: - return "C:/workdir" - } - log.Fatalf("Invalid OS: %q", os) - return "" -} - // BazeliskDownloadInfo returns the URL and name of the local downloaded file to use for downloading // bazelisk for the given OS. func BazeliskDownloadInfo(os string) (string, string, error) { @@ -244,132 +202,16 @@ func BazeliskDownloadInfo(os string) (string, string, error) { return "https://github.com/bazelbuild/bazelisk/releases/download/v1.7.4/bazelisk-linux-amd64", "bazelisk", nil case OSWindows: return "https://github.com/bazelbuild/bazelisk/releases/download/v1.7.4/bazelisk-windows-amd64.exe", "bazelisk.exe", nil + case OSMacos: + return "https://github.com/bazelbuild/bazelisk/releases/download/v1.7.4/bazelisk-darwin-amd64", "bazelisk", nil } return "", "", fmt.Errorf("invalid OS %q", os) } -// newDockerRunner creates a new running container of the given containerImage. stopContainer -// determines if the cleanup function on the dockerRunner will stop the running container when -// called. -func newDockerRunner(containerImage string, stopContainer bool) (*dockerRunner, error) { - if containerImage == "" { - return nil, fmt.Errorf("container image was not specified") - } - d := &dockerRunner{ - containerImage: containerImage, - stopContainer: stopContainer, - dockerPath: "docker", - } - if _, err := runCmd(d.dockerPath, "pull", d.containerImage); err != nil { - return nil, fmt.Errorf("docker was unable to pull the toolchain container image %q: %w", d.containerImage, err) - } - resolvedImage, err := runCmd(d.dockerPath, "inspect", "--format={{index .RepoDigests 0}}", d.containerImage) - if err != nil { - return nil, fmt.Errorf("failed to convert toolchain container image %q into a fully qualified image name by digest: %w", d.containerImage, err) - } - resolvedImage = strings.TrimSpace(resolvedImage) - log.Printf("Resolved toolchain image %q to fully qualified reference %q.", d.containerImage, resolvedImage) - d.resolvedImage = resolvedImage - - cid, err := runCmd(d.dockerPath, "create", "--rm", d.resolvedImage, "sleep", "infinity") - if err != nil { - return nil, fmt.Errorf("failed to create a container with the toolchain container image: %w", err) - } - cid = strings.TrimSpace(cid) - if len(cid) != 64 { - return nil, fmt.Errorf("container ID %q extracted from the stdout of the container create command had unexpected length, got %d, want 64", cid, len(cid)) - } - d.containerID = cid - log.Printf("Created container ID %v for toolchain container image %v.", d.containerID, d.resolvedImage) - if _, err := runCmd(d.dockerPath, "start", d.containerID); err != nil { - return nil, fmt.Errorf("failed to run the toolchain container: %w", err) - } - return d, nil -} - -// execCmd runs the given command inside the docker container and returns the output with whitespace -// trimmed from the edges. -func (d *dockerRunner) execCmd(args ...string) (string, error) { - a := []string{"exec"} - if d.workdir != "" { - a = append(a, "-w", d.workdir) - } - for _, e := range d.env { - a = append(a, "-e", e) - } - a = append(a, d.containerID) - a = append(a, args...) - o, err := runCmd(d.dockerPath, a...) - return strings.TrimSpace(o), err -} - -// cleanup stops the running container if stopContainer was true when the dockerRunner was created. -func (d *dockerRunner) cleanup() { - if !d.stopContainer { - log.Printf("Not stopping container %v of image %v because the Cleanup option was set to false.", d.containerID, d.resolvedImage) - return - } - if _, err := runCmd(d.dockerPath, "stop", "-t", "0", d.containerID); err != nil { - log.Printf("Failed to stop container %v of toolchain image %v but it's ok to ignore this error if config generation & extraction succeeded.", d.containerID, d.resolvedImage) - } -} - -// copyToContainer copies the local file at 'src' to the container where 'dst' is the path inside -// the container. d.workdir has no impact on this function. -func (d *dockerRunner) copyToContainer(src, dst string) error { - if _, err := runCmd(d.dockerPath, "cp", src, fmt.Sprintf("%s:%s", d.containerID, dst)); err != nil { - return err - } - return nil -} - -// copyFromContainer extracts the file at 'src' from inside the container and copies it to the path -// 'dst' locally. d.workdir has no impact on this function. -func (d *dockerRunner) copyFromContainer(src, dst string) error { - if _, err := runCmd(d.dockerPath, "cp", fmt.Sprintf("%s:%s", d.containerID, src), dst); err != nil { - return err - } - return nil -} - -// getEnv gets the shell environment values from the toolchain container as determined by the -// image config. Env value set or changed by running commands after starting the container aren't -// captured by the return value of this function. -// The return value of this function is a map from env keys to their values. If the image config, -// specifies the same env key multiple times, later values supercede earlier ones. -func (d *dockerRunner) getEnv() (map[string]string, error) { - result := make(map[string]string) - o, err := runCmd(d.dockerPath, "inspect", "-f", "{{range $i, $v := .Config.Env}}{{println $v}}{{end}}", d.resolvedImage) - if err != nil { - return nil, fmt.Errorf("failed to inspect the docker image to get environment variables: %w", err) - } - split := strings.Split(o, "\n") - for _, s := range split { - s = strings.TrimSpace(s) - if len(s) == 0 { - continue - } - keyVal := strings.SplitN(s, "=", 2) - key := "" - val := "" - if len(keyVal) == 2 { - key, val = keyVal[0], keyVal[1] - } else if len(keyVal) == 1 { - // Maybe something like 'KEY=' was specified. We assume value is blank. - key = keyVal[0] - } - if len(key) == 0 { - continue - } - result[key] = val - } - return result, nil -} - // installBazelisk downloads bazelisk locally to the specified directory for the given os and copies // it into the running toolchain container. // Returns the path Bazelisk was installed to inside the running toolchain container. -func installBazelisk(d *dockerRunner, downloadDir, execOS string) (string, error) { +func installBazelisk(r runner, downloadDir, execOS string) (string, error) { url, filename, err := BazeliskDownloadInfo(execOS) if err != nil { return "", fmt.Errorf("unable to determine how to download Bazelisk for execution OS %q: %w", execOS, err) @@ -389,22 +231,23 @@ func installBazelisk(d *dockerRunner, downloadDir, execOS string) (string, error return "", fmt.Errorf("error while downloading Bazelisk to %s: %w", localPath, err) } - bazeliskContainerPath := path.Join(d.workdir, filename) - if err := d.copyToContainer(localPath, bazeliskContainerPath); err != nil { - return "", fmt.Errorf("failed to copy the downloaded Bazelisk binary into the container: %w", err) + bazeliskRunnerPath := path.Join(r.getWorkdir(), filename) + log.Printf("Copying %s to %s", localPath, bazeliskRunnerPath) + if err := r.copyTo(localPath, bazeliskRunnerPath); err != nil { + return "", fmt.Errorf("failed to copy the downloaded Bazelisk binary: %w", err) } - if _, err := d.execCmd("chmod", "+x", bazeliskContainerPath); err != nil { + if _, err := r.execCmd("chmod", "+x", bazeliskRunnerPath); err != nil { return "", fmt.Errorf("failed to mark the Bazelisk binary as executable inside the container: %w", err) } - return bazeliskContainerPath, nil + return bazeliskRunnerPath, nil } // appendCppEnv appends environment variables set in the C++ environment map as well as variables // specified in the C++ environment JSON file to the given environment as "key=value". -func appendCppEnv(env []string, o *Options) ([]string, error) { +func appendCppEnv(env map[string]string, o *Options) (map[string]string, error) { for k, v := range o.CppGenEnv { - env = append(env, fmt.Sprintf("%s=%s", k, v)) + env[k] = v } if len(o.CppGenEnvJSON) == 0 { @@ -422,7 +265,7 @@ func appendCppEnv(env []string, o *Options) ([]string, error) { } for k, v := range e { - env = append(env, fmt.Sprintf("%s=%s", k, v)) + env[k] = v } return env, nil @@ -432,36 +275,32 @@ func appendCppEnv(env []string, o *Options) ([]string, error) { // given docker runner according to the given options. bazeliskPath is the path to the bazelisk // binary inside the running toolchain container. // The return value is the path to the C++ configs tarball copied out of the toolchain container. -func genCppConfigs(d *dockerRunner, o *Options, bazeliskPath string) (string, error) { +func genCppConfigs(r runner, o *Options, bazeliskPath string) (string, error) { if !o.GenCPPConfigs { return "", nil } // Change the working directory to a dedicated empty directory for C++ configs for each // command we run in this function. - cppProjDir := path.Join(d.workdir, "cpp_configs_project") - if _, err := d.execCmd("mkdir", cppProjDir); err != nil { + cppProjDir := path.Join(r.getWorkdir(), "cpp_configs_project") + if _, err := r.execCmd("mkdir", cppProjDir); err != nil { return "", fmt.Errorf("failed to create empty directory %q inside the toolchain container: %w", cppProjDir, err) } - oldWorkDir := d.workdir - d.workdir = cppProjDir - defer func() { - d.workdir = oldWorkDir - }() + oldWorkDir := r.getWorkdir() + r.setWorkdir(cppProjDir) + defer r.setWorkdir(oldWorkDir) - if _, err := d.execCmd("touch", "WORKSPACE", "BUILD.bazel"); err != nil { + if _, err := r.execCmd("touch", "WORKSPACE", "BUILD.bazel"); err != nil { return "", fmt.Errorf("failed to create empty build & workspace files in the container to initialize a blank Bazel repository: %w", err) } // Backup the current environment & restore it before returning. - oldEnv := d.env - defer func() { - d.env = oldEnv - }() + oldEnv := r.getAdditionalEnv() + defer r.setAdditionalEnv(oldEnv) // Create a new environment for bazelisk commands used to specify the Bazel version to use to // Bazelisk. - bazeliskEnv := []string{fmt.Sprintf("USE_BAZEL_VERSION=%s", o.BazelVersion)} + bazeliskEnv := map[string]string{"USE_BAZEL_VERSION": o.BazelVersion} // Add the environment variables needed for the generation only and remove them immediately // because they aren't necessary for the config extraction and add unnecessary noise to the // logs. @@ -469,20 +308,20 @@ func genCppConfigs(d *dockerRunner, o *Options, bazeliskPath string) (string, er if err != nil { return "", fmt.Errorf("failed to add additional environment variables to the C++ config generation docker command: %w", err) } - d.env = generationEnv + r.setAdditionalEnv(generationEnv) cmd := []string{ bazeliskPath, o.CppBazelCmd, } cmd = append(cmd, o.CPPConfigTargets...) - if _, err := d.execCmd(cmd...); err != nil { + if _, err := r.execCmd(cmd...); err != nil { return "", fmt.Errorf("Bazel was unable to build the C++ config generation targets in the toolchain container: %w", err) } // Restore the env needed for Bazelisk. - d.env = bazeliskEnv - bazelOutputRoot, err := d.execCmd(bazeliskPath, "info", "output_base") + r.setAdditionalEnv(bazeliskEnv) + bazelOutputRoot, err := r.execCmd(bazeliskPath, "info", "output_base") if err != nil { return "", fmt.Errorf("unable to determine the build output directory where Bazel produced C++ configs in the toolchain container: %w", err) } @@ -491,7 +330,7 @@ func genCppConfigs(d *dockerRunner, o *Options, bazeliskPath string) (string, er // Restore the old env now that we're done with Bazelisk commands. This is purely to reduce // noise in the logs. - d.env = oldEnv + r.setAdditionalEnv(oldEnv) // 1. Get a list of symlinks in the config output directory. // 2. Harden each link. @@ -499,9 +338,9 @@ func genCppConfigs(d *dockerRunner, o *Options, bazeliskPath string) (string, er // 4. Copy the tarball from the container to the local temp directory. var out string if o.ExecOS == "windows" { - out, err = d.execCmd("cmd", "/r", "dir", filepath.Clean(cppConfigDir), "/a:l", "/b") + out, err = r.execCmd("cmd", "/r", "dir", filepath.Clean(cppConfigDir), "/a:l", "/b") } else { - out, err = d.execCmd("find", cppConfigDir, "-type", "l") + out, err = r.execCmd("find", cppConfigDir, "-type", "l") } if err != nil { errMsg := fmt.Sprintf("unable to list symlinks in the C++ config generation build output directory: ") @@ -520,11 +359,11 @@ func genCppConfigs(d *dockerRunner, o *Options, bazeliskPath string) (string, er if s == "" { continue } - resolvedPath, err := d.execCmd("readlink", s) + resolvedPath, err := r.execCmd("readlink", s) if err != nil { return "", fmt.Errorf("unable to determine what the symlink %q in %q in the toolchain container points to: %w", s, cppConfigDir, err) } - if _, err := d.execCmd("ln", "-f", resolvedPath, s); err != nil { + if _, err := r.execCmd("ln", "-f", resolvedPath, s); err != nil { return "", fmt.Errorf("failed to harden symlink %q in %q pointing to %q: %w", s, cppConfigDir, resolvedPath, err) } } @@ -533,10 +372,10 @@ func genCppConfigs(d *dockerRunner, o *Options, bazeliskPath string) (string, er // Explicitly use absolute paths to avoid confusion on what's the working directory. outputTarballPath := path.Join(o.TempWorkDir, outputTarball) outputTarballContainerPath := path.Join(cppProjDir, outputTarball) - if _, err := d.execCmd("tar", "-cf", outputTarballContainerPath, "-C", cppConfigDir, "."); err != nil { + if _, err := r.execCmd("tar", "-cf", outputTarballContainerPath, "-C", cppConfigDir, "."); err != nil { return "", fmt.Errorf("failed to archive the C++ configs into a tarball inside the toolchain container: %w", err) } - if err := d.copyFromContainer(outputTarballContainerPath, outputTarballPath); err != nil { + if err := r.copyFrom(outputTarballContainerPath, outputTarballPath); err != nil { return "", fmt.Errorf("failed to copy the C++ config tarball out of the toolchain container: %w", err) } log.Printf("Generated C++ configs at %s.", outputTarballPath) @@ -562,28 +401,25 @@ func UsesLocalJavaRuntime(bazelVersion string) (bool, error) { // 1. Value of the JAVA_HOME environment variable set in the toolchain image. // 2. Value of the Java version as reported by the java binary installed in JAVA_HOME inside the // running toolchain container. -func genJavaConfigs(d *dockerRunner, o *Options) (generatedFile, error) { +func genJavaConfigs(r runner, o *Options) (generatedFile, error) { if !o.GenJavaConfigs { return generatedFile{}, nil } - imageEnv, err := d.getEnv() + javaBin := "java" + imageEnv, err := r.getEnv() if err != nil { return generatedFile{}, fmt.Errorf("unable to get the environment of the toolchain image to determine JAVA_HOME: %w", err) } javaHome, ok := imageEnv["JAVA_HOME"] - if !ok { - return generatedFile{}, fmt.Errorf("toolchain image didn't specify environment value JAVA_HOME") - } - if len(javaHome) == 0 { - return generatedFile{}, fmt.Errorf("the value of the JAVA_HOME environment variable was blank in the toolchain image") + if ok && len(javaHome) > 0 { + log.Printf("JAVA_HOME was %q.", javaHome) + javaBin = path.Join(javaHome, "bin/java") } - log.Printf("JAVA_HOME was %q.", javaHome) - javaBin := path.Join(javaHome, "bin/java") // "-XshowSettings:properties" is actually what makes java output the version string we're // looking for in a more deterministic format. "-version" is just a placeholder so that the // command doesn't error out. Although it will likely print the same version string but with // some non-deterministic prefix. - out, err := d.execCmd(javaBin, "-XshowSettings:properties", "-version") + out, err := r.execCmd(javaBin, "-XshowSettings:properties", "-version") if err != nil { return generatedFile{}, fmt.Errorf("unable to determine the Java version installed in the toolchain container: %w", err) } @@ -597,10 +433,12 @@ func genJavaConfigs(d *dockerRunner, o *Options) (generatedFile, error) { } key := strings.TrimSpace(splitVersion[0]) val := strings.TrimSpace(splitVersion[1]) - if key != "java.version" { - continue + if key == "java.version" { + javaVersion = val + } + if key == "java.home" && len(javaHome) == 0 { + javaHome = val } - javaVersion = val } if len(javaVersion) == 0 { return generatedFile{}, fmt.Errorf("unable to determine the java version installed in the container by running 'java -XshowSettings:properties' in the container because it didn't return a line that looked like java.version = ") @@ -701,7 +539,7 @@ func copyCppConfigsToTarball(inTarPath string, outTar *tar.Writer) error { switch h.Typeflag { case tar.TypeDir: break - case tar.TypeReg: + case tar.TypeReg, tar.TypeLink: if strings.HasSuffix(h.Name, "WORKSPACE") { break } @@ -802,22 +640,36 @@ func copyCppConfigsToOutputDir(outDir string, cppConfigsTarball string) error { if err != nil { return fmt.Errorf("error while reading input tarball %q: %w", cppConfigsTarball, err) } - if h.Typeflag != tar.TypeReg { + switch h.Typeflag { + case tar.TypeReg: + filePath := path.Join(outDir, h.Name) + dirPath := path.Dir(filePath) + if err := os.MkdirAll(dirPath, os.ModePerm); err != nil { + return fmt.Errorf("unable to create directory %q to extract file %q from the C++ config tarball %q: %w", dirPath, h.Name, cppConfigsTarball, err) + } + o, err := os.Create(filePath) + if err != nil { + return fmt.Errorf("failed to create file %q for writing %q from the C++ config tarball %q: %w", filePath, h.Name, cppConfigsTarball, err) + } + if _, err := io.Copy(o, inTar); err != nil { + return fmt.Errorf("error while extracting %q from %q to %q: %w", h.Name, cppConfigsTarball, filePath, err) + } + o.Close() + + case tar.TypeLink: + filePath := path.Join(outDir, h.Name) + dirPath := path.Dir(filePath) + if err := os.MkdirAll(dirPath, os.ModePerm); err != nil { + return fmt.Errorf("unable to create directory %q to extract symlink %q from the C++ config tarball %q: %w", dirPath, h.Name, cppConfigsTarball, err) + } + err := os.Symlink(h.Linkname, filePath) + if err != nil { + return fmt.Errorf("failed to create symlink %q to %q from the C++ config tarball %q: %w", filePath, h.Linkname, cppConfigsTarball, err) + } + + default: continue } - filePath := path.Join(outDir, h.Name) - dirPath := path.Dir(filePath) - if err := os.MkdirAll(dirPath, os.ModePerm); err != nil { - return fmt.Errorf("unable to create directory %q to extract file %q from the C++ config tarball %q: %w", dirPath, h.Name, cppConfigsTarball, err) - } - o, err := os.Create(filePath) - if err != nil { - return fmt.Errorf("failed to create file %q for writing %q from the C++ config tarball %q: %w", filePath, h.Name, cppConfigsTarball, err) - } - if _, err := io.Copy(o, inTar); err != nil { - return fmt.Errorf("error while extracting %q from %q to %q: %w", h.Name, cppConfigsTarball, filePath, err) - } - o.Close() } return nil } @@ -979,29 +831,33 @@ func Run(o Options) error { if err := processTempDir(&o); err != nil { return fmt.Errorf("unable to initialize a local temporary working directory to store intermediate files: %w", err) } - d, err := newDockerRunner(o.ToolchainContainer, o.Cleanup) + + var r runner + var err error + + if o.ExecOS == OSMacos { + r, err = newHostRunner(o.Cleanup) + } else { + r, err = newDockerRunner(o.ToolchainContainer, o.Cleanup, o.ExecOS) + o.PlatformParams.ToolchainContainer = r.(*dockerRunner).resolvedImage + } + if err != nil { return fmt.Errorf("failed to initialize a docker container: %w", err) } - defer d.cleanup() + defer r.cleanup() - o.PlatformParams.ToolchainContainer = d.resolvedImage - if _, err := d.execCmd("mkdir", workdir(o.ExecOS)); err != nil { - return fmt.Errorf("failed to create an empty working directory in the container") - } - d.workdir = workdir(o.ExecOS) - - bazeliskPath, err := installBazelisk(d, o.TempWorkDir, o.ExecOS) + bazeliskPath, err := installBazelisk(r, o.TempWorkDir, o.ExecOS) if err != nil { return fmt.Errorf("failed to install Bazelisk into the toolchain container: %w", err) } - cppConfigsTarball, err := genCppConfigs(d, &o, bazeliskPath) + cppConfigsTarball, err := genCppConfigs(r, &o, bazeliskPath) if err != nil { return fmt.Errorf("failed to generate C++ configs: %w", err) } - javaBuild, err := genJavaConfigs(d, &o) + javaBuild, err := genJavaConfigs(r, &o) if err != nil { return fmt.Errorf("failed to extract information about the installed JDK version in the toolchain container needed to generate Java configs: %w", err) } @@ -1025,14 +881,13 @@ func Run(o Options) error { } if err := createManifest(&o); err != nil { - return fmt.Errorf("unable to create the manifest file: %w", err) - } + return fmt.Errorf("unable to create the manifest file: %w", err) + } if o.Cleanup { if err := os.RemoveAll(o.TempWorkDir); err != nil { log.Printf("Warning: Unable to delete temporary working directory %q: %v", o.TempWorkDir, err) } } - return nil } diff --git a/pkg/rbeconfigsgen/runner.go b/pkg/rbeconfigsgen/runner.go new file mode 100644 index 000000000..d12518880 --- /dev/null +++ b/pkg/rbeconfigsgen/runner.go @@ -0,0 +1,50 @@ +// Copyright 2021 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +package rbeconfigsgen + +import "fmt" + +type runner interface { + // execCmd runs the given command in runner env and returns the output with whitespace + // trimmed from the edges. + execCmd(args ...string) (string, error) + + // copyTo copies the local file at 'src' to 'dst' in the runner env. + copyTo(src, dst string) error + + // copyFrom extracts the file at 'src' from runner and copies it to the path 'dst' locally. + copyFrom(src, dst string) error + + cleanup() + + // getEnv gets the shell environment values from the runner + getEnv() (map[string]string, error) + + setAdditionalEnv(map[string]string) + + getAdditionalEnv() map[string]string + + getWorkdir() string + + setWorkdir(string) +} + +func convertAdditionalEnv(r runner) []string { + addEnv := []string{} + for k, v := range r.getAdditionalEnv() { + addEnv = append(addEnv, fmt.Sprintf("%s=%s", k, v)) + } + return addEnv +} \ No newline at end of file From a76512e6033221b6a35c05cec701b12d5e4095c6 Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Fri, 28 May 2021 17:57:35 +0700 Subject: [PATCH 02/10] add separate package for Runner --- cmd/rbe_configs_gen/rbe_configs_gen.go | 5 +- pkg/{rbeconfigsgen => options}/options.go | 21 ++- pkg/rbeconfigsgen/rbeconfigsgen.go | 122 +++++++----------- pkg/rbeconfigsgen/rbeconfigsgen_test.go | 15 ++- .../docker_runner.go | 69 +++++----- pkg/{rbeconfigsgen => runner}/host_runner.go | 30 ++--- pkg/runner/runcmd.go | 23 ++++ pkg/{rbeconfigsgen => runner}/runner.go | 34 ++--- tests/scripts/configs_e2e/configs_e2e.go | 3 +- 9 files changed, 168 insertions(+), 154 deletions(-) rename pkg/{rbeconfigsgen => options}/options.go (94%) rename pkg/{rbeconfigsgen => runner}/docker_runner.go (78%) rename pkg/{rbeconfigsgen => runner}/host_runner.go (82%) create mode 100644 pkg/runner/runcmd.go rename pkg/{rbeconfigsgen => runner}/runner.go (55%) diff --git a/cmd/rbe_configs_gen/rbe_configs_gen.go b/cmd/rbe_configs_gen/rbe_configs_gen.go index 0e5d1d642..32e9ff20b 100644 --- a/cmd/rbe_configs_gen/rbe_configs_gen.go +++ b/cmd/rbe_configs_gen/rbe_configs_gen.go @@ -20,6 +20,7 @@ import ( "context" "flag" "fmt" + "github.com/bazelbuild/bazel-toolchains/pkg/options" "log" "os" @@ -123,7 +124,7 @@ func initMonitoringClient(ctx context.Context) (*monitoring.Client, error) { // genConfigs is just a wrapper for the config generation code so that the caller can report // results if monitoring is enabled before exiting. -func genConfigs(o rbeconfigsgen.Options) error { +func genConfigs(o options.Options) error { if err := o.ApplyDefaults(o.ExecOS); err != nil { return fmt.Errorf("failed to apply default options for OS name %q specified to --exec_os: %w", *execOS, err) } @@ -146,7 +147,7 @@ func main() { log.Fatalf("Failed to initialize monitoring: %v", err) } - o := rbeconfigsgen.Options{ + o := options.Options{ BazelVersion: *bazelVersion, ToolchainContainer: *toolchainContainer, ExecOS: *execOS, diff --git a/pkg/rbeconfigsgen/options.go b/pkg/options/options.go similarity index 94% rename from pkg/rbeconfigsgen/options.go rename to pkg/options/options.go index 78e078144..65c50e6a4 100644 --- a/pkg/rbeconfigsgen/options.go +++ b/pkg/options/options.go @@ -14,7 +14,7 @@ // // Package rbeconfigsgen contains utilities to generate C++ & Java Toolchain configs for Bazel to be // used to run RBE builds -package rbeconfigsgen +package options import ( "fmt" @@ -26,6 +26,21 @@ import ( "github.com/bazelbuild/bazelisk/repositories" ) +// PlatformToolchainsTemplateParams is used as the input to the toolchains & platform BUILD file +// template 'platformsToolchainBuildTemplate'. +type PlatformToolchainsTemplateParams struct { + ExecConstraints []string + TargetConstraints []string + CppToolchainTarget string + ToolchainContainer string + OSFamily string +} + +func (p PlatformToolchainsTemplateParams) String() string { + return fmt.Sprintf("{ExecConstraints: %v, TargetConstraints: %v, CppToolchainTarget: %q, ToolchainContainer: %q, OSFamily: %q}", + p.ExecConstraints, p.TargetConstraints, p.CppToolchainTarget, p.ToolchainContainer, p.OSFamily) +} + // Options are the options to tweak Bazel C++/Java Toolchain config generation. type Options struct { // BazelVersion is the version of Bazel to generate configs for. If unset, the latest Bazel @@ -189,10 +204,10 @@ var ( CppGenEnv: map[string]string{ "ABI_VERSION": "clang", "BAZEL_COMPILER": "clang", - "BAZEL_TARGET_CPU": "k8", + "BAZEL_TARGET_CPU": "darwin_x86_64", "CC": "clang", }, - CPPToolchainTargetName: "cc-compiler-k8", + CPPToolchainTargetName: "cc-compiler-darwin_x86_64", }, } ) diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index 3206ebd78..b0543a769 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -22,12 +22,13 @@ import ( "encoding/hex" "encoding/json" "fmt" + "github.com/bazelbuild/bazel-toolchains/pkg/options" + "github.com/bazelbuild/bazel-toolchains/pkg/runner" "io" "io/ioutil" "log" "net/http" "os" - "os/exec" "path" "path/filepath" "regexp" @@ -131,21 +132,6 @@ local_java_runtime( licenseBlob []byte ) -// PlatformToolchainsTemplateParams is used as the input to the toolchains & platform BUILD file -// template 'platformsToolchainBuildTemplate'. -type PlatformToolchainsTemplateParams struct { - ExecConstraints []string - TargetConstraints []string - CppToolchainTarget string - ToolchainContainer string - OSFamily string -} - -func (p PlatformToolchainsTemplateParams) String() string { - return fmt.Sprintf("{ExecConstraints: %v, TargetConstraints: %v, CppToolchainTarget: %q, ToolchainContainer: %q, OSFamily: %q}", - p.ExecConstraints, p.TargetConstraints, p.CppToolchainTarget, p.ToolchainContainer, p.OSFamily) -} - // javaBuildTemplateParams is used as the input to the Java toolchains BUILD file template. type javaBuildTemplateParams struct { JavaHome string @@ -180,29 +166,15 @@ type outputConfigs struct { javaBuild generatedFile } -// runCmd runs an arbitrary command in a shell, logs the exact command that was run and returns -// the generated stdout/stderr. If the command fails, the stdout/stderr is always logged. -func runCmd(cmd string, args ...string) (string, error) { - cmdStr := fmt.Sprintf("'%s'", strings.Join(append([]string{cmd}, args...), " ")) - log.Printf("Running: %s", cmdStr) - c := exec.Command(cmd, args...) - o, err := c.CombinedOutput() - if err != nil { - log.Printf("Output: %s", o) - return "", err - } - return string(o), nil -} - // BazeliskDownloadInfo returns the URL and name of the local downloaded file to use for downloading // bazelisk for the given OS. func BazeliskDownloadInfo(os string) (string, string, error) { switch os { - case OSLinux: + case options.OSLinux: return "https://github.com/bazelbuild/bazelisk/releases/download/v1.7.4/bazelisk-linux-amd64", "bazelisk", nil - case OSWindows: + case options.OSWindows: return "https://github.com/bazelbuild/bazelisk/releases/download/v1.7.4/bazelisk-windows-amd64.exe", "bazelisk.exe", nil - case OSMacos: + case options.OSMacos: return "https://github.com/bazelbuild/bazelisk/releases/download/v1.7.4/bazelisk-darwin-amd64", "bazelisk", nil } return "", "", fmt.Errorf("invalid OS %q", os) @@ -211,7 +183,7 @@ func BazeliskDownloadInfo(os string) (string, string, error) { // installBazelisk downloads bazelisk locally to the specified directory for the given os and copies // it into the running toolchain container. // Returns the path Bazelisk was installed to inside the running toolchain container. -func installBazelisk(r runner, downloadDir, execOS string) (string, error) { +func installBazelisk(r runner.Runner, downloadDir, execOS string) (string, error) { url, filename, err := BazeliskDownloadInfo(execOS) if err != nil { return "", fmt.Errorf("unable to determine how to download Bazelisk for execution OS %q: %w", execOS, err) @@ -231,13 +203,13 @@ func installBazelisk(r runner, downloadDir, execOS string) (string, error) { return "", fmt.Errorf("error while downloading Bazelisk to %s: %w", localPath, err) } - bazeliskRunnerPath := path.Join(r.getWorkdir(), filename) + bazeliskRunnerPath := path.Join(r.GetWorkdir(), filename) log.Printf("Copying %s to %s", localPath, bazeliskRunnerPath) - if err := r.copyTo(localPath, bazeliskRunnerPath); err != nil { + if err := r.CopyTo(localPath, bazeliskRunnerPath); err != nil { return "", fmt.Errorf("failed to copy the downloaded Bazelisk binary: %w", err) } - if _, err := r.execCmd("chmod", "+x", bazeliskRunnerPath); err != nil { + if _, err := r.ExecCmd("chmod", "+x", bazeliskRunnerPath); err != nil { return "", fmt.Errorf("failed to mark the Bazelisk binary as executable inside the container: %w", err) } return bazeliskRunnerPath, nil @@ -245,7 +217,7 @@ func installBazelisk(r runner, downloadDir, execOS string) (string, error) { // appendCppEnv appends environment variables set in the C++ environment map as well as variables // specified in the C++ environment JSON file to the given environment as "key=value". -func appendCppEnv(env map[string]string, o *Options) (map[string]string, error) { +func appendCppEnv(env map[string]string, o *options.Options) (map[string]string, error) { for k, v := range o.CppGenEnv { env[k] = v } @@ -275,28 +247,28 @@ func appendCppEnv(env map[string]string, o *Options) (map[string]string, error) // given docker runner according to the given options. bazeliskPath is the path to the bazelisk // binary inside the running toolchain container. // The return value is the path to the C++ configs tarball copied out of the toolchain container. -func genCppConfigs(r runner, o *Options, bazeliskPath string) (string, error) { +func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (string, error) { if !o.GenCPPConfigs { return "", nil } // Change the working directory to a dedicated empty directory for C++ configs for each // command we run in this function. - cppProjDir := path.Join(r.getWorkdir(), "cpp_configs_project") - if _, err := r.execCmd("mkdir", cppProjDir); err != nil { + cppProjDir := path.Join(r.GetWorkdir(), "cpp_configs_project") + if _, err := r.ExecCmd("mkdir", cppProjDir); err != nil { return "", fmt.Errorf("failed to create empty directory %q inside the toolchain container: %w", cppProjDir, err) } - oldWorkDir := r.getWorkdir() - r.setWorkdir(cppProjDir) - defer r.setWorkdir(oldWorkDir) + oldWorkDir := r.GetWorkdir() + r.SetWorkdir(cppProjDir) + defer r.SetWorkdir(oldWorkDir) - if _, err := r.execCmd("touch", "WORKSPACE", "BUILD.bazel"); err != nil { + if _, err := r.ExecCmd("touch", "WORKSPACE", "BUILD.bazel"); err != nil { return "", fmt.Errorf("failed to create empty build & workspace files in the container to initialize a blank Bazel repository: %w", err) } // Backup the current environment & restore it before returning. - oldEnv := r.getAdditionalEnv() - defer r.setAdditionalEnv(oldEnv) + oldEnv := r.GetAdditionalEnv() + defer r.SetAdditionalEnv(oldEnv) // Create a new environment for bazelisk commands used to specify the Bazel version to use to // Bazelisk. @@ -308,20 +280,20 @@ func genCppConfigs(r runner, o *Options, bazeliskPath string) (string, error) { if err != nil { return "", fmt.Errorf("failed to add additional environment variables to the C++ config generation docker command: %w", err) } - r.setAdditionalEnv(generationEnv) + r.SetAdditionalEnv(generationEnv) cmd := []string{ bazeliskPath, o.CppBazelCmd, } cmd = append(cmd, o.CPPConfigTargets...) - if _, err := r.execCmd(cmd...); err != nil { + if _, err := r.ExecCmd(cmd...); err != nil { return "", fmt.Errorf("Bazel was unable to build the C++ config generation targets in the toolchain container: %w", err) } // Restore the env needed for Bazelisk. - r.setAdditionalEnv(bazeliskEnv) - bazelOutputRoot, err := r.execCmd(bazeliskPath, "info", "output_base") + r.SetAdditionalEnv(bazeliskEnv) + bazelOutputRoot, err := r.ExecCmd(bazeliskPath, "info", "output_base") if err != nil { return "", fmt.Errorf("unable to determine the build output directory where Bazel produced C++ configs in the toolchain container: %w", err) } @@ -330,7 +302,7 @@ func genCppConfigs(r runner, o *Options, bazeliskPath string) (string, error) { // Restore the old env now that we're done with Bazelisk commands. This is purely to reduce // noise in the logs. - r.setAdditionalEnv(oldEnv) + r.SetAdditionalEnv(oldEnv) // 1. Get a list of symlinks in the config output directory. // 2. Harden each link. @@ -338,9 +310,9 @@ func genCppConfigs(r runner, o *Options, bazeliskPath string) (string, error) { // 4. Copy the tarball from the container to the local temp directory. var out string if o.ExecOS == "windows" { - out, err = r.execCmd("cmd", "/r", "dir", filepath.Clean(cppConfigDir), "/a:l", "/b") + out, err = r.ExecCmd("cmd", "/r", "dir", filepath.Clean(cppConfigDir), "/a:l", "/b") } else { - out, err = r.execCmd("find", cppConfigDir, "-type", "l") + out, err = r.ExecCmd("find", cppConfigDir, "-type", "l") } if err != nil { errMsg := fmt.Sprintf("unable to list symlinks in the C++ config generation build output directory: ") @@ -359,11 +331,11 @@ func genCppConfigs(r runner, o *Options, bazeliskPath string) (string, error) { if s == "" { continue } - resolvedPath, err := r.execCmd("readlink", s) + resolvedPath, err := r.ExecCmd("readlink", s) if err != nil { return "", fmt.Errorf("unable to determine what the symlink %q in %q in the toolchain container points to: %w", s, cppConfigDir, err) } - if _, err := r.execCmd("ln", "-f", resolvedPath, s); err != nil { + if _, err := r.ExecCmd("ln", "-f", resolvedPath, s); err != nil { return "", fmt.Errorf("failed to harden symlink %q in %q pointing to %q: %w", s, cppConfigDir, resolvedPath, err) } } @@ -372,10 +344,10 @@ func genCppConfigs(r runner, o *Options, bazeliskPath string) (string, error) { // Explicitly use absolute paths to avoid confusion on what's the working directory. outputTarballPath := path.Join(o.TempWorkDir, outputTarball) outputTarballContainerPath := path.Join(cppProjDir, outputTarball) - if _, err := r.execCmd("tar", "-cf", outputTarballContainerPath, "-C", cppConfigDir, "."); err != nil { + if _, err := r.ExecCmd("tar", "-cf", outputTarballContainerPath, "-C", cppConfigDir, "."); err != nil { return "", fmt.Errorf("failed to archive the C++ configs into a tarball inside the toolchain container: %w", err) } - if err := r.copyFrom(outputTarballContainerPath, outputTarballPath); err != nil { + if err := r.CopyFrom(outputTarballContainerPath, outputTarballPath); err != nil { return "", fmt.Errorf("failed to copy the C++ config tarball out of the toolchain container: %w", err) } log.Printf("Generated C++ configs at %s.", outputTarballPath) @@ -401,12 +373,12 @@ func UsesLocalJavaRuntime(bazelVersion string) (bool, error) { // 1. Value of the JAVA_HOME environment variable set in the toolchain image. // 2. Value of the Java version as reported by the java binary installed in JAVA_HOME inside the // running toolchain container. -func genJavaConfigs(r runner, o *Options) (generatedFile, error) { +func genJavaConfigs(r runner.Runner, o *options.Options) (generatedFile, error) { if !o.GenJavaConfigs { return generatedFile{}, nil } javaBin := "java" - imageEnv, err := r.getEnv() + imageEnv, err := r.GetEnv() if err != nil { return generatedFile{}, fmt.Errorf("unable to get the environment of the toolchain image to determine JAVA_HOME: %w", err) } @@ -419,7 +391,7 @@ func genJavaConfigs(r runner, o *Options) (generatedFile, error) { // looking for in a more deterministic format. "-version" is just a placeholder so that the // command doesn't error out. Although it will likely print the same version string but with // some non-deterministic prefix. - out, err := r.execCmd(javaBin, "-XshowSettings:properties", "-version") + out, err := r.ExecCmd(javaBin, "-XshowSettings:properties", "-version") if err != nil { return generatedFile{}, fmt.Errorf("unable to determine the Java version installed in the toolchain container: %w", err) } @@ -467,7 +439,7 @@ func genJavaConfigs(r runner, o *Options) (generatedFile, error) { } // processTempDir creates a local temporary working directory to store intermediate files. -func processTempDir(o *Options) error { +func processTempDir(o *options.Options) error { if o.TempWorkDir != "" { s, err := os.Stat(o.TempWorkDir) if err != nil { @@ -486,7 +458,7 @@ func processTempDir(o *Options) error { return nil } -func genCppToolchainTarget(o *Options) string { +func genCppToolchainTarget(o *options.Options) string { if o.OutputConfigPath != "" { return fmt.Sprintf("//%s/cc:%s", strings.ReplaceAll(o.OutputConfigPath, "\\", "/"), o.CPPToolchainTargetName) } @@ -495,7 +467,7 @@ func genCppToolchainTarget(o *Options) string { // genConfigBuild generates the contents of a BUILD file with a toolchain target pointing to the // C++ toolchain related rules generated by Bazel and a default platforms target. -func genConfigBuild(o *Options) (generatedFile, error) { +func genConfigBuild(o *options.Options) (generatedFile, error) { if o.PlatformParams.CppToolchainTarget != "" { return generatedFile{}, fmt.Errorf(" C++ toolchain target was already set") } @@ -583,7 +555,7 @@ func writeGeneratedFileToTarball(g generatedFile, outTar *tar.Writer) error { // assembleConfigTarball combines the C++/Java configs represented by 'oc' into a single output // tarball if requested in the given options. -func assembleConfigTarball(o *Options, oc outputConfigs) error { +func assembleConfigTarball(o *options.Options, oc outputConfigs) error { out, err := os.Create(o.OutputTarball) if err != nil { return fmt.Errorf("unable to open output tarball %q for writing: %w", o.OutputTarball, err) @@ -691,7 +663,7 @@ func writeGeneratedFile(outDir string, g generatedFile) error { // copyConfigsToOutputDir copies the C++/Java configs represented by 'oc' to an output directory // if one was specified in the given options. This involves extracting C++ configs and generating // BUILD files for the Java & toolchain entrypoint & platform targets. -func copyConfigsToOutputDir(o *Options, oc outputConfigs) error { +func copyConfigsToOutputDir(o *options.Options, oc outputConfigs) error { configsRootDir := path.Join(o.OutputSourceRoot, o.OutputConfigPath) if err := os.MkdirAll(configsRootDir, os.ModePerm); err != nil { return fmt.Errorf("unable to create directory %q for writing configs: %w", configsRootDir, err) @@ -721,7 +693,7 @@ func copyConfigsToOutputDir(o *Options, oc outputConfigs) error { // given options. This could involve: // 1. Generate a single output tarball. // 2. Copy all configs into a specified directory. -func assembleConfigs(o *Options, oc outputConfigs) error { +func assembleConfigs(o *options.Options, oc outputConfigs) error { if len(o.OutputTarball) != 0 { if err := assembleConfigTarball(o, oc); err != nil { return fmt.Errorf("failed to assemble configs into a tarball: %w", err) @@ -788,7 +760,7 @@ func ManifestFromJSONFile(filePath string) (*Manifest, error) { // createManifest writes a manifest JSON file containing information about the generated configs if // the given options specified a manifest file. -func createManifest(o *Options) error { +func createManifest(o *options.Options) error { if len(o.OutputManifest) == 0 { return nil } @@ -827,25 +799,25 @@ func createManifest(o *Options) error { // - cc- C++ configs as generated by Bazel's internal C++ toolchain detection logic. // - config- Toolchain entrypoint target for cc_crosstool_top & the auto-generated platform target. // - java- Java toolchain definition. -func Run(o Options) error { +func Run(o options.Options) error { if err := processTempDir(&o); err != nil { return fmt.Errorf("unable to initialize a local temporary working directory to store intermediate files: %w", err) } - var r runner + var r runner.Runner var err error - if o.ExecOS == OSMacos { - r, err = newHostRunner(o.Cleanup) + if o.ExecOS == options.OSMacos { + r, err = runner.NewHostRunner(o.Cleanup) } else { - r, err = newDockerRunner(o.ToolchainContainer, o.Cleanup, o.ExecOS) - o.PlatformParams.ToolchainContainer = r.(*dockerRunner).resolvedImage + r, err = runner.NewDockerRunner(o.ToolchainContainer, o.Cleanup, o.ExecOS) + o.PlatformParams.ToolchainContainer = r.(*runner.DockerRunner).ResolvedImage } if err != nil { return fmt.Errorf("failed to initialize a docker container: %w", err) } - defer r.cleanup() + defer r.Cleanup() bazeliskPath, err := installBazelisk(r, o.TempWorkDir, o.ExecOS) diff --git a/pkg/rbeconfigsgen/rbeconfigsgen_test.go b/pkg/rbeconfigsgen/rbeconfigsgen_test.go index 62a6b1119..cfeca59cc 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen_test.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen_test.go @@ -15,6 +15,7 @@ package rbeconfigsgen import ( + "github.com/bazelbuild/bazel-toolchains/pkg/options" "testing" ) @@ -22,38 +23,38 @@ func TestGenCppToolchainTarget(t *testing.T) { tests := []struct { name string want string - opt *Options + opt *options.Options }{ { name: "No options, linux, choose default", want: "//cc:cc-compiler-k8", - opt: &Options{ + opt: &options.Options{ ExecOS: "linux", }, }, { name: "No options, windows, choose default", want: "//cc:cc-compiler-x64_windows", - opt: &Options{ + opt: &options.Options{ ExecOS: "windows", }, }, { name: "Windows pick compiler", want: "//cc:cc-compiler-x64_windows_mingw", - opt: &Options{ + opt: &options.Options{ ExecOS: "windows", CPPToolchainTargetName: "cc-compiler-x64_windows_mingw", }, }, { name: "Linux pick output path", want: "//configs/foo/bar/cc:cc-compiler-k8", - opt: &Options{ + opt: &options.Options{ ExecOS: "linux", OutputConfigPath: "configs/foo/bar", }, }, { name: "Windows pick output path and compiler", want: "//configs/fizz/buzz/cc:foobar-cc-good", - opt: &Options{ + opt: &options.Options{ ExecOS: "windows", OutputConfigPath: "configs/fizz/buzz", CPPToolchainTargetName: "foobar-cc-good", @@ -61,7 +62,7 @@ func TestGenCppToolchainTarget(t *testing.T) { }, { name: "Windows pick backslash style path and compiler", want: "//configs/fizz/buzz/cc:foobar-cc-good", - opt: &Options{ + opt: &options.Options{ ExecOS: "windows", OutputConfigPath: "configs\\fizz\\buzz", CPPToolchainTargetName: "foobar-cc-good", diff --git a/pkg/rbeconfigsgen/docker_runner.go b/pkg/runner/docker_runner.go similarity index 78% rename from pkg/rbeconfigsgen/docker_runner.go rename to pkg/runner/docker_runner.go index 9d3991ae9..7eb095a10 100644 --- a/pkg/rbeconfigsgen/docker_runner.go +++ b/pkg/runner/docker_runner.go @@ -12,10 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. // -package rbeconfigsgen +package runner import ( "fmt" + "github.com/bazelbuild/bazel-toolchains/pkg/options" "log" "strings" ) @@ -24,20 +25,20 @@ import ( // OS where the OS refers to the OS of the toolchain container. func workdir(os string) string { switch os { - case OSLinux: + case options.OSLinux: return "/workdir" - case OSWindows: + case options.OSWindows: return "C:/workdir" } log.Fatalf("Invalid OS: %q", os) return "" } -// dockerRunner implements interface runner -// dockerRunner allows starting a container for a given docker image and subsequently running +// DockerRunner implements interface Runner +// DockerRunner allows starting a container for a given docker image and subsequently running // arbitrary commands inside the container or extracting files from it. -// dockerRunner uses the docker client to spin up & interact with containers. -type dockerRunner struct { +// DockerRunner uses the docker client to spin up & interact with containers. +type DockerRunner struct { // Input arguments. // containerImage is the docker image to spin up as a running container. This could be a tagged // or floating reference to a docker image but in a format acceptable to the docker client. @@ -46,30 +47,30 @@ type dockerRunner struct { stopContainer bool // Parameters that affect how commands are executed inside the running toolchain container. - // These parameters can be changed between calls to the execCmd function. + // These parameters can be changed between calls to the ExecCmd function. // workdir is the working directory to use to run commands inside the container. workdir string // additionalEnv is the environment variables to set when executing commands additionalEnv map[string]string - // Populated by the runner. + // Populated by the Runner. // dockerPath is the path to the docker client. dockerPath string // containerID is the ID of the running docker container. containerID string - // resolvedImage is the container image referenced by its sha256 digest. - resolvedImage string + // ResolvedImage is the container image referenced by its sha256 digest. + ResolvedImage string } -// newDockerRunner creates a new running container of the given containerImage. stopContainer -// determines if the cleanup function on the dockerRunner will stop the running container when +// NewDockerRunner creates a new running container of the given containerImage. stopContainer +// determines if the Cleanup function on the DockerRunner will stop the running container when // called. -func newDockerRunner(containerImage string, stopContainer bool, execOS string) (*dockerRunner, error) { +func NewDockerRunner(containerImage string, stopContainer bool, execOS string) (*DockerRunner, error) { if containerImage == "" { return nil, fmt.Errorf("container image was not specified") } - d := &dockerRunner{ + d := &DockerRunner{ containerImage: containerImage, stopContainer: stopContainer, dockerPath: "docker", @@ -83,9 +84,9 @@ func newDockerRunner(containerImage string, stopContainer bool, execOS string) ( } resolvedImage = strings.TrimSpace(resolvedImage) log.Printf("Resolved toolchain image %q to fully qualified reference %q.", d.containerImage, resolvedImage) - d.resolvedImage = resolvedImage + d.ResolvedImage = resolvedImage - cid, err := runCmd(d.dockerPath, "create", "--rm", d.resolvedImage, "sleep", "infinity") + cid, err := runCmd(d.dockerPath, "create", "--rm", d.ResolvedImage, "sleep", "infinity") if err != nil { return nil, fmt.Errorf("failed to create a container with the toolchain container image: %w", err) } @@ -94,21 +95,21 @@ func newDockerRunner(containerImage string, stopContainer bool, execOS string) ( return nil, fmt.Errorf("container ID %q extracted from the stdout of the container create command had unexpected length, got %d, want 64", cid, len(cid)) } d.containerID = cid - log.Printf("Created container ID %v for toolchain container image %v.", d.containerID, d.resolvedImage) + log.Printf("Created container ID %v for toolchain container image %v.", d.containerID, d.ResolvedImage) if _, err := runCmd(d.dockerPath, "start", d.containerID); err != nil { return nil, fmt.Errorf("failed to run the toolchain container: %w", err) } - if _, err := d.execCmd("mkdir", workdir(execOS)); err != nil { - d.cleanup() + if _, err := d.ExecCmd("mkdir", workdir(execOS)); err != nil { + d.Cleanup() return nil, fmt.Errorf("failed to create workdir in toolchain container: %w", err) } - d.setWorkdir(workdir(execOS)) + d.SetWorkdir(workdir(execOS)) return d, nil } // execCmd runs the given command inside the docker container and returns the output with whitespace // trimmed from the edges. -func (d *dockerRunner) execCmd(args ...string) (string, error) { +func (d *DockerRunner) ExecCmd(args ...string) (string, error) { a := []string{"exec"} if d.workdir != "" { a = append(a, "-w", d.workdir) @@ -122,20 +123,20 @@ func (d *dockerRunner) execCmd(args ...string) (string, error) { return strings.TrimSpace(o), err } -// cleanup stops the running container if stopContainer was true when the dockerRunner was created. -func (d *dockerRunner) cleanup() { +// cleanup stops the running container if stopContainer was true when the DockerRunner was created. +func (d *DockerRunner) Cleanup() { if !d.stopContainer { - log.Printf("Not stopping container %v of image %v because the Cleanup option was set to false.", d.containerID, d.resolvedImage) + log.Printf("Not stopping container %v of image %v because the Cleanup option was set to false.", d.containerID, d.ResolvedImage) return } if _, err := runCmd(d.dockerPath, "stop", "-t", "0", d.containerID); err != nil { - log.Printf("Failed to stop container %v of toolchain image %v but it's ok to ignore this error if config generation & extraction succeeded.", d.containerID, d.resolvedImage) + log.Printf("Failed to stop container %v of toolchain image %v but it's ok to ignore this error if config generation & extraction succeeded.", d.containerID, d.ResolvedImage) } } // copyTo copies the local file at 'src' to the container where 'dst' is the path inside // the container. d.workdir has no impact on this function. -func (d *dockerRunner) copyTo(src, dst string) error { +func (d *DockerRunner) CopyTo(src, dst string) error { if _, err := runCmd(d.dockerPath, "cp", src, fmt.Sprintf("%s:%s", d.containerID, dst)); err != nil { return err } @@ -144,7 +145,7 @@ func (d *dockerRunner) copyTo(src, dst string) error { // copyFrom extracts the file at 'src' from inside the container and copies it to the path // 'dst' locally. d.workdir has no impact on this function. -func (d *dockerRunner) copyFrom(src, dst string) error { +func (d *DockerRunner) CopyFrom(src, dst string) error { if _, err := runCmd(d.dockerPath, "cp", fmt.Sprintf("%s:%s", d.containerID, src), dst); err != nil { return err } @@ -156,9 +157,9 @@ func (d *dockerRunner) copyFrom(src, dst string) error { // captured by the return value of this function. // The return value of this function is a map from env keys to their values. If the image config, // specifies the same env key multiple times, later values supercede earlier ones. -func (d *dockerRunner) getEnv() (map[string]string, error) { +func (d *DockerRunner) GetEnv() (map[string]string, error) { result := make(map[string]string) - o, err := runCmd(d.dockerPath, "inspect", "-f", "{{range $i, $v := .Config.Env}}{{println $v}}{{end}}", d.resolvedImage) + o, err := runCmd(d.dockerPath, "inspect", "-f", "{{range $i, $v := .Config.Env}}{{println $v}}{{end}}", d.ResolvedImage) if err != nil { return nil, fmt.Errorf("failed to inspect the docker image to get environment variables: %w", err) } @@ -185,18 +186,18 @@ func (d *dockerRunner) getEnv() (map[string]string, error) { return result, nil } -func (d *dockerRunner) getWorkdir() string { +func (d *DockerRunner) GetWorkdir() string { return d.workdir } -func (d *dockerRunner) setWorkdir(wd string) { +func (d *DockerRunner) SetWorkdir(wd string) { d.workdir = wd } -func (d *dockerRunner) getAdditionalEnv() map[string]string { +func (d *DockerRunner) GetAdditionalEnv() map[string]string { return d.additionalEnv } -func (d *dockerRunner) setAdditionalEnv(env map[string]string) { +func (d *DockerRunner) SetAdditionalEnv(env map[string]string) { d.additionalEnv = env } diff --git a/pkg/rbeconfigsgen/host_runner.go b/pkg/runner/host_runner.go similarity index 82% rename from pkg/rbeconfigsgen/host_runner.go rename to pkg/runner/host_runner.go index c864fff08..6c2ebb7b6 100644 --- a/pkg/rbeconfigsgen/host_runner.go +++ b/pkg/runner/host_runner.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. // -package rbeconfigsgen +package runner import ( "fmt" @@ -23,7 +23,7 @@ import ( "strings" ) -// hostRunner implements interface runner +// hostRunner implements interface Runner // hostRunner allows subsequently running // arbitrary commands directly on the host. type hostRunner struct { @@ -39,13 +39,13 @@ type hostRunner struct { additionalEnv map[string]string } -// newHostRunner creates a new runner which executes commands directly in host environment. deleteWorkdir -// determines if the cleanup function on the hostRunner will remove temporary directory -func newHostRunner(deleteWorkdir bool) (*hostRunner, error) { +// NewHostRunner creates a new Runner which executes commands directly in host environment. deleteWorkdir +// determines if the Cleanup function on the hostRunner will remove temporary directory +func NewHostRunner(deleteWorkdir bool) (*hostRunner, error) { workdir, err := ioutil.TempDir("", "host_runner_") if err != nil { - return nil, fmt.Errorf("failed to create a temporary local directory for host runner: %w", err) + return nil, fmt.Errorf("failed to create a temporary local directory for host Runner: %w", err) } return &hostRunner{ @@ -57,7 +57,7 @@ func newHostRunner(deleteWorkdir bool) (*hostRunner, error) { // execCmd runs the given command and returns the output with whitespace // trimmed from the edges. -func (r *hostRunner) execCmd(args ...string) (string, error) { +func (r *hostRunner) ExecCmd(args ...string) (string, error) { log.Printf("Running: %s", strings.Join(args, " ")) c := exec.Command(args[0], args[1:]...) c.Env = append(os.Environ(), convertAdditionalEnv(r)...) @@ -71,7 +71,7 @@ func (r *hostRunner) execCmd(args ...string) (string, error) { } // cleanup stops the running container if stopContainer was true when the hostRunner was created. -func (r *hostRunner) cleanup() { +func (r *hostRunner) Cleanup() { if !r.deleteWorkdir { log.Printf("Not deleting workdir %v because the Cleanup option was set to false.", r.globalWorkdir) return @@ -84,7 +84,7 @@ func (r *hostRunner) cleanup() { // copyTo copies the local file at 'src' to the container where 'dst' is the path inside // the container. d.workdir has no impact on this function. -func (r *hostRunner) copyTo(src, dst string) error { +func (r *hostRunner) CopyTo(src, dst string) error { if _, err := runCmd("cp", src, dst); err != nil { return err } @@ -93,7 +93,7 @@ func (r *hostRunner) copyTo(src, dst string) error { // copyFrom extracts the file at 'src' from inside the container and copies it to the path // 'dst' locally. d.workdir has no impact on this function. -func (r *hostRunner) copyFrom(src, dst string) error { +func (r *hostRunner) CopyFrom(src, dst string) error { if _, err := runCmd("cp", src, dst); err != nil { return err } @@ -105,7 +105,7 @@ func (r *hostRunner) copyFrom(src, dst string) error { // captured by the return value of this function. // The return value of this function is a map from env keys to their values. If the image config, // specifies the same env key multiple times, later values supercede earlier ones. -func (r *hostRunner) getEnv() (map[string]string, error) { +func (r *hostRunner) GetEnv() (map[string]string, error) { result := make(map[string]string) for _, s := range os.Environ() { s = strings.TrimSpace(s) @@ -129,18 +129,18 @@ func (r *hostRunner) getEnv() (map[string]string, error) { return result, nil } -func (r *hostRunner) getWorkdir() string { +func (r *hostRunner) GetWorkdir() string { return r.workdir } -func (r *hostRunner) setWorkdir(wd string) { +func (r *hostRunner) SetWorkdir(wd string) { r.workdir = wd } -func (r *hostRunner) getAdditionalEnv() map[string]string { +func (r *hostRunner) GetAdditionalEnv() map[string]string { return r.additionalEnv } -func (r *hostRunner) setAdditionalEnv(env map[string]string) { +func (r *hostRunner) SetAdditionalEnv(env map[string]string) { r.additionalEnv = env } diff --git a/pkg/runner/runcmd.go b/pkg/runner/runcmd.go new file mode 100644 index 000000000..68ad3e3a0 --- /dev/null +++ b/pkg/runner/runcmd.go @@ -0,0 +1,23 @@ +package runner + +import ( + "fmt" + "log" + "os/exec" + "strings" +) + +// runCmd runs an arbitrary command in a shell, logs the exact command that was run and returns +// the generated stdout/stderr. If the command fails, the stdout/stderr is always logged. +func runCmd(cmd string, args ...string) (string, error) { + cmdStr := fmt.Sprintf("'%s'", strings.Join(append([]string{cmd}, args...), " ")) + log.Printf("Running: %s", cmdStr) + c := exec.Command(cmd, args...) + o, err := c.CombinedOutput() + if err != nil { + log.Printf("Output: %s", o) + return "", err + } + return string(o), nil +} + diff --git a/pkg/rbeconfigsgen/runner.go b/pkg/runner/runner.go similarity index 55% rename from pkg/rbeconfigsgen/runner.go rename to pkg/runner/runner.go index d12518880..aadbec185 100644 --- a/pkg/rbeconfigsgen/runner.go +++ b/pkg/runner/runner.go @@ -12,38 +12,38 @@ // See the License for the specific language governing permissions and // limitations under the License. // -package rbeconfigsgen +package runner import "fmt" -type runner interface { - // execCmd runs the given command in runner env and returns the output with whitespace +type Runner interface { + // execCmd runs the given command in Runner env and returns the output with whitespace // trimmed from the edges. - execCmd(args ...string) (string, error) + ExecCmd(args ...string) (string, error) - // copyTo copies the local file at 'src' to 'dst' in the runner env. - copyTo(src, dst string) error + // copyTo copies the local file at 'src' to 'dst' in the Runner env. + CopyTo(src, dst string) error - // copyFrom extracts the file at 'src' from runner and copies it to the path 'dst' locally. - copyFrom(src, dst string) error + // copyFrom extracts the file at 'src' from Runner and copies it to the path 'dst' locally. + CopyFrom(src, dst string) error - cleanup() + Cleanup() - // getEnv gets the shell environment values from the runner - getEnv() (map[string]string, error) + // getEnv gets the shell environment values from the Runner + GetEnv() (map[string]string, error) - setAdditionalEnv(map[string]string) + SetAdditionalEnv(map[string]string) - getAdditionalEnv() map[string]string + GetAdditionalEnv() map[string]string - getWorkdir() string + GetWorkdir() string - setWorkdir(string) + SetWorkdir(string) } -func convertAdditionalEnv(r runner) []string { +func convertAdditionalEnv(r Runner) []string { addEnv := []string{} - for k, v := range r.getAdditionalEnv() { + for k, v := range r.GetAdditionalEnv() { addEnv = append(addEnv, fmt.Sprintf("%s=%s", k, v)) } return addEnv diff --git a/tests/scripts/configs_e2e/configs_e2e.go b/tests/scripts/configs_e2e/configs_e2e.go index dce747894..0b2d07986 100644 --- a/tests/scripts/configs_e2e/configs_e2e.go +++ b/tests/scripts/configs_e2e/configs_e2e.go @@ -33,6 +33,7 @@ import ( "encoding/json" "flag" "fmt" + "github.com/bazelbuild/bazel-toolchains/pkg/options" "io" "log" "net/http" @@ -323,7 +324,7 @@ func validateRBEInstName(instName string) error { // downloadBazelisk downloads Bazelisk for Linux to the given directory and returns the path to the // downloaded Bazelisk executable. func downloadBazelisk(outputDir string) (string, error) { - bazeliskURL, bazeliskFile, err := rbeconfigsgen.BazeliskDownloadInfo(rbeconfigsgen.OSLinux) + bazeliskURL, bazeliskFile, err := rbeconfigsgen.BazeliskDownloadInfo(options.OSLinux) if err != nil { return "", fmt.Errorf("unable to determine URL to download Bazelisk from for Linux: %w", err) } From 356832b67e5250f625a61cf36a3aa54ae5a1fbdc Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Thu, 3 Jun 2021 00:05:38 +0700 Subject: [PATCH 03/10] add option --runner --- cmd/rbe_configs_gen/rbe_configs_gen.go | 16 +++++++++---- pkg/options/options.go | 27 ++++++++++++++++++---- pkg/rbeconfigsgen/rbeconfigsgen.go | 29 +++++++++++++---------- pkg/runner/docker_runner.go | 5 ++-- pkg/runner/host_runner.go | 7 +++--- pkg/runner/runner.go | 32 +++++++++++++++++++------- 6 files changed, 82 insertions(+), 34 deletions(-) diff --git a/cmd/rbe_configs_gen/rbe_configs_gen.go b/cmd/rbe_configs_gen/rbe_configs_gen.go index 32e9ff20b..ae6baa1d6 100644 --- a/cmd/rbe_configs_gen/rbe_configs_gen.go +++ b/cmd/rbe_configs_gen/rbe_configs_gen.go @@ -30,11 +30,13 @@ import ( var ( // Mandatory input arguments. - toolchainContainer = flag.String("toolchain_container", "", "Repository path to toolchain image to generate configs for. E.g., l.gcr.io/google/rbe-ubuntu16-04:latest. Omit if configuration should be built locally") - execOS = flag.String("exec_os", "", "The OS (linux|windows|osx) of the toolchain container image a.k.a, the execution platform in Bazel.") - targetOS = flag.String("target_os", "", "The OS (linux|windows|osx) artifacts built will target a.k.a, the target platform in Bazel.") + execOS = flag.String("exec_os", "", "The OS (linux|windows|macos) of the toolchain container image a.k.a, the execution platform in Bazel.") + targetOS = flag.String("target_os", "", "The OS (linux|windows|macos) artifacts built will target a.k.a, the target platform in Bazel.") // Optional input arguments. + runner = flag.String("runner", "", "Runner (host|docker) to use to generate configs. Defaults to docker for linux|windows, host for macos.") + // toolchainContainer is required option for runner=docker + toolchainContainer = flag.String("toolchain_container", "", "Repository path to toolchain image to generate configs for. E.g., l.gcr.io/google/rbe-ubuntu16-04:latest. Required if runner=docker, ignored if runner=host.") bazelVersion = flag.String("bazel_version", "", "(Optional) Bazel release version to generate configs for. E.g., 4.0.0. If unspecified, the latest available Bazel release is picked.") // Arguments affecting output generation not specific to either C++ or Java Configs. @@ -63,7 +65,12 @@ var ( // binary. Printing defaults are skipped as much as possible to avoid cluttering the output. func printFlags() { log.Println("rbe_configs_gen.go \\") - log.Printf("--toolchain_container=%q \\", *toolchainContainer) + if len(*runner) != 0 { + log.Printf("--runner=%q \\", *runner) + } + if len(*toolchainContainer) != 0 { + log.Printf("--toolchain_container=%q \\", *toolchainContainer) + } log.Printf("--exec_os=%q \\", *execOS) log.Printf("--target_os=%q \\", *targetOS) log.Printf("--bazel_version=%q \\", *bazelVersion) @@ -149,6 +156,7 @@ func main() { o := options.Options{ BazelVersion: *bazelVersion, + Runner: *runner, ToolchainContainer: *toolchainContainer, ExecOS: *execOS, TargetOS: *targetOS, diff --git a/pkg/options/options.go b/pkg/options/options.go index 65c50e6a4..89ff9eb11 100644 --- a/pkg/options/options.go +++ b/pkg/options/options.go @@ -54,6 +54,9 @@ type Options struct { // TargetOS is the OS to be used as the target platform in the generated platform rule. This // is the OS that artifacts built by Bazel will be executed on. TargetOS string + // Runner is a name of the runner to use for generating toolchain configuration. + // Allowed values are: docker (default), host + Runner string // OutputTarball is the path at with a tarball will be generated containing the C++/Java // configs. OutputTarball string @@ -125,7 +128,7 @@ const ( OSLinux = "linux" // OSWindows represents Windows when selecting platforms. OSWindows = "windows" - // OSWindows represents MacOS when selecting platforms. + // OSMacos represents MacOS when selecting platforms. OSMacos = "osx" ) @@ -194,10 +197,12 @@ var ( }, TargetConstraints: []string{ "@bazel_tools//platforms:osx", - "@bazel_tools//platforms:x86_64", + //"@bazel_tools//platforms:x86_64", }, OSFamily: "MacOS", }, + // excluding osx_archs.bzl because of issue + // described here https://github.com/bazelbuild/bazel/pull/13528 CPPConfigTargets: []string{"@local_config_cc//...", "--", "-@local_config_cc//:osx_archs.bzl"}, CPPConfigRepo: "local_config_cc", CppBazelCmd: "build", @@ -224,6 +229,14 @@ func strListContains(l []string, s string) bool { // ApplyDefaults applies platform specific default values to the given options for the given // OS. func (o *Options) ApplyDefaults(os string) error { + if o.Runner == "" { + switch o.ExecOS { + case OSMacos: + o.Runner = "host" + case OSLinux, OSWindows: + o.Runner = "docker" + } + } dopts, ok := DefaultExecOptions[os] if !ok { return fmt.Errorf("got unknown OS %q, want one of %s", os, strings.Join(validOS, ", ")) @@ -264,15 +277,18 @@ func (o *Options) Validate() error { } o.BazelVersion = v } - if o.ToolchainContainer == "" && (o.ExecOS == "linux" || o.ExecOS == "windows"){ - return fmt.Errorf("ToolchainContainer was not specified") - } if o.ExecOS == "" { return fmt.Errorf("ExecOS was not specified") } if !strListContains(validOS, o.ExecOS) { return fmt.Errorf("invalid exec_os, got %q, want one of %s", o.ExecOS, strings.Join(validOS, ", ")) } + if o.Runner == "" { + return fmt.Errorf("Runner wasn't specified") + } + if o.ToolchainContainer == "" && (o.Runner == "docker"){ + return fmt.Errorf("ToolchainContainer was not specified") + } if o.TargetOS == "" { return fmt.Errorf("TargetOS was not specified") } @@ -305,6 +321,7 @@ func (o *Options) Validate() error { } log.Printf("rbeconfigsgen.Options:") log.Printf("BazelVersion=%q", o.BazelVersion) + log.Printf("Runner=%q", o.Runner) log.Printf("ToolchainContainer=%q", o.ToolchainContainer) log.Printf("ExecOS=%q", o.ExecOS) log.Printf("TargetOS=%q", o.TargetOS) diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index b0543a769..04f3a1b3b 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -85,7 +85,7 @@ platform( {{ range .ExecConstraints }} "{{ . }}", {{ end }} ], exec_properties = { - "container-image": "docker://{{.ToolchainContainer}}", + {{ if .ToolchainContainer }}"container-image": "docker://{{.ToolchainContainer}}",{{ end }} "OSFamily": "{{.OSFamily}}", }, ) @@ -282,12 +282,9 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st } r.SetAdditionalEnv(generationEnv) - cmd := []string{ - bazeliskPath, - o.CppBazelCmd, - } - cmd = append(cmd, o.CPPConfigTargets...) - if _, err := r.ExecCmd(cmd...); err != nil { + args := []string{o.CppBazelCmd} + args = append(args, o.CPPConfigTargets...) + if _, err := r.ExecCmd(bazeliskPath, args...); err != nil { return "", fmt.Errorf("Bazel was unable to build the C++ config generation targets in the toolchain container: %w", err) } @@ -807,22 +804,30 @@ func Run(o options.Options) error { var r runner.Runner var err error - if o.ExecOS == options.OSMacos { + switch o.Runner { + case "host": r, err = runner.NewHostRunner(o.Cleanup) - } else { + // allow to specify toolchain container for host runner + // can be useful if host runner is in fact container + if o.ToolchainContainer != "" { + o.PlatformParams.ToolchainContainer = o.ToolchainContainer + } + case "docker": r, err = runner.NewDockerRunner(o.ToolchainContainer, o.Cleanup, o.ExecOS) o.PlatformParams.ToolchainContainer = r.(*runner.DockerRunner).ResolvedImage + default: + return fmt.Errorf("unknown runner: %s", o.Runner) } if err != nil { - return fmt.Errorf("failed to initialize a docker container: %w", err) + return fmt.Errorf("failed to initialize runner: %w", err) } defer r.Cleanup() bazeliskPath, err := installBazelisk(r, o.TempWorkDir, o.ExecOS) if err != nil { - return fmt.Errorf("failed to install Bazelisk into the toolchain container: %w", err) + return fmt.Errorf("failed to install Bazelisk into the runner: %w", err) } cppConfigsTarball, err := genCppConfigs(r, &o, bazeliskPath) @@ -831,7 +836,7 @@ func Run(o options.Options) error { } javaBuild, err := genJavaConfigs(r, &o) if err != nil { - return fmt.Errorf("failed to extract information about the installed JDK version in the toolchain container needed to generate Java configs: %w", err) + return fmt.Errorf("failed to extract information about the installed JDK version in the runner needed to generate Java configs: %w", err) } configBuild, err := genConfigBuild(&o) diff --git a/pkg/runner/docker_runner.go b/pkg/runner/docker_runner.go index 7eb095a10..62c7db743 100644 --- a/pkg/runner/docker_runner.go +++ b/pkg/runner/docker_runner.go @@ -109,15 +109,16 @@ func NewDockerRunner(containerImage string, stopContainer bool, execOS string) ( // execCmd runs the given command inside the docker container and returns the output with whitespace // trimmed from the edges. -func (d *DockerRunner) ExecCmd(args ...string) (string, error) { +func (d *DockerRunner) ExecCmd(cmd string, args ...string) (string, error) { a := []string{"exec"} if d.workdir != "" { a = append(a, "-w", d.workdir) } - for _, e := range d.additionalEnv { + for _, e := range convertAdditionalEnv(d) { a = append(a, "-e", e) } a = append(a, d.containerID) + a = append(a, cmd) a = append(a, args...) o, err := runCmd(d.dockerPath, a...) return strings.TrimSpace(o), err diff --git a/pkg/runner/host_runner.go b/pkg/runner/host_runner.go index 6c2ebb7b6..9db0edf2f 100644 --- a/pkg/runner/host_runner.go +++ b/pkg/runner/host_runner.go @@ -57,10 +57,11 @@ func NewHostRunner(deleteWorkdir bool) (*hostRunner, error) { // execCmd runs the given command and returns the output with whitespace // trimmed from the edges. -func (r *hostRunner) ExecCmd(args ...string) (string, error) { - log.Printf("Running: %s", strings.Join(args, " ")) - c := exec.Command(args[0], args[1:]...) +func (r *hostRunner) ExecCmd(cmd string, args ...string) (string, error) { + log.Printf("Running: %s", strings.Join(append([]string{cmd}, args...), " ")) + c := exec.Command(cmd, args...) c.Env = append(os.Environ(), convertAdditionalEnv(r)...) + //log.Printf("Running env: %v", c.Env) c.Dir = r.workdir o, err := c.CombinedOutput() if err != nil { diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index aadbec185..1d1784fa4 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -12,37 +12,53 @@ // See the License for the specific language governing permissions and // limitations under the License. // + +// Package runner provides generic Runner interface +// and its implementations package runner import "fmt" +// Runner interface represents runner which allows to execute commands in +// (possibly) isolated environment type Runner interface { - // execCmd runs the given command in Runner env and returns the output with whitespace - // trimmed from the edges. - ExecCmd(args ...string) (string, error) - // copyTo copies the local file at 'src' to 'dst' in the Runner env. + // ExecCmd executes the given command within the runner environment + // and returns the output with whitespace trimmed from the edges. + ExecCmd(cmd string, args ...string) (string, error) + + // CopyTo copies the local file to the runner env. CopyTo(src, dst string) error - // copyFrom extracts the file at 'src' from Runner and copies it to the path 'dst' locally. + // CopyFrom copies the file from runner to the local path. CopyFrom(src, dst string) error + // Cleanup removes runner environment (temporary files, containers) + // after toolchain configuration is generated. Cleanup() - // getEnv gets the shell environment values from the Runner + // GetEnv returns default variables from runner environment, + // which are supposed to be available during remote execution. GetEnv() (map[string]string, error) + // SetAdditionalEnv adds auxiliary variables needed during toolchain config generation. SetAdditionalEnv(map[string]string) + // GetAdditionalEnv returns variables previously added with SetAdditionalEnv. GetAdditionalEnv() map[string]string + // SetWorkdir sets new working directory for command execution inside the runner + SetWorkdir(string) + + // GetWorkdir returns current workdir used for command execution. GetWorkdir() string - SetWorkdir(string) } +// convertAdditionalEnv returns list of additional env vars in a format +// suitable for passing to os/exec.Cmd func convertAdditionalEnv(r Runner) []string { - addEnv := []string{} + addEnv := make([]string, 0) for k, v := range r.GetAdditionalEnv() { addEnv = append(addEnv, fmt.Sprintf("%s=%s", k, v)) } From 1c92047d1cdd5e1e4b2e0a183ad7e66c2300dac9 Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Thu, 3 Jun 2021 00:38:11 +0700 Subject: [PATCH 04/10] fix cli flags help strings with osx mentioned --- cmd/rbe_configs_gen/rbe_configs_gen.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/rbe_configs_gen/rbe_configs_gen.go b/cmd/rbe_configs_gen/rbe_configs_gen.go index ae6baa1d6..5df29fda6 100644 --- a/cmd/rbe_configs_gen/rbe_configs_gen.go +++ b/cmd/rbe_configs_gen/rbe_configs_gen.go @@ -30,11 +30,11 @@ import ( var ( // Mandatory input arguments. - execOS = flag.String("exec_os", "", "The OS (linux|windows|macos) of the toolchain container image a.k.a, the execution platform in Bazel.") - targetOS = flag.String("target_os", "", "The OS (linux|windows|macos) artifacts built will target a.k.a, the target platform in Bazel.") + execOS = flag.String("exec_os", "", "The OS (linux|windows|osx) of the toolchain container image a.k.a, the execution platform in Bazel.") + targetOS = flag.String("target_os", "", "The OS (linux|windows|osx) artifacts built will target a.k.a, the target platform in Bazel.") // Optional input arguments. - runner = flag.String("runner", "", "Runner (host|docker) to use to generate configs. Defaults to docker for linux|windows, host for macos.") + runner = flag.String("runner", "", "Runner (host|docker) to use to generate configs. Defaults to docker for linux|windows, host for osx.") // toolchainContainer is required option for runner=docker toolchainContainer = flag.String("toolchain_container", "", "Repository path to toolchain image to generate configs for. E.g., l.gcr.io/google/rbe-ubuntu16-04:latest. Required if runner=docker, ignored if runner=host.") bazelVersion = flag.String("bazel_version", "", "(Optional) Bazel release version to generate configs for. E.g., 4.0.0. If unspecified, the latest available Bazel release is picked.") From dfb6189603a80b4d87f2e48b67177a409c5c902b Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Thu, 3 Jun 2021 02:17:05 +0700 Subject: [PATCH 05/10] check working dir to be inside temporary working dir for host runner --- pkg/rbeconfigsgen/rbeconfigsgen.go | 4 +++- pkg/runner/docker_runner.go | 3 ++- pkg/runner/host_runner.go | 6 +++++- pkg/runner/runner.go | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index 04f3a1b3b..10af683fd 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -259,7 +259,9 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st return "", fmt.Errorf("failed to create empty directory %q inside the toolchain container: %w", cppProjDir, err) } oldWorkDir := r.GetWorkdir() - r.SetWorkdir(cppProjDir) + if err := r.SetWorkdir(cppProjDir); err != nil { + return "", fmt.Errorf("failed to set working directory %q: %w", cppProjDir, err) + } defer r.SetWorkdir(oldWorkDir) if _, err := r.ExecCmd("touch", "WORKSPACE", "BUILD.bazel"); err != nil { diff --git a/pkg/runner/docker_runner.go b/pkg/runner/docker_runner.go index 62c7db743..38eb6b0dd 100644 --- a/pkg/runner/docker_runner.go +++ b/pkg/runner/docker_runner.go @@ -191,8 +191,9 @@ func (d *DockerRunner) GetWorkdir() string { return d.workdir } -func (d *DockerRunner) SetWorkdir(wd string) { +func (d *DockerRunner) SetWorkdir(wd string) error { d.workdir = wd + return nil } func (d *DockerRunner) GetAdditionalEnv() map[string]string { diff --git a/pkg/runner/host_runner.go b/pkg/runner/host_runner.go index 9db0edf2f..05527ad5c 100644 --- a/pkg/runner/host_runner.go +++ b/pkg/runner/host_runner.go @@ -134,8 +134,12 @@ func (r *hostRunner) GetWorkdir() string { return r.workdir } -func (r *hostRunner) SetWorkdir(wd string) { +func (r *hostRunner) SetWorkdir(wd string) error { + if !strings.HasPrefix(wd, r.globalWorkdir) { + return fmt.Errorf("refusing to set working directory %s: host runner allows workdirs only in parent temporary directory %s", wd, r.globalWorkdir) + } r.workdir = wd + return nil } func (r *hostRunner) GetAdditionalEnv() map[string]string { diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 1d1784fa4..4a427c602 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -48,7 +48,7 @@ type Runner interface { GetAdditionalEnv() map[string]string // SetWorkdir sets new working directory for command execution inside the runner - SetWorkdir(string) + SetWorkdir(string) error // GetWorkdir returns current workdir used for command execution. GetWorkdir() string From bb640871a8cff619c1b58c4c8a1c37cb257bb940 Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Thu, 3 Jun 2021 17:56:06 +0700 Subject: [PATCH 06/10] fix doc comments and error messages --- cmd/rbe_configs_gen/rbe_configs_gen.go | 2 +- pkg/options/options.go | 4 +- pkg/rbeconfigsgen/rbeconfigsgen.go | 61 ++++++++++++++------------ pkg/runner/host_runner.go | 23 +++++----- 4 files changed, 47 insertions(+), 43 deletions(-) diff --git a/cmd/rbe_configs_gen/rbe_configs_gen.go b/cmd/rbe_configs_gen/rbe_configs_gen.go index 5df29fda6..1c0a9614a 100644 --- a/cmd/rbe_configs_gen/rbe_configs_gen.go +++ b/cmd/rbe_configs_gen/rbe_configs_gen.go @@ -36,7 +36,7 @@ var ( // Optional input arguments. runner = flag.String("runner", "", "Runner (host|docker) to use to generate configs. Defaults to docker for linux|windows, host for osx.") // toolchainContainer is required option for runner=docker - toolchainContainer = flag.String("toolchain_container", "", "Repository path to toolchain image to generate configs for. E.g., l.gcr.io/google/rbe-ubuntu16-04:latest. Required if runner=docker, ignored if runner=host.") + toolchainContainer = flag.String("toolchain_container", "", "Repository path to toolchain image to generate configs for. E.g., l.gcr.io/google/rbe-ubuntu16-04:latest. Required if runner=docker.") bazelVersion = flag.String("bazel_version", "", "(Optional) Bazel release version to generate configs for. E.g., 4.0.0. If unspecified, the latest available Bazel release is picked.") // Arguments affecting output generation not specific to either C++ or Java Configs. diff --git a/pkg/options/options.go b/pkg/options/options.go index 89ff9eb11..03f70e46d 100644 --- a/pkg/options/options.go +++ b/pkg/options/options.go @@ -12,8 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. // -// Package rbeconfigsgen contains utilities to generate C++ & Java Toolchain configs for Bazel to be -// used to run RBE builds +// Package options describes cli options for rbe_configs_gen package options import ( @@ -211,6 +210,7 @@ var ( "BAZEL_COMPILER": "clang", "BAZEL_TARGET_CPU": "darwin_x86_64", "CC": "clang", + "CC_TOOLCHAIN_NAME": "darwin_x86_64", }, CPPToolchainTargetName: "cc-compiler-darwin_x86_64", }, diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index 10af683fd..1918a4374 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -157,7 +157,7 @@ type outputConfigs struct { // licence will contain the OSS license applicable for the generated configs. license generatedFile // cppConfigsTarball is the path to the tarball file containing the C++ configs generated by - // Bazel inside the toolchain container. + // Bazel inside the runner. cppConfigsTarball string // configBuild represents the BUILD file containing the C++ crosstool top toolchain target // and the default platform definition. @@ -181,8 +181,8 @@ func BazeliskDownloadInfo(os string) (string, string, error) { } // installBazelisk downloads bazelisk locally to the specified directory for the given os and copies -// it into the running toolchain container. -// Returns the path Bazelisk was installed to inside the running toolchain container. +// it into the runner. +// Returns the path Bazelisk was installed to inside the runner. func installBazelisk(r runner.Runner, downloadDir, execOS string) (string, error) { url, filename, err := BazeliskDownloadInfo(execOS) if err != nil { @@ -210,7 +210,7 @@ func installBazelisk(r runner.Runner, downloadDir, execOS string) (string, error } if _, err := r.ExecCmd("chmod", "+x", bazeliskRunnerPath); err != nil { - return "", fmt.Errorf("failed to mark the Bazelisk binary as executable inside the container: %w", err) + return "", fmt.Errorf("failed to mark the Bazelisk binary as executable inside the runner: %w", err) } return bazeliskRunnerPath, nil } @@ -243,10 +243,10 @@ func appendCppEnv(env map[string]string, o *options.Options) (map[string]string, return env, nil } -// genCppConfigs generates C++ configs inside the running toolchain container represented by the -// given docker runner according to the given options. bazeliskPath is the path to the bazelisk -// binary inside the running toolchain container. -// The return value is the path to the C++ configs tarball copied out of the toolchain container. +// genCppConfigs generates C++ configs inside the runner represented by the +// given runner according to the given options. bazeliskPath is the path to the bazelisk +// binary inside the runner. +// The return value is the path to the C++ configs tarball copied out of the runner. func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (string, error) { if !o.GenCPPConfigs { return "", nil @@ -256,7 +256,7 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st // command we run in this function. cppProjDir := path.Join(r.GetWorkdir(), "cpp_configs_project") if _, err := r.ExecCmd("mkdir", cppProjDir); err != nil { - return "", fmt.Errorf("failed to create empty directory %q inside the toolchain container: %w", cppProjDir, err) + return "", fmt.Errorf("failed to create empty directory %q inside the runner: %w", cppProjDir, err) } oldWorkDir := r.GetWorkdir() if err := r.SetWorkdir(cppProjDir); err != nil { @@ -265,7 +265,7 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st defer r.SetWorkdir(oldWorkDir) if _, err := r.ExecCmd("touch", "WORKSPACE", "BUILD.bazel"); err != nil { - return "", fmt.Errorf("failed to create empty build & workspace files in the container to initialize a blank Bazel repository: %w", err) + return "", fmt.Errorf("failed to create empty build & workspace files in the runner to initialize a blank Bazel repository: %w", err) } // Backup the current environment & restore it before returning. @@ -280,24 +280,24 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st // logs. generationEnv, err := appendCppEnv(bazeliskEnv, o) if err != nil { - return "", fmt.Errorf("failed to add additional environment variables to the C++ config generation docker command: %w", err) + return "", fmt.Errorf("failed to add additional environment variables to the C++ config generation command: %w", err) } r.SetAdditionalEnv(generationEnv) args := []string{o.CppBazelCmd} args = append(args, o.CPPConfigTargets...) if _, err := r.ExecCmd(bazeliskPath, args...); err != nil { - return "", fmt.Errorf("Bazel was unable to build the C++ config generation targets in the toolchain container: %w", err) + return "", fmt.Errorf("Bazel was unable to build the C++ config generation targets in the runner: %w", err) } // Restore the env needed for Bazelisk. r.SetAdditionalEnv(bazeliskEnv) bazelOutputRoot, err := r.ExecCmd(bazeliskPath, "info", "output_base") if err != nil { - return "", fmt.Errorf("unable to determine the build output directory where Bazel produced C++ configs in the toolchain container: %w", err) + return "", fmt.Errorf("unable to determine the build output directory where Bazel produced C++ configs in the runner: %w", err) } cppConfigDir := path.Join(bazelOutputRoot, "external", o.CPPConfigRepo) - log.Printf("Extracting C++ config files generated by Bazel at %q from the toolchain container.", cppConfigDir) + log.Printf("Extracting C++ config files generated by Bazel at %q from the runner.", cppConfigDir) // Restore the old env now that we're done with Bazelisk commands. This is purely to reduce // noise in the logs. @@ -306,7 +306,7 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st // 1. Get a list of symlinks in the config output directory. // 2. Harden each link. // 3. Archive the contents of the config output directory into a tarball. - // 4. Copy the tarball from the container to the local temp directory. + // 4. Copy the tarball from the runner to the local temp directory. var out string if o.ExecOS == "windows" { out, err = r.ExecCmd("cmd", "/r", "dir", filepath.Clean(cppConfigDir), "/a:l", "/b") @@ -332,7 +332,7 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st } resolvedPath, err := r.ExecCmd("readlink", s) if err != nil { - return "", fmt.Errorf("unable to determine what the symlink %q in %q in the toolchain container points to: %w", s, cppConfigDir, err) + return "", fmt.Errorf("unable to determine what the symlink %q in %q in the runner points to: %w", s, cppConfigDir, err) } if _, err := r.ExecCmd("ln", "-f", resolvedPath, s); err != nil { return "", fmt.Errorf("failed to harden symlink %q in %q pointing to %q: %w", s, cppConfigDir, resolvedPath, err) @@ -342,11 +342,11 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st outputTarball := "cpp_configs.tar" // Explicitly use absolute paths to avoid confusion on what's the working directory. outputTarballPath := path.Join(o.TempWorkDir, outputTarball) - outputTarballContainerPath := path.Join(cppProjDir, outputTarball) - if _, err := r.ExecCmd("tar", "-cf", outputTarballContainerPath, "-C", cppConfigDir, "."); err != nil { + outputTarballRunnerPath := path.Join(cppProjDir, outputTarball) + if _, err := r.ExecCmd("tar", "-cf", outputTarballRunnerPath, "-C", cppConfigDir, "."); err != nil { return "", fmt.Errorf("failed to archive the C++ configs into a tarball inside the toolchain container: %w", err) } - if err := r.CopyFrom(outputTarballContainerPath, outputTarballPath); err != nil { + if err := r.CopyFrom(outputTarballRunnerPath, outputTarballPath); err != nil { return "", fmt.Errorf("failed to copy the C++ config tarball out of the toolchain container: %w", err) } log.Printf("Generated C++ configs at %s.", outputTarballPath) @@ -368,20 +368,20 @@ func UsesLocalJavaRuntime(bazelVersion string) (bool, error) { // genJavaConfigs returns a BUILD file containing a Java toolchain rule definition that contains // the following attributes determined by probing details about the JDK version installed in the -// running toolchain container. -// 1. Value of the JAVA_HOME environment variable set in the toolchain image. +// runner. +// 1. Value of the JAVA_HOME environment variable set in the runner or as reported by java binary. // 2. Value of the Java version as reported by the java binary installed in JAVA_HOME inside the -// running toolchain container. +// runner. func genJavaConfigs(r runner.Runner, o *options.Options) (generatedFile, error) { if !o.GenJavaConfigs { return generatedFile{}, nil } javaBin := "java" - imageEnv, err := r.GetEnv() + runnerEnv, err := r.GetEnv() if err != nil { return generatedFile{}, fmt.Errorf("unable to get the environment of the toolchain image to determine JAVA_HOME: %w", err) } - javaHome, ok := imageEnv["JAVA_HOME"] + javaHome, ok := runnerEnv["JAVA_HOME"] if ok && len(javaHome) > 0 { log.Printf("JAVA_HOME was %q.", javaHome) javaBin = path.Join(javaHome, "bin/java") @@ -392,7 +392,7 @@ func genJavaConfigs(r runner.Runner, o *options.Options) (generatedFile, error) // some non-deterministic prefix. out, err := r.ExecCmd(javaBin, "-XshowSettings:properties", "-version") if err != nil { - return generatedFile{}, fmt.Errorf("unable to determine the Java version installed in the toolchain container: %w", err) + return generatedFile{}, fmt.Errorf("unable to determine the Java version installed in the runner: %w", err) } javaVersion := "" for _, line := range strings.Split(out, "\n") { @@ -412,9 +412,13 @@ func genJavaConfigs(r runner.Runner, o *options.Options) (generatedFile, error) } } if len(javaVersion) == 0 { - return generatedFile{}, fmt.Errorf("unable to determine the java version installed in the container by running 'java -XshowSettings:properties' in the container because it didn't return a line that looked like java.version = ") + return generatedFile{}, fmt.Errorf("unable to determine the java version installed in the runner by running 'java -XshowSettings:properties' because it didn't return a line that looked like java.version = ") } log.Printf("Java version: '%s'.", javaVersion) + if len(javaHome) == 0 { + return generatedFile{}, fmt.Errorf("unable to determine the JAVA_HOME installed in the runner: not in env, not returned as line `java.home = ` by 'java -XshowSettings:properties'") + } + log.Printf("Java home: '%s'.", javaHome) usesNewJavaRule, err := UsesLocalJavaRuntime(o.BazelVersion) if err != nil { @@ -770,10 +774,11 @@ func createManifest(o *options.Options) error { } // Extract the sha256 digest from the image name to be included in the manifest. s := imageDigestRegexp.FindStringSubmatch(o.PlatformParams.ToolchainContainer) - if len(s) != 2 { + if len(s) != 2 && o.Runner == "docker" { return fmt.Errorf("failed to extract sha256 digest using regex from image name %q, got %d substrings, want 2", o.PlatformParams.ToolchainContainer, len(s)) + } else if len(s) == 2 { + m.ImageDigest = s[1] } - m.ImageDigest = s[1] // Include the sha256 digest of the configs tarball if output tarball generation was enabled by // actually hashing the contents of the output tarball. if len(o.OutputTarball) != 0 { diff --git a/pkg/runner/host_runner.go b/pkg/runner/host_runner.go index 05527ad5c..3f243de04 100644 --- a/pkg/runner/host_runner.go +++ b/pkg/runner/host_runner.go @@ -39,8 +39,8 @@ type hostRunner struct { additionalEnv map[string]string } -// NewHostRunner creates a new Runner which executes commands directly in host environment. deleteWorkdir -// determines if the Cleanup function on the hostRunner will remove temporary directory +// NewHostRunner creates a new Runner which executes commands directly on the host where rbe_configs_gen runs. +// deleteWorkdir determines if the Cleanup function of the hostRunner will remove temporary directory func NewHostRunner(deleteWorkdir bool) (*hostRunner, error) { workdir, err := ioutil.TempDir("", "host_runner_") @@ -55,7 +55,7 @@ func NewHostRunner(deleteWorkdir bool) (*hostRunner, error) { }, nil } -// execCmd runs the given command and returns the output with whitespace +// ExecCmd runs the given command and returns the output with whitespace // trimmed from the edges. func (r *hostRunner) ExecCmd(cmd string, args ...string) (string, error) { log.Printf("Running: %s", strings.Join(append([]string{cmd}, args...), " ")) @@ -71,7 +71,7 @@ func (r *hostRunner) ExecCmd(cmd string, args ...string) (string, error) { return strings.TrimSpace(string(o)), err } -// cleanup stops the running container if stopContainer was true when the hostRunner was created. +// Cleanup deletes runner temporary files if deleteWorkdir was true when the hostRunner was created. func (r *hostRunner) Cleanup() { if !r.deleteWorkdir { log.Printf("Not deleting workdir %v because the Cleanup option was set to false.", r.globalWorkdir) @@ -83,8 +83,8 @@ func (r *hostRunner) Cleanup() { } } -// copyTo copies the local file at 'src' to the container where 'dst' is the path inside -// the container. d.workdir has no impact on this function. +// CopyTo copies the local file at 'src' to 'dst' on the host +// CopyTo works the same way as CopyFrom for the host runner func (r *hostRunner) CopyTo(src, dst string) error { if _, err := runCmd("cp", src, dst); err != nil { return err @@ -92,8 +92,8 @@ func (r *hostRunner) CopyTo(src, dst string) error { return nil } -// copyFrom extracts the file at 'src' from inside the container and copies it to the path -// 'dst' locally. d.workdir has no impact on this function. +// CopyFrom copies the local file at 'src' to 'dst' on the host +// CopyFrom works the same way as CopyTo for the host runner func (r *hostRunner) CopyFrom(src, dst string) error { if _, err := runCmd("cp", src, dst); err != nil { return err @@ -101,11 +101,10 @@ func (r *hostRunner) CopyFrom(src, dst string) error { return nil } -// getEnv gets the shell environment values from the toolchain container as determined by the -// image config. Env value set or changed by running commands after starting the container aren't +// GetEnv gets the shell environment values from the host. +// Env values set or changed by running commands inside the runner aren't // captured by the return value of this function. -// The return value of this function is a map from env keys to their values. If the image config, -// specifies the same env key multiple times, later values supercede earlier ones. +// The return value of this function is a map from env keys to their values. func (r *hostRunner) GetEnv() (map[string]string, error) { result := make(map[string]string) for _, s := range os.Environ() { From a1c349e18f78cab47a2010fb26926099339d93fd Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Sat, 5 Jun 2021 01:09:37 +0700 Subject: [PATCH 07/10] add DEVELOPER_DIR and SDKROOT --- pkg/rbeconfigsgen/rbeconfigsgen.go | 99 ++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index 1918a4374..17d7a4254 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -296,6 +296,68 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st if err != nil { return "", fmt.Errorf("unable to determine the build output directory where Bazel produced C++ configs in the runner: %w", err) } + + // extract DEVELOPER_DIR for MacOS runner + developerDir := "" + if o.ExecOS == options.OSMacos { + out, err := r.ExecCmd(bazeliskPath, "query", "@local_config_xcode//:host_xcodes", "--output", "build") + if err != nil { + return "", fmt.Errorf("`bazel query @local_config_xcode//:host_xcodes` fails in the runner: %w", err) + } + versionLabel := "" + for _, line := range strings.Split(out, "\n") { + // We're looking for a line that looks like `default = "@local_config_xcode//:",` and we want to + // extract @local_config_xcode//:. + split := strings.SplitN(line, "=", 2) + if len(split) != 2 { + continue + } + key := strings.TrimSpace(split[0]) + val := strings.Trim(strings.TrimSpace(split[1]),"\",") + if key == "default" { + versionLabel = val + } + } + if len(versionLabel) == 0 { + return "", fmt.Errorf("target @local_config_xcode//:host_xcodes has no default version") + } + + out, err = r.ExecCmd(bazeliskPath, "query", versionLabel, "--output", "build") + if err != nil { + return "", fmt.Errorf("`bazel query %s` fails in the runner: %w", versionLabel, err) + } + xcodeVersion := "" + for _, line := range strings.Split(out, "\n") { + // We're looking for a line that looks like `version = "",` and we want to + // extract . + split := strings.SplitN(line, "=", 2) + if len(split) != 2 { + continue + } + key := strings.TrimSpace(split[0]) + val := strings.Trim(strings.TrimSpace(split[1]),"\",") + if key == "version" { + xcodeVersion = val + } + } + if len(xcodeVersion) == 0 { + return "", fmt.Errorf("target %s has no version attr", versionLabel) + } + + xcodeLocatorPath := path.Join(bazelOutputRoot, "external", "local_config_xcode", "xcode-locator-bin") + out, err = r.ExecCmd(xcodeLocatorPath, xcodeVersion) + if err != nil { + return "", fmt.Errorf("`%s %s` fails in the runner: %w", xcodeLocatorPath, xcodeVersion, err) + } + // developerDir will be the last line + outLines := strings.Split(out, "\n") + developerDir = outLines[len(outLines)-1] + if developerDir == "" { + return "", fmt.Errorf("failed to find DEVELOPER_DIR") + } + log.Printf("DEVELOPER_DIR: '%s'.", developerDir) + } + cppConfigDir := path.Join(bazelOutputRoot, "external", o.CPPConfigRepo) log.Printf("Extracting C++ config files generated by Bazel at %q from the runner.", cppConfigDir) @@ -339,6 +401,43 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st } } + if o.ExecOS == options.OSMacos { + // w/a for the issue described here: https://github.com/bazelbuild/bazel/pull/13529 + sedArgs := []string{ + "-I", + "", + "-e", + "s|@local_config_cc_toolchains//:osx_archs.bzl|@bazel_tools//tools/osx/crosstool:osx_archs.bzl|", + path.Join(cppConfigDir, "BUILD"), + } + if _, err := r.ExecCmd("sed", sedArgs...); err != nil { + return "", fmt.Errorf("failed to replace osx_archs.bzl label in BUILD") + } + // add DEVELOPER_DIR and SDKROOT into remote executor env + // for local executions bazel takes care about it, see + // https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/exec/local/XcodeLocalEnvProvider.java + sdkRoot := path.Join(developerDir, "Platforms", + "%{apple_sdk_platform_value}.platform", + "Developer", + "SDKs", + "%{apple_sdk_platform_value}%{apple_sdk_version_override_value}.sdk", + ) + sedArgs = []string{ + "-I", + "", + "-e", + "/cc_toolchain_config(/ a\\\n" + + " extra_env = { \\\n" + + " \"DEVELOPER_DIR\": \"" + developerDir + "\",\\\n" + + " \"SDKROOT\": \"" + sdkRoot + "\",\\\n" + + " },", + path.Join(cppConfigDir, "BUILD"), + } + if _, err := r.ExecCmd("sed", sedArgs...); err != nil { + return "", fmt.Errorf("failed to add extra_env to cc_toolchain_config in BUILD") + } + } + outputTarball := "cpp_configs.tar" // Explicitly use absolute paths to avoid confusion on what's the working directory. outputTarballPath := path.Join(o.TempWorkDir, outputTarball) From d01f1168a49a3f24214b115a402351638e820722 Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Wed, 9 Jun 2021 03:23:44 +0700 Subject: [PATCH 08/10] exclude cc/WORKSPACE when output is directory --- pkg/rbeconfigsgen/rbeconfigsgen.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index 17d7a4254..3680d6cb9 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -442,7 +442,8 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st // Explicitly use absolute paths to avoid confusion on what's the working directory. outputTarballPath := path.Join(o.TempWorkDir, outputTarball) outputTarballRunnerPath := path.Join(cppProjDir, outputTarball) - if _, err := r.ExecCmd("tar", "-cf", outputTarballRunnerPath, "-C", cppConfigDir, "."); err != nil { + if _, err := r.ExecCmd("tar","-cf", outputTarballRunnerPath, "-C", cppConfigDir, + "--exclude", "WORKSPACE", "."); err != nil { return "", fmt.Errorf("failed to archive the C++ configs into a tarball inside the toolchain container: %w", err) } if err := r.CopyFrom(outputTarballRunnerPath, outputTarballPath); err != nil { From a562d8992379db3f0dd39e25a7bfee1055c61f83 Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Wed, 9 Jun 2021 03:30:35 +0700 Subject: [PATCH 09/10] Revert "exclude cc/WORKSPACE when output is directory" This reverts commit d01f1168a49a3f24214b115a402351638e820722. --- pkg/rbeconfigsgen/rbeconfigsgen.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index 3680d6cb9..17d7a4254 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -442,8 +442,7 @@ func genCppConfigs(r runner.Runner, o *options.Options, bazeliskPath string) (st // Explicitly use absolute paths to avoid confusion on what's the working directory. outputTarballPath := path.Join(o.TempWorkDir, outputTarball) outputTarballRunnerPath := path.Join(cppProjDir, outputTarball) - if _, err := r.ExecCmd("tar","-cf", outputTarballRunnerPath, "-C", cppConfigDir, - "--exclude", "WORKSPACE", "."); err != nil { + if _, err := r.ExecCmd("tar", "-cf", outputTarballRunnerPath, "-C", cppConfigDir, "."); err != nil { return "", fmt.Errorf("failed to archive the C++ configs into a tarball inside the toolchain container: %w", err) } if err := r.CopyFrom(outputTarballRunnerPath, outputTarballPath); err != nil { From 13219a02d4e90cf5039bec1ef2c20dff675f298c Mon Sep 17 00:00:00 2001 From: Andrey Kartashov Date: Wed, 9 Jun 2021 03:39:53 +0700 Subject: [PATCH 10/10] exclude cc/WORKSPACE when output is directory --- pkg/rbeconfigsgen/rbeconfigsgen.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/rbeconfigsgen/rbeconfigsgen.go b/pkg/rbeconfigsgen/rbeconfigsgen.go index 17d7a4254..cd016c88d 100644 --- a/pkg/rbeconfigsgen/rbeconfigsgen.go +++ b/pkg/rbeconfigsgen/rbeconfigsgen.go @@ -716,6 +716,9 @@ func copyCppConfigsToOutputDir(outDir string, cppConfigsTarball string) error { } switch h.Typeflag { case tar.TypeReg: + if strings.HasSuffix(h.Name, "WORKSPACE") { + break + } filePath := path.Join(outDir, h.Name) dirPath := path.Dir(filePath) if err := os.MkdirAll(dirPath, os.ModePerm); err != nil {