Skip to content

Commit

Permalink
bazel,dev: in dev, handle different --compilation_modes correctly...
Browse files Browse the repository at this point in the history
... 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
  • Loading branch information
rickystewart committed Jul 17, 2023
1 parent 6f0fe89 commit cbf5176
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 38 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 -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
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,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
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
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
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
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
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
26 changes: 20 additions & 6 deletions pkg/cmd/dev/build.go
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,21 @@ 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
}
if arg == "--config" || arg == "--compilation_mode" || arg == "-c" {
ret = append(ret, arg)
addNext = true
}
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 @@ -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
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
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
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

0 comments on commit cbf5176

Please sign in to comment.