This is an archive of the discontinued LLVM Phabricator instance.

[Release] Add bump-version script.
ClosedPublic

Authored by thieta on Sep 15 2022, 1:32 AM.

Details

Summary

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.

Diff Detail

Unit TestsFailed

Event Timeline

thieta created this revision.Sep 15 2022, 1:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 15 2022, 1:32 AM
thieta requested review of this revision.Sep 15 2022, 1:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 15 2022, 1:32 AM

There is no room for a VERSION file at the root of the repository and everybody reads the file if necessary?

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.

kwk added a comment.Sep 15 2022, 2:04 AM

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.

kwk added a comment.Sep 15 2022, 3:16 AM

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.

Just my 2 cnts I prefer the plain rust way of storing the version.

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.

nikic added a subscriber: nikic.Sep 15 2022, 3:29 AM

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.

Erlang has a OTP_VERSION file at the root of the repository:

https://github.com/erlang/otp/blob/master/OTP_VERSION

hans added a comment.Sep 15 2022, 10:17 AM

I think libcxx/include/__config also needs updating.

thieta updated this revision to Diff 461209.Sep 19 2022, 7:57 AM

Refactor script to share more code

Process libcxx/include/__config as well

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.

thieta updated this revision to Diff 461270.Sep 19 2022, 10:43 AM
thieta added a subscriber: ldionne.

Handle libcxx __config header better after discussion with @ldionne

ldionne added inline comments.Sep 19 2022, 12:26 PM
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.

ldionne added inline comments.Sep 19 2022, 12:27 PM
llvm/utils/release/bump-version.py
177–179

Ugh, sorry, I mean in the follow-up to D134187 that will change the scheme from XXYZZ to XXYYZZ (and update the docs, etc.).

ldionne added inline comments.Sep 19 2022, 12:40 PM
llvm/utils/release/bump-version.py
177–179

The follow-up is D134209. Sorry for the numerous comments, but basically what I suggest is that you only handle XXYZZ in this script, and then I can modify this script again in D134209 to handle XXYYZZ (and only that).

thieta updated this revision to Diff 461315.Sep 19 2022, 12:41 PM

Relax libc++ version check and address some feedback

Can anyone have a look at this one? Would like to merge it if no-one objects.

hans accepted this revision.Sep 27 2022, 5:47 AM

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())}
...
]
This revision is now accepted and ready to land.Sep 27 2022, 5:47 AM
kwk accepted this revision.Sep 27 2022, 6:03 AM

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
ldionne accepted this revision.Sep 27 2022, 6:04 AM

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
thieta updated this revision to Diff 463435.Sep 27 2022, 11:39 PM
thieta marked 5 inline comments as done.

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.

This revision was landed with ongoing or failed builds.Sep 27 2022, 11:41 PM
This revision was automatically updated to reflect the committed changes.

Thanks everyone for the comments! I landed this now and we can always follow-up with fixes if something doesn't look right.