From 60e8bcec61269c6648dc40c8094fbf303b8a02aa Mon Sep 17 00:00:00 2001 From: Ricky Stewart Date: Mon, 17 Jul 2023 15:09:52 -0500 Subject: [PATCH] bazel,dev: in `dev`, handle different `--compilation_mode`s correctly... ... and switch our default compilation mode to `dbg`. Under `fastbuild`, built binaries are stripped. This is not the case with `dbg`. Have `dev doctor` recommend using `dbg`, and either way make `dev` resilient to whatever kind of `--compilation_mode` you're using. In principle or theory `dbg` is slower than `fastbuild`, but for the Go compiler it generates debuggable binaries by default and you have to opt-in to stripping, so it shoould make no real difference. Now, we think of the `--compilation_mode` simply as follows: `dbg` is the default that everyone is opted into by default unless they explicitly set `-c opt` (we use this for release). For now I don't see a reason anyone would need `fastbuild`. Epic: CRDB-17171 Release note: None Closes: #106820 --- .bazelrc | 1 + .../cockroach/ci/builds/build_impl.sh | 2 +- .../nightlies/pebble_nightly_common.sh | 12 +++++----- .../nightlies/pebble_nightly_race_common.sh | 8 +++---- .../nightlies/roachtest_compile_bits.sh | 8 +++---- build/toolchains/BUILD.bazel | 7 ------ dev | 2 +- pkg/cmd/dev/build.go | 24 ++++++++++++++----- pkg/cmd/dev/doctor.go | 2 +- pkg/cmd/dev/generate.go | 2 +- pkg/cmd/dev/roachprod_stress.go | 2 +- pkg/cmd/dev/ui.go | 2 +- pkg/cmd/dev/util.go | 12 ++++++---- 13 files changed, 46 insertions(+), 38 deletions(-) diff --git a/.bazelrc b/.bazelrc index c6a143a4ad4c..67594bbdcd6a 100644 --- a/.bazelrc +++ b/.bazelrc @@ -36,6 +36,7 @@ build --define gotags=bazel,gss build --experimental_proto_descriptor_sets_include_source_info build --incompatible_strict_action_env --incompatible_enable_cc_toolchain_resolution build --symlink_prefix=_bazel/ +build -c dbg common --experimental_allow_tags_propagation test --config=test --experimental_ui_max_stdouterr_bytes=10485760 build --ui_event_filters=-DEBUG diff --git a/build/teamcity/cockroach/ci/builds/build_impl.sh b/build/teamcity/cockroach/ci/builds/build_impl.sh index 7c5b32fe3576..a37c1410bd9a 100755 --- a/build/teamcity/cockroach/ci/builds/build_impl.sh +++ b/build/teamcity/cockroach/ci/builds/build_impl.sh @@ -38,7 +38,7 @@ fi bazel build //pkg/cmd/bazci --config=ci BAZEL_BIN=$(bazel info bazel-bin --config=ci) -"$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci" -- build -c opt \ +"$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci" -- build \ --config "$CONFIG" --config ci $EXTRA_ARGS \ //pkg/cmd/cockroach-short //pkg/cmd/cockroach \ //pkg/cmd/cockroach-sql \ diff --git a/build/teamcity/cockroach/nightlies/pebble_nightly_common.sh b/build/teamcity/cockroach/nightlies/pebble_nightly_common.sh index 80464b7f651f..b4c6bdb53939 100755 --- a/build/teamcity/cockroach/nightlies/pebble_nightly_common.sh +++ b/build/teamcity/cockroach/nightlies/pebble_nightly_common.sh @@ -27,8 +27,8 @@ mkdir -p "$PWD/bin" chmod o+rwx "$PWD/bin" # Build the roachtest binary. -bazel build //pkg/cmd/roachtest --config ci -c opt -BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt) +bazel build //pkg/cmd/roachtest --config ci +BAZEL_BIN=$(bazel info bazel-bin --config ci) cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin chmod a+w bin/roachtest @@ -39,8 +39,8 @@ chmod a+w bin/roachtest bazel run @go_sdk//:bin/go get github.com/cockroachdb/pebble@latest NEW_DEPS_BZL_CONTENT=$(bazel run //pkg/cmd/mirror/go:mirror) echo "$NEW_DEPS_BZL_CONTENT" > DEPS.bzl -bazel build @com_github_cockroachdb_pebble//cmd/pebble --config ci -c opt -BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt) +bazel build @com_github_cockroachdb_pebble//cmd/pebble --config ci +BAZEL_BIN=$(bazel info bazel-bin --config ci) cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/cmd/pebble/pebble_/pebble ./pebble.linux chmod a+w ./pebble.linux @@ -67,8 +67,8 @@ function prepare_datadir() { # Build the mkbench tool from within the Pebble repo. This is used to parse # the benchmark data. function build_mkbench() { - bazel build @com_github_cockroachdb_pebble//internal/mkbench --config ci -c opt - BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt) + bazel build @com_github_cockroachdb_pebble//internal/mkbench --config ci + BAZEL_BIN=$(bazel info bazel-bin --config ci) cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/internal/mkbench/mkbench_/mkbench . chmod a+w mkbench } diff --git a/build/teamcity/cockroach/nightlies/pebble_nightly_race_common.sh b/build/teamcity/cockroach/nightlies/pebble_nightly_race_common.sh index 631ce2e1b241..43f2e0950345 100755 --- a/build/teamcity/cockroach/nightlies/pebble_nightly_race_common.sh +++ b/build/teamcity/cockroach/nightlies/pebble_nightly_race_common.sh @@ -27,8 +27,8 @@ mkdir -p "$PWD/bin" chmod o+rwx "$PWD/bin" # Build the roachtest binary. -bazel build //pkg/cmd/roachtest --config ci -c opt -BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt) +bazel build //pkg/cmd/roachtest --config ci +BAZEL_BIN=$(bazel info bazel-bin --config ci) cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin chmod a+w bin/roachtest @@ -39,8 +39,8 @@ chmod a+w bin/roachtest bazel run @go_sdk//:bin/go get github.com/cockroachdb/pebble@latest NEW_DEPS_BZL_CONTENT=$(bazel run //pkg/cmd/mirror/go:mirror) echo "$NEW_DEPS_BZL_CONTENT" > DEPS.bzl -bazel build @com_github_cockroachdb_pebble//cmd/pebble --config race --config ci -c opt -BAZEL_BIN=$(bazel info bazel-bin --config race --config ci -c opt) +bazel build @com_github_cockroachdb_pebble//cmd/pebble --config race --config ci +BAZEL_BIN=$(bazel info bazel-bin --config race --config ci) cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/cmd/pebble/pebble_/pebble ./pebble.linux chmod a+w ./pebble.linux diff --git a/build/teamcity/cockroach/nightlies/roachtest_compile_bits.sh b/build/teamcity/cockroach/nightlies/roachtest_compile_bits.sh index 60e33c81e8f2..1d3405b6d56c 100644 --- a/build/teamcity/cockroach/nightlies/roachtest_compile_bits.sh +++ b/build/teamcity/cockroach/nightlies/roachtest_compile_bits.sh @@ -35,21 +35,21 @@ for platform in "${cross_builds[@]}"; do echo "Building $platform, os=$os, arch=$arch..." # Build cockroach, workload and geos libs. - bazel build --config $platform --config ci -c opt --config force_build_cdeps \ + bazel build --config $platform --config ci --config force_build_cdeps \ //pkg/cmd/cockroach //pkg/cmd/workload \ //c-deps:libgeos - BAZEL_BIN=$(bazel info bazel-bin --config $platform --config ci -c opt) + BAZEL_BIN=$(bazel info bazel-bin --config $platform --config ci) # N.B. roachtest is built once, for the host architecture. if [[ $os == "linux" && $arch == $host_arch ]]; then - bazel build --config $platform --config ci -c opt //pkg/cmd/roachtest + bazel build --config $platform --config ci //pkg/cmd/roachtest cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin/roachtest # Make it writable to simplify cleanup and copying (e.g., scp retry). chmod a+w bin/roachtest fi # Build cockroach-short with assertions enabled. - bazel build --config $platform --config ci -c opt //pkg/cmd/cockroach-short --crdb_test + bazel build --config $platform --config ci //pkg/cmd/cockroach-short --crdb_test # Copy the binaries. cp $BAZEL_BIN/pkg/cmd/cockroach/cockroach_/cockroach bin/cockroach.$os-$arch cp $BAZEL_BIN/pkg/cmd/workload/workload_/workload bin/workload.$os-$arch diff --git a/build/toolchains/BUILD.bazel b/build/toolchains/BUILD.bazel index 373850ad24a8..e1b00f28f2c2 100644 --- a/build/toolchains/BUILD.bazel +++ b/build/toolchains/BUILD.bazel @@ -404,13 +404,6 @@ config_setting( }, ) -config_setting( - name = "opt", - values = { - "compilation_mode": "opt", - }, -) - bool_flag( name = "nogo_flag", build_setting_default = False, diff --git a/dev b/dev index 8b7a8e569191..53101ce1b9ea 100755 --- a/dev +++ b/dev @@ -8,7 +8,7 @@ fi set -euo pipefail # Bump this counter to force rebuilding `dev` on all machines. -DEV_VERSION=79 +DEV_VERSION=80 THIS_DIR=$(cd "$(dirname "$0")" && pwd) BINARY_DIR=$THIS_DIR/bin/dev-versions diff --git a/pkg/cmd/dev/build.go b/pkg/cmd/dev/build.go index 9a46e511baff..af12af411f17 100644 --- a/pkg/cmd/dev/build.go +++ b/pkg/cmd/dev/build.go @@ -170,6 +170,8 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error { return err } args = append(args, additionalBazelArgs...) + configArgs := getConfigArgs(args) + configArgs = append(configArgs, getConfigArgs(additionalBazelArgs)...) if cross == "" { // Do not log --build_event_binary_file=... because it is not relevant to the actual call @@ -183,7 +185,7 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error { if err := d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...); err != nil { return err } - return d.stageArtifacts(ctx, buildTargets) + return d.stageArtifacts(ctx, buildTargets, configArgs) } volume := mustGetFlagString(cmd, volumeFlag) cross = "cross" + cross @@ -252,7 +254,9 @@ func (d *dev) crossBuild( return err } -func (d *dev) stageArtifacts(ctx context.Context, targets []buildTarget) error { +func (d *dev) stageArtifacts( + ctx context.Context, targets []buildTarget, configArgs []string, +) error { workspace, err := d.getWorkspace(ctx) if err != nil { return err @@ -261,7 +265,7 @@ func (d *dev) stageArtifacts(ctx context.Context, targets []buildTarget) error { if err = d.os.MkdirAll(path.Join(workspace, "bin")); err != nil { return err } - bazelBin, err := d.getBazelBin(ctx) + bazelBin, err := d.getBazelBin(ctx, configArgs) if err != nil { return err } @@ -453,11 +457,19 @@ func (d *dev) getBasicBuildArgs( return args, buildTargets, nil } -// Given a list of Bazel arguments, find the ones starting with --config= and -// return them. +// Given a list of Bazel arguments, find the ones that represent a "config" +// (either --config or -c) and return all of these. This is used to find +// the appropriate bazel-bin for any invocation. func getConfigArgs(args []string) (ret []string) { + var addNext bool for _, arg := range args { - if strings.HasPrefix(arg, "--config=") { + if addNext { + ret = append(ret, arg) + addNext = false + } else if arg == "--config" || arg == "--compilation_mode" || arg == "-c" { + ret = append(ret, arg) + addNext = true + } else if strings.HasPrefix(arg, "--config=") || strings.HasPrefix(arg, "--compilation_mode=") { ret = append(ret, arg) } } diff --git a/pkg/cmd/dev/doctor.go b/pkg/cmd/dev/doctor.go index 2aa8285e6324..35a3f4d7917e 100644 --- a/pkg/cmd/dev/doctor.go +++ b/pkg/cmd/dev/doctor.go @@ -503,7 +503,7 @@ func (d *dev) doctor(cmd *cobra.Command, _ []string) error { if err != nil { return err } - bazelBin, err := d.getBazelBin(ctx) + bazelBin, err := d.getBazelBin(ctx, []string{}) if err != nil { return err } diff --git a/pkg/cmd/dev/generate.go b/pkg/cmd/dev/generate.go index 202562466165..d7bf520587b4 100644 --- a/pkg/cmd/dev/generate.go +++ b/pkg/cmd/dev/generate.go @@ -315,7 +315,7 @@ func (d *dev) generateJs(cmd *cobra.Command) error { return fmt.Errorf("building JS development prerequisites: %w", err) } - bazelBin, err := d.getBazelBin(ctx) + bazelBin, err := d.getBazelBin(ctx, []string{}) if err != nil { return err } diff --git a/pkg/cmd/dev/roachprod_stress.go b/pkg/cmd/dev/roachprod_stress.go index abe737b34c10..febf81414062 100644 --- a/pkg/cmd/dev/roachprod_stress.go +++ b/pkg/cmd/dev/roachprod_stress.go @@ -120,7 +120,7 @@ func (d *dev) roachprodStress(cmd *cobra.Command, commandLine []string) error { if _, err := d.exec.CommandContextSilent(ctx, "bazel", args...); err != nil { return err } - if err := d.stageArtifacts(ctx, roachprodStressTarget); err != nil { + if err := d.stageArtifacts(ctx, roachprodStressTarget, []string{}); err != nil { return err } diff --git a/pkg/cmd/dev/ui.go b/pkg/cmd/dev/ui.go index f2cd4269a05c..b472d66666fa 100644 --- a/pkg/cmd/dev/ui.go +++ b/pkg/cmd/dev/ui.go @@ -463,7 +463,7 @@ func makeUICleanCmd(d *dev) *cobra.Command { // // See https://github.com/bazelbuild/rules_nodejs/issues/2028 func arrangeFilesForWatchers(d *dev, ossOnly bool) error { - bazelBin, err := d.getBazelBin(d.cli.Context()) + bazelBin, err := d.getBazelBin(d.cli.Context(), []string{}) if err != nil { return err } diff --git a/pkg/cmd/dev/util.go b/pkg/cmd/dev/util.go index 24c090444db8..dc22087b8390 100644 --- a/pkg/cmd/dev/util.go +++ b/pkg/cmd/dev/util.go @@ -102,7 +102,7 @@ func mustGetFlagDuration(cmd *cobra.Command, name string) time.Duration { return val } -func (d *dev) getBazelInfo(ctx context.Context, key string) (string, error) { +func (d *dev) getBazelInfo(ctx context.Context, key string, extraArgs []string) (string, error) { args := []string{"info", key, "--color=no"} out, err := d.exec.CommandContextSilent(ctx, "bazel", args...) if err != nil { @@ -117,15 +117,17 @@ func (d *dev) getWorkspace(ctx context.Context) (string, error) { return os.Getwd() } - return d.getBazelInfo(ctx, "workspace") + return d.getBazelInfo(ctx, "workspace", []string{}) } -func (d *dev) getBazelBin(ctx context.Context) (string, error) { - return d.getBazelInfo(ctx, "bazel-bin") +// The second argument should be the relevant "config args", namely Bazel arguments +// that are --config or --compilation_mode arguments (see getConfigArgs()). +func (d *dev) getBazelBin(ctx context.Context, configArgs []string) (string, error) { + return d.getBazelInfo(ctx, "bazel-bin", configArgs) } func (d *dev) getExecutionRoot(ctx context.Context) (string, error) { - return d.getBazelInfo(ctx, "execution_root") + return d.getBazelInfo(ctx, "execution_root", []string{}) } // getDevBin returns the path to the running dev executable.