-
Notifications
You must be signed in to change notification settings - Fork 13.5k
feat(build): centralize LLVM_VERSION #142786
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
base: master
Are you sure you want to change the base?
feat(build): centralize LLVM_VERSION #142786
Conversation
r? @marcoieni rustbot has assigned @marcoieni. Use |
This comment has been minimized.
This comment has been minimized.
8a4c3e9
to
bd8d591
Compare
|
||
# To keep docker / non-docker builds in sync | ||
|
||
# renovate: datasource=github-releases depName=llvm/llvm-project versioning=semver-coerced extractVersion=^llvmorg-(?<version>\d+\.\d+\.\d+(?:.*)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be automatically updated I think. LLVM updates are more risky than regular dependency updates and frequently need changes on the rust side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renovate doesn't merge automatically, maintainers would still have to do that manually like for dependabot.
For what it's worth, you saw that I didn't enable the regex customManager for these lines in renovate.json5...
# To keep docker / non-docker builds in sync | ||
|
||
# renovate: datasource=github-releases depName=llvm/llvm-project versioning=semver-coerced extractVersion=^llvmorg-(?<version>\d+\.\d+\.\d+(?:.*)) | ||
export LLVM_VERSION=20.1.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put this in shared.sh instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be possible of course.
I didn't know if all those functions would be unwanted in the non-docker builds, that's why I separated the shared versions from shared functions...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These versions should not be shared. There is little value in keeping them exactly synchronized, and the process for updating them is quite different. One of them is just built in CI, while the other requires uploading new artifacts via ci-mirrors.
There was no comment explaining why non-docker would need the outdated 20.1.0-rc2 instead of 20.1.0-rc3 or a stable release like in docker builds (20.1.3).
If both comments have been wrong, please clarify them and upgrade both scripts separately. You would still be able to enable renovate to update both separately and benefit from automatic CI builds for docker and non-docker to see if a new release is compatible respectively brings no regression for one of them. |
The comment isn't wrong per-se, it just maybe could be clarified a bit. Normally we would say something like "WARNING: KEEP IN SYNC WITH X" if it was really required. Here it's more like "when you bump the Linux version, try to think of also updating the Windows/MacOS version sometime in the near-ish future, just to make sure that these versions don't diverge by 5 years.. again :D". Happy to review clarification of the comments. We don't need renovatebot here though. |
Features
LLVM_VERSION
in docker / non-docker builds.github/renovate.json5
Working example
https://docs.renovatebot.com/modules/manager/regex/#regular-expression-capture-groups