From 3309de977b7fdde69957e2970318b5d94a533968 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 + 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 +++++++----- 7 files changed, 30 insertions(+), 15 deletions(-) diff --git a/.bazelrc b/.bazelrc index b347fb7ba2c7..fcb27ebcf0a8 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:dev --strip=never common --experimental_allow_tags_propagation test --config=test --experimental_ui_max_stdouterr_bytes=10485760 build --ui_event_filters=-DEBUG diff --git a/pkg/cmd/dev/build.go b/pkg/cmd/dev/build.go index d197fa41fc64..af703ad4c795 100644 --- a/pkg/cmd/dev/build.go +++ b/pkg/cmd/dev/build.go @@ -172,6 +172,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 err := d.assertNoLinkedNpmDeps(buildTargets); err != nil { return err @@ -189,7 +191,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 @@ -258,7 +260,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 @@ -267,7 +271,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 } @@ -459,11 +463,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 fa0fcc00d6c7..abd1fd462bfb 100644 --- a/pkg/cmd/dev/generate.go +++ b/pkg/cmd/dev/generate.go @@ -303,7 +303,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 08d82291153b..73bc4211e9a3 100644 --- a/pkg/cmd/dev/ui.go +++ b/pkg/cmd/dev/ui.go @@ -675,7 +675,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 0769302e8416..b552c980e179 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.