Skip to content

Clangs 19+ for AArch64 can't be used because of "LLVM"/"clang+llvm" confusion #464

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.

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

Open
jwbee opened this issue Feb 18, 2025 · 2 comments
Open

Comments

@jwbee
Copy link

jwbee commented Feb 18, 2025

PR #461 looks as if it partially addresses this but the function llvm_release_name is still forcing clang 19 archives to be called "LLVM-...tar.xz" even though the archives for linux-aarch64 are named "clang+llvm-...tar.xz"

@helly25
Copy link
Contributor

helly25 commented Mar 15, 2025

I improved on 471 in #472.

The new PR allows to not only automatically find the latest version it allows to restrict versions.

For instance the following would find any version starting with 19.1.1 but would not allow 19.1.7 (did not work for us) and would not accept any version starting with 20. Effectively allowing any newer version in the 19 branches but skipping 19.1.0 and 19.1.7.

llvm = use_extension("@toolchains_llvm//toolchain/extensions:llvm.bzl", "llvm", dev_dependency = True)
llvm.toolchain(
    name = "llvm_toolchain_llvm",
    llvm_version = "latest:>=19.1.1,!=19.1.7,<20",
)
use_repo(llvm, "llvm_toolchain_llvm")

fmeum added a commit that referenced this issue May 11, 2025
See issue: #473

Instead of implementing a complex way of predicting the basename find
them by common prefixes in the database we anyway have.

Rather then forcing maintainers to enter more complex information this
keeps us at simply adding the versions as generated from the hasher
script.

This should also ultimately put issue
#464 and any
future occurrence to rest.

The results of the new search based algorithm are generally be better.
The algorithm removes a lot of the mistakes from the old implementation,
is more correct in some cases and performs other new correct decisions.
Yet, the new algorithm may not always be perfect, as it may have
inherited some bad decisions of the old algorithm.

As the diff in `toolchain/internal/llvm_distributions.golden.out.txt`
shows we only drop `del:` lines, which means we can find more
distributions.

The diff in `` is harder to interpret. The diffs are best viewed side by
side as both old and new file are supposed to have the same contents. At
least their "keys" are in the same order. I use a custom diff program
that is designed for this. It strips the actual error messages. When
only looking at new errors (as shown below), then we see no difference:

```sh
bazel run //mbo/diff -- --regex_replace_lhs='/(.*)(ERROR).*/\1\2/' --regex_replace_rhs='/(.*)(ERROR).*/\1\2/' ~/llvm_distributions.golden.sel.orig.txt ~/llvm_distributions.golden.sel.find.txt --algorithm=direct | awk '{l=c;c=$0}/^+.*ERROR/{print l"\n"c}'
```

If we look at all differences, then we see some 782 former errors now
resulting in a distribution. They now find the likely best choice.

Some of these can be improved. But imo it is not worth doing so.

The PR does not change anything regarding manually provided information.
But some folks will be able to switch to automatic selection instead.

---------

Co-authored-by: Fabian Meumertzheim <[email protected]>
@helly25
Copy link
Contributor

helly25 commented May 11, 2025

Automatic distribution resolution has been reimplemented in #471 waiting for release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants