Skip to content

Commit

Permalink
Upgrade bazel to 6.0.0 (#109)
Browse files Browse the repository at this point in the history
This will give us the `add_prefix` parameter for our `http_archive`
repository rules.  We need this to stay clean once we start bringing in
other libraries to use them in our unit tests.

For example, we're about to provide a compatibility layer for the
nholthaus/units repo.  Without `add_prefix`, our include would look like
either `"include/units.h"` or `"units.h"`, which is generic and error
prone.  With `add_prefix = "nholthaus_units"`, we can make this file
`"nholthaus_units/units.h"`.

I found one breakage from this change.  `bazel` now zealously complains
when we run `clang-format`:

```
WARNING: /home/chogg/.cache/bazel/_bazel_chogg/f4320e9077711029fb47fb5166db48f7/external/llvm_14_toolchain_llvm/BUILD.bazel:18:14: @llvm_14_toolchain_llvm//:bin/clang-format is a source file, nothing will be built for it. If you want to build a target that consumes this file, try --compile_one_dependency
```

The suggested remedy, `--compile_one_dependency`, is a red herring.
Nothing depends on this target, so this won't work.  The solution is
twofold.

1. Distinguish between tools we need to `build`, and those we need to
   simply `fetch`.  This forces an extra argument on us for
   `make_command_from_bazel_run` so each tool can tell us which kind of
   tool it is.

2. Run the executable directly, instead of using `bazel run`.  We need
   to do a `bazel query` to translate it from "label" form to "filename"
   form.  We also need some post-processing, since the result of
   `location` output is of the form `<filename>:1:1 <more stuff>`.  This
   is what the `%%:*` is for: it removes everything after the first `:`.

You may be asking yourself whether we could simply do away with all of
this `lib/command_from_bazel` overhead for `clang-format`, since we now
realize it's just a simple file.  The answer, I think, is no: we need to
ensure fetching happens, and this could still take a while if the llvm
14 toolchain hasn't been fetched.

Test plan
---------

- [x] `bazel test //...:all`
- [x] `clang-format --version`
- [x] `buildifier --version`

The last of these still lists the versions as "redacted", but that's a
pre-existing issue.
  • Loading branch information
chiphogg authored Feb 20, 2023
1 parent 13252ad commit bacf892
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
5.3.0
6.0.0
2 changes: 1 addition & 1 deletion tools/bin/buildifier
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@
SCRIPT_ROOT=$(dirname $(readlink -f $0))
source "$SCRIPT_ROOT/../lib/command_from_bazel.sh"

make_command_from_bazel_run @com_github_bazelbuild_buildtools//buildifier:buildifier "$@"
make_command_from_bazel_run build @com_github_bazelbuild_buildtools//buildifier:buildifier "$@"
2 changes: 1 addition & 1 deletion tools/bin/clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@
SCRIPT_ROOT=$(dirname $(readlink -f $0))
source "$SCRIPT_ROOT/../lib/command_from_bazel.sh"

make_command_from_bazel_run @llvm_14_toolchain_llvm//:bin/clang-format "$@"
make_command_from_bazel_run fetch @llvm_14_toolchain_llvm//:bin/clang-format "$@"
15 changes: 12 additions & 3 deletions tools/lib/command_from_bazel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,24 @@ function wrap_bazel () {
}

function make_command_from_bazel_run () {
BUILD_CMD="$1"; shift
TARGET="$1"; shift
USER_MESSAGE="Building tool. If slow, you can Ctrl-C and run: bazel --nohome_rc build $TARGET"
USER_MESSAGE="Building tool. If slow, you can Ctrl-C and run: bazel --nohome_rc $BUILD_CMD $TARGET"

# Write message, then run _building_ command.
# When done: back up; then, write spaces; then, back up again.
echo -n "$USER_MESSAGE" >&2
wrap_bazel build "$TARGET"
wrap_bazel "$BUILD_CMD" "$TARGET"
echo -n "$USER_MESSAGE" | sed 's/./\x08 \x08/g' >&2

# Run _real_ command.
wrap_bazel run "$TARGET" "$@"
if [ "$BUILD_CMD" == "fetch" ]; then
# A target that is simply fetched is a pre-built target. Bazel 6.0.0 will complain if we use
# `bazel run`, because `bazel run` uses `bazel build` under the hood, and there is nothing to
# build. So in this case, we simply execute the target directly.
QUERY_RESULT="$(bazel --nohome_rc query --noshow_progress $TARGET --output=location)"
"${QUERY_RESULT%%:*}" "$@"
else
wrap_bazel run "$TARGET" "$@"
fi
}

0 comments on commit bacf892

Please sign in to comment.