Skip to content

Commit

Permalink
Allow changing the CPP Toolchain. (#955)
Browse files Browse the repository at this point in the history
The cpp default toolchain on windows was hardset to cc-compiler-k8, which
is linux exclusive. Additionally it's quite common to need to change the
compiler being used.

The drawback of this approach is that it potentially creates many repetitive
equal cc_toolchains if you use the same container with multiple toolings installed.

Long term we could add multiple cpp toolchains to configs/, but so far the limited
amount of repetition isn't harmful.
  • Loading branch information
rubensf authored Mar 3, 2021
1 parent 883a619 commit 27f2db2
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 30 deletions.
34 changes: 18 additions & 16 deletions cmd/rbe_configs_gen/rbe_configs_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ var (
outputManifest = flag.String("output_manifest", "", "(Optional) Generate a JSON file with details about the generated configs.")

// Optional input arguments that affect config generation for either C++ or Java configs.
genCppConfigs = flag.Bool("generate_cpp_configs", true, "(Optional) Generate C++ configs. Defaults to true.")
cppEnvJSON = flag.String("cpp_env_json", "", "(Optional) JSON file containing a str -> str dict of environment variables to be set when generating C++ configs inside the toolchain container. This replaces any exec OS specific defaults that would usually be applied.")
genJavaConfigs = flag.Bool("generate_java_configs", true, "(Optional) Generate Java configs. Defaults to true.")
genCppConfigs = flag.Bool("generate_cpp_configs", true, "(Optional) Generate C++ configs. Defaults to true.")
cppEnvJSON = flag.String("cpp_env_json", "", "(Optional) JSON file containing a str -> str dict of environment variables to be set when generating C++ configs inside the toolchain container. This replaces any exec OS specific defaults that would usually be applied.")
cppToolchainTarget = flag.String("cpp_toolchain_target", "", "(Optional) Set the CPP toolchain target. When exec_os is linux, the default is cc-compiler-k8. When exec_os is windows, the default is cc-compiler-x64_windows.")
genJavaConfigs = flag.Bool("generate_java_configs", true, "(Optional) Generate Java configs. Defaults to true.")

// Other misc arguments.
tempWorkDir = flag.String("temp_work_dir", "", "(Optional) Temporary directory to use to store intermediate files. Defaults to a temporary directory automatically allocated by the OS. The temporary working directory is deleted at the end unless --cleanup=false is specified.")
Expand Down Expand Up @@ -146,19 +147,20 @@ func main() {
}

o := rbeconfigsgen.Options{
BazelVersion: *bazelVersion,
ToolchainContainer: *toolchainContainer,
ExecOS: *execOS,
TargetOS: *targetOS,
OutputTarball: *outputTarball,
OutputSourceRoot: *outputSrcRoot,
OutputConfigPath: *outputConfigPath,
OutputManifest: *outputManifest,
GenCPPConfigs: *genCppConfigs,
CppGenEnvJSON: *cppEnvJSON,
GenJavaConfigs: *genJavaConfigs,
TempWorkDir: *tempWorkDir,
Cleanup: *cleanup,
BazelVersion: *bazelVersion,
ToolchainContainer: *toolchainContainer,
ExecOS: *execOS,
TargetOS: *targetOS,
OutputTarball: *outputTarball,
OutputSourceRoot: *outputSrcRoot,
OutputConfigPath: *outputConfigPath,
OutputManifest: *outputManifest,
GenCPPConfigs: *genCppConfigs,
CppGenEnvJSON: *cppEnvJSON,
CPPToolchainTargetName: *cppToolchainTarget,
GenJavaConfigs: *genJavaConfigs,
TempWorkDir: *tempWorkDir,
Cleanup: *cleanup,
}

result := true
Expand Down
32 changes: 22 additions & 10 deletions pkg/rbeconfigsgen/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package rbeconfigsgen
import (
"fmt"
"log"
"path"
"strings"

"github.com/bazelbuild/bazelisk/core"
Expand Down Expand Up @@ -51,8 +52,8 @@ type Options struct {
// The manifest aims to be easily parseable by shell utilities like grep/sed.
OutputManifest string
// PlatformParams specify platform specific constraints used to generate a BUILD file with the
// toolchain & platform targets in the generated configs. It's recommended to use the defaults
// corresponding to the ExecOS.
// toolchain & platform targets in the generated configs. This is set to default values and not
// directly configurable.
PlatformParams *PlatformToolchainsTemplateParams

// C++ Config generation options.
Expand All @@ -79,6 +80,8 @@ type Options struct {
// command to generate C++ configs inside the toolchain container. Only one of CppGenEnv or
// CppGenEnvJSON can be specified.
CppGenEnvJSON string
// CPPToolchainTarget is the toolchain to be used by the cpp configs.
CPPToolchainTargetName string

// Java config generation options.
// GenJavaConfigs determines whether Java configs are generated.
Expand All @@ -94,11 +97,12 @@ type Options struct {
// DefaultOptions are some option values that are populated as default values for certain fields
// of "Options". See "Options" for explanation on what the fields mean.
type DefaultOptions struct {
PlatformParams PlatformToolchainsTemplateParams
CPPConfigTargets []string
CPPConfigRepo string
CppBazelCmd string
CppGenEnv map[string]string
PlatformParams PlatformToolchainsTemplateParams
CPPConfigTargets []string
CPPConfigRepo string
CppBazelCmd string
CppGenEnv map[string]string
CPPToolchainTargetName string
}

const (
Expand Down Expand Up @@ -144,6 +148,7 @@ var (
"CC": "clang",
"CC_TOOLCHAIN_NAME": "linux_gnu_x86",
},
CPPToolchainTargetName: "cc-compiler-k8",
},
OSWindows: {
PlatformParams: PlatformToolchainsTemplateParams{
Expand All @@ -157,9 +162,10 @@ var (
},
OSFamily: "Windows",
},
CPPConfigTargets: []string{"@local_config_cc//..."},
CPPConfigRepo: "local_config_cc",
CppBazelCmd: "query",
CPPConfigTargets: []string{"@local_config_cc//..."},
CPPConfigRepo: "local_config_cc",
CppBazelCmd: "query",
CPPToolchainTargetName: "cc-compiler-x64_windows",
},
}
)
Expand Down Expand Up @@ -190,6 +196,9 @@ func (o *Options) ApplyDefaults(os string) error {
if len(o.CppGenEnv) == 0 && len(o.CppGenEnvJSON) == 0 {
o.CppGenEnv = dopts.CppGenEnv
}
if o.CPPToolchainTargetName == "" {
o.CPPToolchainTargetName = dopts.CPPToolchainTargetName
}
return nil
}

Expand Down Expand Up @@ -234,6 +243,9 @@ func (o *Options) Validate() error {
if o.OutputSourceRoot == "" && o.OutputConfigPath != "" {
return fmt.Errorf("OutputSourceRoot is required because OutputConfigPath was specified")
}
if path.IsAbs(o.OutputConfigPath) {
return fmt.Errorf("OutputConfigPath should be a relative path")
}
if o.PlatformParams == nil {
return fmt.Errorf("PlatformParams was not initialized")
}
Expand Down
13 changes: 9 additions & 4 deletions pkg/rbeconfigsgen/rbeconfigsgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,13 @@ func processTempDir(o *Options) error {
return nil
}

func genCppToolchainTarget(o *Options) string {
if o.OutputConfigPath != "" {
return fmt.Sprintf("//%s/cc:%s", strings.ReplaceAll(o.OutputConfigPath, "\\", "/"), o.CPPToolchainTargetName)
}
return fmt.Sprintf("//cc:%s", o.CPPToolchainTargetName)
}

// 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) {
Expand All @@ -656,11 +663,9 @@ func genConfigBuild(o *Options) (generatedFile, error) {
}
// Populate the C++ toolchain target if C++ config generation is enabled.
if o.GenCPPConfigs {
o.PlatformParams.CppToolchainTarget = "//cc:cc-compiler-k8"
if o.OutputConfigPath != "" {
o.PlatformParams.CppToolchainTarget = fmt.Sprintf("//%s/cc:cc-compiler-k8", path.Clean(o.OutputConfigPath))
}
o.PlatformParams.CppToolchainTarget = genCppToolchainTarget(o)
} else {
o.PlatformParams.CppToolchainTarget = ""
log.Printf("Not generating a toolchain target to be used for the C++ Crosstool top because C++ config generation is disabled.")
}
buf := bytes.NewBuffer(nil)
Expand Down
86 changes: 86 additions & 0 deletions pkg/rbeconfigsgen/rbeconfigsgen_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// 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 (
"testing"
)

func TestGenCppToolchainTarget(t *testing.T) {
tests := []struct {
name string
want string
opt *Options
}{
{
name: "No options, linux, choose default",
want: "//cc:cc-compiler-k8",
opt: &Options{
ExecOS: "linux",
},
}, {
name: "No options, windows, choose default",
want: "//cc:cc-compiler-x64_windows",
opt: &Options{
ExecOS: "windows",
},
}, {
name: "Windows pick compiler",
want: "//cc:cc-compiler-x64_windows_mingw",
opt: &Options{
ExecOS: "windows",
CPPToolchainTargetName: "cc-compiler-x64_windows_mingw",
},
}, {
name: "Linux pick output path",
want: "//configs/foo/bar/cc:cc-compiler-k8",
opt: &Options{
ExecOS: "linux",
OutputConfigPath: "configs/foo/bar",
},
}, {
name: "Windows pick output path and compiler",
want: "//configs/fizz/buzz/cc:foobar-cc-good",
opt: &Options{
ExecOS: "windows",
OutputConfigPath: "configs/fizz/buzz",
CPPToolchainTargetName: "foobar-cc-good",
},
}, {
name: "Windows pick backslash style path and compiler",
want: "//configs/fizz/buzz/cc:foobar-cc-good",
opt: &Options{
ExecOS: "windows",
OutputConfigPath: "configs\\fizz\\buzz",
CPPToolchainTargetName: "foobar-cc-good",
},
},
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
if err := tc.opt.ApplyDefaults(tc.opt.ExecOS); err != nil {
t.Fatalf("ApplyDefaults: Failed to apply defaults=%v", err)
}
// We skip validation since we don't set all options required for
// regular execution.
if got := genCppToolchainTarget(tc.opt); got != tc.want {
t.Fatalf("GenCppToolchainTarget: %v, wanted %v", got, tc.want)
}
})
}
}

0 comments on commit 27f2db2

Please sign in to comment.