Update dependencies - #6572
Conversation
|
The generated output of Full Type Diff |
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This is an automated dependency update covering Rust crates, Bazel build deps, workflow version bumps, and a few build configuration changes.
Issues found (ranked by severity):
-
Hardcoded LLVM-19 paths in
.bazelrcbreak builds without exact LLVM-19 install location — The newbuild:linuxlines hardcode/usr/lib/llvm-19/bin/paths. This overrides the existing genericCC=clang(line 259) and will break builds for anyone whose LLVM-19 is installed elsewhere (e.g., via Homebrew paths on WSL, or a different package layout). Additionally,--ld-pathon lines 502-503 potentially conflicts with the existing--linkopt="-fuse-ld=lld"on line 327. -
llvm.shis added but never referenced — A 259-line vendored copy of the LLVM installer script is added to the repo root, but no CI workflow, Dockerfile, or action in this PR is updated to use it. The existingDockerfile.release,.github/actions/setup-runner/action.yml, and.devcontainer/Dockerfileall stillwget/curlthe remote version. This looks like an incomplete change — either the consumers should be updated to use the local copy, or the file shouldn't be added yet.
| build:linux --repo_env=CC=/usr/lib/llvm-19/bin/clang | ||
| build:linux --linkopt=--ld-path=/usr/lib/llvm-19/bin/ld.lld | ||
| build:linux --host_linkopt=--ld-path=/usr/lib/llvm-19/bin/ld.lld | ||
| build:linux --repo_env=AR=/usr/lib/llvm-19/bin/llvm-ar |
There was a problem hiding this comment.
These lines hardcode /usr/lib/llvm-19/bin/ paths, which overrides the generic CC=clang on line 259 for all Linux builds. This will break builds for anyone who doesn't have LLVM-19 installed at this exact system path.
Also, --ld-path=/usr/lib/llvm-19/bin/ld.lld on line 502 may conflict with the existing --linkopt="-fuse-ld=lld" on line 327 — both attempt to control the linker but via different mechanisms.
Is this intentional? If this is only needed for CI, it should probably go in a .bazelrc.ci or be scoped to a specific config (e.g., build:ci-linux) rather than the global build:linux config.
| @@ -0,0 +1,259 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
This 259-line script is added to the repo root but nothing in this PR references it. The existing consumers (.github/actions/setup-runner/action.yml, Dockerfile.release, .devcontainer/Dockerfile) all still download llvm.sh from https://apt.llvm.org/llvm.sh. Is this an incomplete change, or is it used elsewhere outside this repo?
|
I'm Bonk, and I've done a quick review of your PR. This is an automated dependency update covering Rust crates, Bazel build deps, workflow version bumps, and a few build configuration changes. The version bumps themselves all look like routine patch/minor updates and are consistent across I posted a review with 2 issues:
|
This is an automated pull request for updating the dependencies of workerd.