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.

Sign up for GitHub

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

Jump to bottom

bazel,dev: in dev, handle different --compilation_modes correctly... #106944

Merged
merged 1 commit into from
Jul 18, 2023
Merged

bazel,dev: in dev, handle different --compilation_modes correctly... #106944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
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 -c dbg
common --experimental_allow_tags_propagation
test --config=test --experimental_ui_max_stdouterr_bytes=10485760
build --ui_event_filters=-DEBUG
Expand Down
2 changes: 1 addition & 1 deletion build/teamcity/cockroach/ci/builds/build_impl.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
12 changes: 6 additions & 6 deletions build/teamcity/cockroach/nightlies/pebble_nightly_common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions build/teamcity/cockroach/nightlies/pebble_nightly_race_common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
8 changes: 4 additions & 4 deletions build/teamcity/cockroach/nightlies/roachtest_compile_bits.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions build/toolchains/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,13 +404,6 @@ config_setting(
},
)

config_setting(
name = "opt",
values = {
"compilation_mode": "opt",
},
)

bool_flag(
name = "nogo_flag",
build_setting_default = False,
Expand Down
2 changes: 1 addition & 1 deletion dev
View file
Open in desktop
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=79
DEV_VERSION=80

THIS_DIR=$(cd "$(dirname "$0")" && pwd)
BINARY_DIR=$THIS_DIR/bin/dev-versions
Expand Down
24 changes: 18 additions & 6 deletions pkg/cmd/dev/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nit. You can add continue here, so you don't even try to evaluate the next if (and potential future ifs).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used if .. else if for this to accomplish the same thing without using continue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right. nm then!

} 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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/roachprod_stress.go
View file
Open in desktop
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/ui.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
12 changes: 7 additions & 5 deletions pkg/cmd/dev/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down