- Updates incompatible flags.
- `rules_flex` is no longer used, so enable its flag.
- Fixes `sh_test` deps for
`--incompatible_disable_autoloads_in_main_repo`
- Broadens the exception for `rules_cc` and `bazel_tools` due to changes
to runfiles deps; trying to avoid minutiae that shouldn't affect the
decision.
rules_cc@0.1.0 was yanked from
[BCR](https://registry.bazel.build/modules/rules_cc) due to prematurely
removing cc_proto_library, this inconsistently causes the following
build error:
> ERROR: Error computing the main repository mapping: Yanked version
detected in your resolved dependency graph: rules_cc@0.1.0, for the
reason: rules_cc 0.1.0 is yanked due to incompatible change (prematurely
removing cc_proto_library from defs.bzl), please upgrade to 0.1.1.
Yanked versions may contain serious vulnerabilities and should not be
used. To fix this, use a bazel_dep on a newer version of this module. To
continue using this version, allow it using the --allow_yanked_versions
flag or the BZLMOD_ALLOW_YANKED_VERSIONS env variable.
This PR updates to 0.1.1 as recomended in warning and
[bazelbuild/rules_cc#268](https://github.com/bazelbuild/rules_cc/issues/268#issuecomment-2651269117).
- The actual reason I started this: minor lowering updates in the golden
LLVM IR
- Process.inc changed enough to need a patch context update.
- https://github.com/llvm/llvm-project/pull/123126 added `proto_library`
uses without a `load`, which is broken in bazel 8
- Just commenting these out because we don't use them. I'll follow up
separately about a possible fix, but continuing to use `WORKSPACE` is a
bigger issue LLVM probably should address.
- Note this update is also triggering removal of `migrate_cpp`, in #4887
The WORKSPACE file is deprecated; support is already off by default, and
it'll be removed in the next major bazel release. Our main dependency is
tree-sitter, and I'm trying to address that here.
We're currently using https://github.com/elliottt/rules_tree_sitter, but
that hasn't been updated in a couple years, meaning it lacks
MODULE.bazel support. In the registry, there's
https://registry.bazel.build/modules/tree-sitter-bazel, but this is only
the *parser* libraries of tree-sitter, not the *generator*. I'm using it
for that much, at least.
For the *generator*, which transforms grammar.js to parser.c/h, I'm just
requiring a non-hermetic invocation (i.e., people who want to work on it
will need to install tree-sitter; see the README.md updates). I tried
running it manually, but parser.c is about 600 KB; pre-commit rejects
files that large and I don't think an exception makes sense to override
for this (it'd probably also grow substantially if the grammar were
updated to cover more syntax). In order to make the non-hermetic call
not break "bazel build //..." for most developers, I'm marking most
targets in the package as manual.
Note, I did look long and hard at using `aspect_rules_js`/`rules_nodejs`
to invoke npm. This took a lot of time, and I have a commit that's
mostly working, except I hit a point where it uses `declare_symlink`
which we disallow for compatibility reasons (commit "Lots of work for
figuring out rule_js uses declare_symlink" on the PR). As a consequence,
I think we can't use the primary supported ways to have hermetic npm
calls.
Also, `treesitter` -> `tree_sitter` because it's generally called
`tree-sitter`, two words. We even had a `treesitter/src/tree_sitter`
directory so it's a bit inconsistent.
As far as bugs here, the parser library breaks bazel queries, e.g. the
error:
```
ERROR: Evaluation of query "somepath(//..., @llvm-project//third-party/unittest:gtest)" failed: preloading transitive closure failed: no such package '@@[unknown repo 'platforms' requested from @@tree-sitter-bazel+]//': The repository '@@[unknown repo 'platforms' requested from @@tree-sitter-bazel+]' could not be resolved: No repository visible as '@platforms' from repository '@@tree-sitter-bazel+'
```
I'm just excluding tree_sitter from queries where I can to work around
the error.
This updates to bazel 8.0.0, also updating bazel mod deps and tools to
make that function. The release is a couple weeks old, and we haven't
updated in a while, and it's a major release. Note it includes some
incompatible flag flips that this is trying to update with respect to.
I'll try generally enabling incompatible support separately.
The most visible bazel behavior change here will be the change from `~`
to `+` in repo path names. (If you're curious,
https://github.com/bazelbuild/bazel/issues/23127 indicates this fixes a
Windows performance issue)
Note that this is building on top of both the action env update in #4728
(which got me started down this path) and the proto removal in #4731
(which would add significant work to this update). Only the commit
starting at "Work towards bazel 8.0.0" is specifically part of this PR.
As part of migrating to the latest bazel configurations in #4729, I'm
running into proto toolchain issues. For example:
"Error: <target @@protobuf+//:cc_toolchain> (rule
'proto_lang_toolchain') doesn't contain declared provider
'ProtoLangToolchainInfo'"
Although we may eventually want more use of proto, right now the only
use is for the explorer fuzzer. The explorer codebase is essentially
frozen, so continuing to run it isn't gaining us much (in fact, we've
already disabled autofuzzing for it).
So, rather than trying to fix the proto setup, this change:
1. Deletes `explorer/fuzzing`
2. Removes proto portions of `testing/fuzzing`, which were only in-use
by the explorer
3. Removes some ancillary proto support, which would otherwise break
from the bazel changes and would be difficult to validate as "still
working"
This change is partly isolated in order to make it easier to revive bits
of (3).
Came up due to [libc++ install
issues](https://discord.com/channels/655572317891461132/655577725347561492/1302023663155023905)
We've discussed clang version verification, and adding that as long as
I'm in here. The more significant bit is the libc++ check, which if it's
not installed should fail like:
```
(tons of output)
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
/usr/lib/llvm-16/lib/clang/16/include
/usr/local/include
/usr/include/x86_64-linux-gnu
/usr/include
End of search list.
/usr/local/google/home/jperkins/.cache/bazel/_bazel_jperkins/85deb7d9d96f7e0e80b42618a55969d7/external/_main~clang_toolchain_extension~bazel_cc_toolchain/_temp:6:2: error: "No libc++ install found!"
#error "No libc++ install found!"
^
1 error generated.
ERROR: Analysis of target '//toolchain:toolchain' failed; build aborted: Analysis failed
INFO: Elapsed time: 0.265s, Critical Path: 0.08s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully
```
pre-commit runs bazel, and GitHub runners have an old clang by default
(caught by the new check), so I'm installing here for a consistent
version.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Undoes most of #4347, because of [performance
complaints](https://discord.com/channels/655572317891461132/707150492370862090/1295527235133898772).
With a 30-ish frame stack trace and `-c dbg`, my installed
`llvm-symbolizer` still seems slow (~6s), but the hermetic
`llvm-symbolizer` adds ~4s (i.e., ~10s total). I don't think we can
easily force the hermetic version to build in opt configuration, so I'm
backing it out.
This only sets the symbolizer for our more used targets; not sure if
there's a great way to set it everywhere (I suppose I could try wrapping
cc_binary etc rules if there's a strong preference).
There is a downside here, symbolizing a fastbuild crash seems to take
about 3s. Not sure if there's a good way to get a faster llvm-symbolizer
execution...?
I tried running with the new LLVM update without the
LLVM_SYMBOLIZER_PATH, and it looks like that's insufficient. With the
settings, I now get readable crashes:
```
#9 0x000055dc007d1724 void Carbon::Internal::CheckFail<Carbon::TemplateString<5>{"FATAL"}, Carbon::TemplateString<27>{"toolchain/driver/driver.cpp"}, 84, Carbon::TemplateString<0>{}, Carbon::TemplateString<3>{"err"}>() (/usr/local/google/home/jperkins/.cache/bazel/_bazel_jperkins/85deb7d9d96f7e0e80b42618a55969d7/sandbox/linux-sandbox/9383/execroot/_main/bazel-out/k8-fastbuild/bin/toolchain/testing/file_test.runfiles/_main/toolchain/testing/file_test+0x2894724)
```
Note the LLVM update is for
https://github.com/llvm/llvm-project/pull/109021
This improve the toolchain's performance by about 10%.
It will also allow us to leverage TCMalloc's extensions to do heap
profiling and get other information about how efficiently we're using
the heap.
Note that currently this causes all of our builds to produce a warning
due to an issue with `rules_python` and multiple modules registering
python toolchains:
https://github.com/bazelbuild/rules_python/issues/1818
This is also only enabled on Linux as there is no support for other OSes
at the moment.
Updating to 1.8.4 breaks the patch file, so I'm looking at solutions
that don't require maintaining a patch.
Verifying this is working with `bazel build
//toolchain/lex:tokenized_buffer_benchmark && strings
bazel-bin/toolchain/lex/tokenized_buffer_benchmark |& grep pfm`
With bazel 7.2.0, there are some dependency changes, resulting in:
```
WARNING: For repository 'bazel_skylib', the root module requires module version bazel_skylib@1.5.0, but got bazel_skylib@1.6.1 in the resolved dependency graph.
WARNING: For repository 'platforms', the root module requires module version platforms@0.0.8, but got platforms@0.0.9 in the resolved dependency graph.
```
This goes around doing some updates... protobuf's include structure has
changed a little too.
This uses a header-only extraction of the Boost unordered hashtable
project to allow a trivial Bazel build and for us to benchmark against
it effectively.
This takes the installation layout and replicates it using `rules_pkg`
to build either a tarball or a zip file of the toolchain. Correctly
manages file permissions and symlinks, etc.
There are some big remaining things here:
- Figure out how we want to test this. We can add shell tests maybe?
A bit awkward. Nicer would be to make the `//examples` tree build
using this rather than the more native-bazel install data, however
building these is quite slow and it seems bad to pay that cost
constantly so dedicated testing is probably better. For now, I've
tested these manually.
- Need to add versions to the toolchain and then thread them through
here so they install properly as a versioned release.
But my primary goal for now is just to be able to validate that the
install tree is working outside of Bazel and this does enough for that.
The above will be longer-term things.
I'd kept these in to separate the bazel module update from the BUILD
file changes, then forgot about it. I think all of these can be cleanly
removed now. I think it's something we should clean up for consistency
with the bazel central repository names; I think it's best to reduce
that divergence.
llvm_zlib and llvm_zstd remain because of how llvm depends on the
particular names.
The patches to bazel_clang_tidy were adopted upstream and are no longer
necessary, so I think we can simplify to git_override.
This fixes an existing issue where bazel was complaining that some rules
indirectly requested rules_python 0.29.0.
Note I'm not updating protobuf or com_google_libprotobuf_mutator. This
is because I get build errors with a protobuf -> abseil dependency if
updating.
Co-authored-by: josh11b <github-llvm@technomagi.com>
Update the compiler-rt patch (maybe this could be upstreamed, or try out
https://github.com/google/fuzztest?) and handle the ThreadPool ->
DefaultThreadPool rename.
C++20 mode exposes bugs in `clang-tidy` 16 that are challenging to work
around, but this should manage to do so. The patch to `bazel_clang_tidy`
has been sent upstream but no need to wait for it.
We don't actually specify a specific version of C++ in our style guide
docs, and the Google style guide has already been updated to be based on
C++20 so nothing to be done there.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This should also unblock our switch to C++20 and other improvements.
There are three core parts of the change --
1) Updating our infrastructure to fetch and find Clang-16.
2) Updating our documentation to reflect this and help folks with any
system issues they encounter.
The infrastructure change is unfortunately tricky. We can't get Clang 16
easily on GitHub's runner images, and in the past we've had persistent
problems with flakiness when our actions download this much during their
runs. Due to the flakiness, we've previously removed all downloading of
dependencies outside of Bazel itself, and added retry loops around Bazel
specifically to overcome flaky downloads.
This change tries to address these problems by populating the Clang and
LLVM toolchain in a place that we can then cache using the built-in
GitHub action caching infrastructure. This seems like by far the least
likely to flake way of downloading extra things into our runs. And since
these are relatively slow moving dependencies, we should populate this
cache very, very rarely.
For Linux, this downloads the binary release artifact from GitHub,
prunes out large parts of it that we don't need, and then caches this as
a local toolchain. This proves both small and fast.
For macOS, this uses a trick to cache the destination of Homebrew
installs. It unfortunately caches the *entire* Homebrew installation
though, and so it also goes to some lengths to prune and minimize how
much is installed from Homebrew. The result is "only" a 2gb cache image.
Because of the size and slower download and filesystem, the macOS runs
see a 1 - 2 minute slowdown.
We might extend the Linux infrastructure here usefully if we want to
test multiple LLVM versions. We might also extend the macOS version to
get a cheaper way to prune parts of the system and free up disk space,
or to cache other Homebrew installed tools if needed.
Last but not least, this brought to the forefront an issue with our C++
toolchain integration which relied on a specific CMake build option
being set in the LLVM toolchain install. This option isn't used in the
official release artifacts. Instead, switch to a more robust approach to
linking libc++abi statically that shouldn't have these problems.
Beyond the infrastructure changes, this also updates the documentation
to reflect requiring Clang 16 or newer, and adds some extra tips for
folks that are missing this.
The documentation is also updated to address a problem with getting the
right libc++abi files installed to support the more robust linking
strategy. This may reduce the problems we've seen in the past around
libc++abi and linking on other Linux distros as well.
---------
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
The pip dependencies in github_tools are the reason the
MODULE.bazel.lock is platform-dependent. Following complaints about the
platform-dependence, split apart github_tools from the rest of the bazel
repo and make it not track the lockfile: while there's an incremental
safety risk due to not tracking checksums, it's unlikely the tools there
would ever be part of a Carbon release process. If we eventually add
Python tools that need pip to the release, it might be desirable to go
back to re-unify the bazezl repos.
This does make running pr_comments incrementally more inconvenient
because a "bazel run" needs to be run from the github_tools subdir.
As a consequence of separating the dependency, this means tests will not
be continuously run in github_tools. They're now a separate repo, and we
cannot add a dependency without restoring the platform-dependent issue.
I think pr_comments is sufficiently low value and unchanging that it is
not worth building separate CI for it.
Cleans up some legacy references to third_party/llvm-project, since now
github_tools needs to be added to the main bazelignore.
The `libpfm` in the Bazel central repository uses `make` to build it,
which is difficult to integrate with our toolchain. Rather than try to
fix all the issues there, it's easy to just add a native Bazel build for
the library. I don't know that any of the relevant upstream folks are
interested in this kind of build, but it seems easy for us to maintain
as a Carbon project build configuration. I've also not tried to port all
of the different configurations as a consequence, and only 64-bit x86
and Arm as that seems the only likely architectures we'll care about in
the near term.
I've kept this using the `bzlmod` stuff as best I can, and I *think* I'm
holding all of those pieces correctly, but if not, happy for suggestions
on adjustments.
The `google_benchmark` package also has an awkward way of enabling
`libpfm` support using a top-level `bazel` command line flag. I think
this is because of how brittle the Bazel build of `libpfm` is, but I'm
not sure. With the new build, it seems easy to patch `google_benchmark`
to detect the same conditions as we build `libpfm` under, and enable it
there. So I've done this to avoid folks having to pass a command line
flag on platforms where it is supported.
The result is that we now get really nice CPU counter support in our
benchmarks out-of-the-box on Linux x86-64 and AArch64. For example on my
Fedora Asahi install on a Mac Mini I get:
```console
$ bazel run -c opt --copt=-gmlt //common:hashing_benchmark --run_under="taskset -c 4" -- --benchmark_counters_tabular=true --benchmark_perf_counters=CYCLES,INSTRUCTIONS
INFO: Invocation ID: 4aaeb9e9-7df5-4f1f-b56b-c03411790268
INFO: Analyzed target //common:hashing_benchmark (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //common:hashing_benchmark up-to-date:
bazel-bin/common/hashing_benchmark
INFO: Elapsed time: 0.360s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: /bin/bash -c 'taskset -c 4 bazel-bin/common/hashing_benchmark '\''--benchmark_counters_tabular=true'\'' '\''--benchmark_perf_counters=CYCLES,INSTRUCTIONS'\'''
2024-01-15T00:10:50-08:00
Running /home/chandlerc/.cache/bazel/_bazel_chandlerc/b686aa8910e0845b88c21d715819b076/execroot/_main/bazel-out/aarch64-opt/bin/common/hashing_benchmark
Run on (8 X 2064 MHz CPU s)
CPU Caches:
L1 Data 64 KiB (x8)
L1 Instruction 128 KiB (x8)
L2 Unified 4096 KiB (x2)
Load Average: 0.01, 0.08, 0.08
--------------------------------------------------------------------------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations CYCLES INSTRUCTIONS bytes_per_second
--------------------------------------------------------------------------------------------------------------------------------------------------------------
BM_LatencyHash<RandValues<uint8_t>, CarbonHashBench> 4.11 ns 4.11 ns 170200064 13.1321 9.00587 232.116Mi/s
BM_LatencyHash<RandValues<uint8_t>, AbseilHashBench> 4.82 ns 4.82 ns 145643520 15.3657 12.0059 197.946Mi/s
BM_LatencyHash<RandValues<uint8_t>, LLVMHashBench> 7.96 ns 7.95 ns 87956480 25.3737 17.0068 119.991Mi/s
BM_LatencyHash<RandValues<uint16_t>, CarbonHashBench> 4.11 ns 4.11 ns 170365952 13.1247 9.00587 464.573Mi/s
BM_LatencyHash<RandValues<uint16_t>, AbseilHashBench> 5.51 ns 5.51 ns 127568896 17.5578 14.0059 346.225Mi/s
BM_LatencyHash<RandValues<uint16_t>, LLVMHashBench> 8.00 ns 7.99 ns 87085056 25.377 17.0068 238.834Mi/s
BM_LatencyHash<RandValues<std::pair<uint8_t, uint8_t>>, CarbonHashBench> 4.91 ns 4.90 ns 136013824 15.6456 14.0059 389.006Mi/s
BM_LatencyHash<RandValues<std::pair<uint8_t, uint8_t>>, AbseilHashBench> 6.85 ns 6.85 ns 102630400 21.8041 18.0059 278.637Mi/s
BM_LatencyHash<RandValues<std::pair<uint8_t, uint8_t>>, LLVMHashBench> 7.57 ns 7.56 ns 92798976 24.1437 20.0068 252.151Mi/s
BM_LatencyHash<RandValues<uint32_t>, CarbonHashBench> 4.12 ns 4.12 ns 170229760 13.1272 9.00587 926.444Mi/s
BM_LatencyHash<RandValues<uint32_t>, AbseilHashBench> 4.93 ns 4.92 ns 145304576 15.3738 12.0059 775.224Mi/s
BM_LatencyHash<RandValues<uint32_t>, LLVMHashBench> 8.11 ns 8.10 ns 87127040 25.373 17.0068 470.98Mi/s
```
This patches bazel_clang_tidy handling of headers. I found an equivalent
change at https://github.com/erenon/bazel_clang_tidy/pull/13, but that
was [already
rejected](https://github.com/erenon/bazel_clang_tidy/pull/13#issuecomment-1047007424).
Per the criticism, this will result in redundant processing of headers.
The project instead uses `HeaderFilterRegex: ".*"`, but that results in
two problems:
1. When running with `-k`, errors are repeated when a header is included
more than once, which is common.
2. clang-tidy including errors from headers that are included from other
modules (e.g., abseil-cpp); filtering correctly is difficult.
Given the trade-offs and options (including forking), I thought patching
was preferable so long as it remains narrow.
This sets things up to use `bazel` to run `clang-tidy` using
https://github.com/erenon/bazel_clang_tidy.
I'm fixing issues outside of explorer, and disabling clang-tidy for
targets in explorer that have legacy issues. I was going to disable
clang-tidy for targets in explorer such as interpreter anyways, because
they're slow to parse, and just extended that to the currently failing
targets.
Directories in `external/` now include versions of dependencies, so
continuing to make that work would require version-locking the
compile_flags.txt. We've previously seen issues with directories being
forgotten; this switches to letting bazel generate everything.
Setup follows instructions at
https://github.com/hedronvision/bazel-compile-commands-extractor?tab=readme-ov-file#first-add-this-tool-to-your-bazel-setup
Maintains `./scripts/create_compdb.py`. Per discussion, new files seem
to work okay -- I'm getting #include completion that clearly detects
files.
Note this creates an `external` directory:
```
╚╡ls -ld external
lrwxrwxrwx 1 jperkins primarygroup 27 Dec 19 14:02 external -> bazel-out/../../../external
```
I felt sort of weird checking that in though, at least alongside
`bazel-execroot` it feels like it should have a `bazel-` prefix.
However, I can't rename it. So omitting it, and leaving it to the script
to generate, felt like a reasonable compromise.
Running takes a few minutes (about 3 for me), and should be faster on
reruns. It does print a couple warnings, just tree_sitter missing
parser.c and _GNUC_PREREQ errors from m4.
The use of zlib-ng seemed to work previously, but may have only done so
because of some combination of an implementation detail of
`strip_include_prefix` in older versions of Bazel and some happenstance
of a system installed `zlib.h` being possible to find. With Bazel 7 it
started breaking on my machine which doesn't have a system `zlib.h`.
Using the "normal" zlib and the bzlmod rigging for it is simple and
seems to work.
Also adds the Linux AArch64 component of the Bazel lock file which was
necessary to test this on my Arm Linux machine.
Building on #3505, the toolchain and llvm rules require a little more
special-casing to get them to work well. This also moves
libprotobuffer_mutator, but that one's more minor. The migration
encounters more quirks in repo naming as seen by various queries.
This changes some of the toolchain work that was recently done for bazel
7 in #3496, dropping a bzl file I'd suggested to add, instead using
`:all` for toolchain registration. (somewhat as an improvement, somewhat
just to avoid a `load`)
Remaining in the WORKSPACE are example code repos and tree sitter rules.
Neither of these are part of the main toolchain builds, and so will
probably be lower impact if there's a good solution for them.
Some background information is at https://bazel.build/external/migration
Trying to handle the simple cases first. This adds a requirement for
bazel 7 due to differences in bzlmod handling between 6 and 7 (also
discussed on
[#infra](https://discord.com/channels/655572317891461132/707150492370862090/1184942191412510720)).
Bazel seems to be okay with a partial migration such as ths.
The python import behavior has subtly shifted, so `carbon.` is no longer
part of import paths. There's a version-incompatible change for `@@`.
bzlmod makes repos sometimes show as `name~version`.
`target-determinator` seems to be okay with `@@` after a version update.
Things not moved here are things that basically need more dep work:
- clang_register_toolchains because I need to dive into its format.
- llvm-project because we need something slightly atypical, I need to
make sure patching and the repo work carries over.
- com_google_libprotobuf_mutator is sufficiently atypical that it
doesn't have a module already, but should be one of the easier things to
fix.
- brotli/woff2: I think we should actually consider removing these. But
again, they're not trivial moves.
- treesitter due to toolchain registration, which has shifted a bit.
- rules_nodejs because treesitter depends on it in an awkward way to
migrate.
Jon Ross-Perkins
josh11bandJosh L
DavidLoftus
