Skip to content

Commit

Permalink
Merge pull request #130831 from rickystewart/backport23.1-106944-123241
Browse files Browse the repository at this point in the history
bazel,dev: fix staging behavior from non-default configurations
  • Loading branch information
rickystewart authored Sep 16, 2024
2 parents 01738eb + e2ba593 commit 6a51c13
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 19 deletions.
1 change: 1 addition & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 18 additions & 7 deletions pkg/cmd/dev/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error {
return err
}
args = append(args, additionalBazelArgs...)
configArgs := getConfigArgs(args)

if err := d.assertNoLinkedNpmDeps(buildTargets); err != nil {
return err
Expand All @@ -189,7 +190,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
Expand Down Expand Up @@ -258,7 +259,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
Expand All @@ -267,7 +270,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
}
Expand All @@ -288,7 +291,7 @@ func (d *dev) stageArtifacts(ctx context.Context, targets []buildTarget) error {
}
var geosDir string
if archived != "" {
execRoot, err := d.getExecutionRoot(ctx)
execRoot, err := d.getExecutionRoot(ctx, configArgs)
if err != nil {
return err
}
Expand Down Expand Up @@ -459,11 +462,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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/roachprod_stress.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

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 @@ -80,4 +80,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
2 changes: 1 addition & 1 deletion pkg/cmd/dev/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
15 changes: 9 additions & 6 deletions pkg/cmd/dev/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,9 @@ 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"}
args = append(args, extraArgs...)
out, err := d.exec.CommandContextSilent(ctx, "bazel", args...)
if err != nil {
return "", err
Expand All @@ -117,15 +118,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")
func (d *dev) getExecutionRoot(ctx context.Context, configArgs []string) (string, error) {
return d.getBazelInfo(ctx, "execution_root", configArgs)
}

// getDevBin returns the path to the running dev executable.
Expand Down

0 comments on commit 6a51c13

Please sign in to comment.