There are many files that needs to be updated when you
bump the version of LLVM. This script tries to automate
that in order to make the release managers job easier.
Details
- Reviewers
kwk hans tstellar ldionne - Commits
- rG12ba7f27b88e: [Release] Add bump-version script.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
60,050 ms | x64 debian > MLIR.Examples/standalone::test.toy |
Event Timeline
There is no room for a VERSION file at the root of the repository and everybody reads the file if necessary?
I suggested this at some point - but it never got traction. Maybe showing the complexity of this script can be a catalyst of changing this (especially for the bazel build system).
I have used this script for 15.x branch just to make sure I don't forget anything, since it's many files that needs to be updated.
So far I've consumed only the llvm/CMakeLists.txt to get the version but that's of course hard-coded into the llvm subproject. I didn't know that there were so many places. I highly suggest to create a discourse post and point to this script. IMHO there's no point in doing this in so many places but rather have everything in one place and let the subprojects consume from it. In order to gain acceptance of such a mundane task, I suggest to create another patch that shows how to consume this version.
For Swift I found a PR:
https://github.com/apple/swift/pull/58931/files
The version is in a CMake file, but there were not that many files updated.
For Rust. I found a version file:
https://github.com/rust-lang/rust/blob/master/src/version
Most projects store the current version in ChangeLog files.
Yeah I like a single file like VERSION.txt or .version or something. But we need buy-in. I'll start a thread on discourse to discuss first.
Subprojects already consume the version from llvm/CMakeLists.txt. That's the only version that matters for most purposes. All the duplication is due to alternative build systems. Would bazel even be able to read this from a file?
There is also the lit configuration.
Actually there are many more places in the documentation - especially the release notes that we should update with this script as well later.
I added processing of libcxx/include/__config plus that I refactored the script to be subclasses instead.
Can you guys review the code? We might circle back and centralize version information later on - but for now I would like to have this script.
llvm/utils/release/bump-version.py | ||
---|---|---|
175 | This should be more future-proof in case e.g. clang-format causes this to change in the future. Then you can also use t = match.group(1) instead of t = line[line.rfind(' ')+1:].strip(), if you prefer that. | |
177–179 | I suggest that you only check-in the version that handles XXYZZ, and I can modify this script in D134187. |
I put some comments, but overall lgtm as in the end it's going to be you and Tom who run it.
llvm/utils/release/bump-version.py | ||
---|---|---|
195 | s/i.e./e.g./ | |
197 | Maybe add an example, like /foo/llvm-project/ ? Could this default to the current working directory? Oh I see later on that it refers to the source root of this file. Maybe the help text could mention that somehow. | |
216 | This is very sparse, especially with all the newlines. Could it be just [ ("llvm/utils/gn/secondary/llvm/version.gni", GNIProcessor())} ... ] |
With respect to the future solution of making the version more centralized (see https://discourse.llvm.org/t/rfc-centralized-location-for-version-information/65295) I think this looks good. Great job!
llvm/utils/release/bump-version.py | ||
---|---|---|
97 |
LGTM, but please note that the XXYZZ => XXYYZZ change in libc++ has been done. My suggested changes should cover that.
llvm/utils/release/bump-version.py | ||
---|---|---|
175–180 |
Adressed some comments
llvm/utils/release/bump-version.py | ||
---|---|---|
216 | I made it a bit more compact. The formatting comes from black which is pretty much pep8 - so I like to keep that even if I don't always agree with it. But it looks better now I think. |
Thanks everyone for the comments! I landed this now and we can always follow-up with fixes if something doesn't look right.