Skip to content

Commit

Permalink
Merge #123241
Browse files Browse the repository at this point in the history
123241: dev: fix staging behavior from non-default configurations r=rail a=rickystewart

We added support for building in non-default configurations/compilation modes in a previous PR, but this was buggy and we were never actually using the configuration arguments in `bazel info`, so staging was always happening from the `fastbuild` location. This PR fixes that buggy behavior, so staging will happen from the appropriate source location.

Epic: CRDB-17171
Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
craig[bot] and rickystewart committed Apr 29, 2024
2 parents 37e0fc5 + 678be4e commit 4e178de
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 7 deletions.
2 changes: 1 addition & 1 deletion dev
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fi
set -euo pipefail

# Bump this counter to force rebuilding `dev` on all machines.
DEV_VERSION=92
DEV_VERSION=93

THIS_DIR=$(cd "$(dirname "$0")" && pwd)
BINARY_DIR=$THIS_DIR/bin/dev-versions
Expand Down
3 changes: 1 addition & 2 deletions pkg/cmd/dev/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error {
}
args = append(args, additionalBazelArgs...)
configArgs := getConfigArgs(args)
configArgs = append(configArgs, getConfigArgs(additionalBazelArgs)...)

if err := d.assertNoLinkedNpmDeps(buildTargets); err != nil {
return err
Expand Down Expand Up @@ -295,7 +294,7 @@ func (d *dev) stageArtifacts(
}
var geosDir string
if archived != "" {
execRoot, err := d.getExecutionRoot(ctx)
execRoot, err := d.getExecutionRoot(ctx, configArgs)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/testdata/datadriven/dev-build
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,4 @@ dev build tests
bazel build //pkg:all_tests --config=test --build_event_binary_file=/tmp/path
bazel info workspace --color=no
mkdir crdb-checkout/bin
bazel info bazel-bin --color=no
bazel info bazel-bin --color=no --config=test
2 changes: 1 addition & 1 deletion pkg/cmd/dev/testdata/recorderdriven/dev-build
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ bazel query pkg/roachpb:roachpb_test --output=label_kind
bazel build //pkg/roachpb:roachpb_test --config=test --build_event_binary_file=/tmp/path
bazel info workspace --color=no
mkdir crdb-checkout/bin
bazel info bazel-bin --color=no
bazel info bazel-bin --color=no --config=test

# TODO(irfansharif): This test case is skipped -- it's too verbose given it
# scans through the sandbox for each generated file and copies them over
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/dev/testdata/recorderdriven/dev-build.rec
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ bazel build //pkg/roachpb:roachpb_test --config=test --build_event_binary_file=/
mkdir crdb-checkout/bin
----

bazel info bazel-bin --color=no --config=test
----
/path/to/bazel/bin
5 changes: 3 additions & 2 deletions pkg/cmd/dev/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func mustGetFlagDuration(cmd *cobra.Command, name string) time.Duration {

func (d *dev) getBazelInfo(ctx context.Context, key string, extraArgs []string) (string, error) {
args := []string{"info", key, "--color=no"}
args = append(args, extraArgs...)
out, err := d.exec.CommandContextSilent(ctx, "bazel", args...)
if err != nil {
return "", err
Expand All @@ -126,8 +127,8 @@ func (d *dev) getBazelBin(ctx context.Context, configArgs []string) (string, err
return d.getBazelInfo(ctx, "bazel-bin", configArgs)
}

func (d *dev) getExecutionRoot(ctx context.Context) (string, error) {
return d.getBazelInfo(ctx, "execution_root", []string{})
func (d *dev) getExecutionRoot(ctx context.Context, configArgs []string) (string, error) {
return d.getBazelInfo(ctx, "execution_root", configArgs)
}

// getArchivedCdepString returns a non-empty string iff the force_build_cdeps
Expand Down

0 comments on commit 4e178de

Please sign in to comment.