Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bazel,dev: fix staging behavior from non-default configurations #130831

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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