When building Carbon on Fedora, clang reports the
resource directory as `/usr/bin/../lib/clang/22`.
This fails to string match against `/usr/lib/clang/22`
and so bazel reports an error.
This PR canonicalizes the path returned by clang
so that it will string match successfully.
This has been working really well for me, is incredibly faster than the
other approach, and some commits continue to hit bugs in the old system
where files that aren't even going to be run through `clangd-tidy` end
up tripping up the execution. Hopefully all of that is resolved with the
new version.
This also switches to a more Bazel-based install layout, skipping the
FHS-based synthetic layout. The FHS-based layout is still reconstructed
explicitly when building an installable tar-ball.
The biggest change is to configure the just-built install as a Bazel
toolchain, including allowing it to build its own runtime libraries as
native Bazel libraries. This removes the need for a monolithic runtimes
build, all of that code logic is removed.
This should also pave the way to using the just-built toolchain for
doing a full 3-stage bootstrap. Building the 2nd stage is included here
as it was a particularly effective way to test that the Bazel
integration was fully working. Adding a 3rd-stage check for stability is
future work, but should be pretty easy.
There is a down-side: this uses the busybox to do the runtimes
compilation, which means they will be re-built after ~any change to
Carbon. However, the integration with Bazel should largely pay for this,
and we can continue to factor the tests away from depending on built
runtimes in most cases.
Now that we're building and testing the runtimes more directly, this
surfaced a problem with the layout of runtimes on macOS that is fixed
here. All of the Darwin OSes use a custom layout for their resource
directory compared to other targets. We now model this in both the C++
built runtimes and the Bazel built runtimes.
Assisted-by: Gemini via Antigravity
Noticed this in bazel central registry, I'm interested in trying it out.
It's using a faster approach, but leaving the other around for the
moment in case it doesn't work out well.
Assisted-by: Google Antigravity with Gemini
By using git_override, we get some validation from the sha, while
removing the sha256 on the .tar.gz which has been brittle lately. Note
the difference between downloading via sha is this still locally
validates content.
Versus something like #6844, this doesn't update the llvm version, just
how we get it.
Assisted-by: Google Antigravity with Gemini
This is a mostly routine update, with some edits for a benchmark API
change.
I'm not updating LLVM here, since that could conflict with other ongoing
work.
I wasn't sure I'd be able to really test this code path, but then
I remembered that Bazel has a whole platform for running Bazel from
within an integration test, and it turns out to work brilliantly. It
even lets us point the child Bazel invocations to the just-built
toolchain.
This should both give us confidence that we don't accidentally hit
a Bazel incompatibility with the example project, and it should ensure
that if something about the installed toolchain would stop being
compatible with building via Bazel we'll catch it early.
The tests are integration tests and so a bit slow: 15s or so. But
`//examples/...` is already pretty expensive and no other testing
patterns are impacted.
This leaves behind project-specific features such as the system header
management of our dependencies and the fancy cache management string.
No expected changes here, but yet another slightly different order of
flags.
This introduces the first pieces of a cleaner way to configure toolchain
components on target dimensions: dedicated features for those target
dimensions.
With that, we extract a `libcxx_feature` that can always be present but
disables its flags on unsupported targets.
With `-stdlib` in its own feature, move `-std=c++20` to not require
a variable but directly live in the flags.
This should enable us to extract the largest remaining feature into its
own file cleanly by removing dynamic configuration of it, along with
libcxx.
Further refactoring of target-specific logic will follow in its
footsteps.
This brings some fixes:
- The handling of `zlib` and `zstd` are much cleaner
- Three of our patches are no longer needed
This also includes the fixes from #6562
It also moves us from `zlib` to `zlib-ng` which is a much better basis
for what we want, and likely makes our toolchain faster when generating
debug info at least.
It fixes another API change in terms of which headers provide the
`createInvocation` we use.
Lastly, it cleans up the deps test to correctly recognize the wrappers
for `zlib-ng` and `zstd`, as well as improving the documentation for why
we allow dependencies on them.
This moves the simplest parts of the toolchain config into separate
files. These parts are either unparameterized or trivially parameterized
and so easily extracted from the main file.
I tried to minimize the interesting edits here, but wasn't _completely_
successful I'm afraid. I'll try to describe them.
First, all of the interesting content of the new files is copied and
re-indented, no interesting edits were done.
The main file sees some more significant edits in order to realize this
refactoring:
- Extract the feature array building to a helper method.
- Collapse some extraneous features as there was no where to extract
them.
- Restructure how the array itself is built to support building it using
array fragments from the various files.
The only interesting semantic change I'm aware of here is that this
somewhat changes the order of command line flags in compiles and links.
The previous order was "fine", but not especially logical. I've tried to
more logically have features that should "override" or are "more
specific" come later here. However, that results in a slightly different
ordering. None of the current features had any flags that overlap, so
this should have no behavior change other than the changed flag order.
This is only the first step, however. There remain complex features in
the main configuration that I want to move out. However, to make those
moves simple requires some significant changes to how these remaining
features work and so I wanted to break them out. I've tried to leave
TODOs that can help as breadcrumbs on the parts of this refactoring that
aren't yet complete.
The comments also are mostly what we already had. I'm happy to try and
add some, but not sure how much I can cover as there is a _lot_ of code
here that I'm just moving around. Please let me know if there are
particularly places that would benefit from comments.
---------
Co-authored-by: Geoff Romer <gromer@google.com>
Also consolidate on using `//bazel/cc_rules:defs.bzl` where appropriate.
Also update a couple of Bazel modules deps of `@rules_cc` to the latest
versions.
- Update bazel to 8.3.1, just to stay reasonably up to date.
- Bazel warned about the platforms version, so I generally updated
packages that have central registry versions.
- Note there's a newer re2 in the central registry, but I got a download
error with it.
- `--experimental_guard_against_concurrent_changes` is deprecated; I
wasn't sure it's worth explicitly setting
`--guard_against_concurrent_changes=full`, but figured it may be
consistent (it's not clear to me -- see
https://github.com/bazelbuild/bazel/pull/25874).
- 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>
Nicholas Bishop
Lucile Rose Nihlen
Chandler Carruth
Jon Ross-Perkins
zadig
josh11bandJosh L
Dana Jansens
DavidLoftus
