rnpgp / rnp

RNP: high performance C++ OpenPGP library used by Mozilla Thunderbird
https://www.rnpgp.org
Other
199 stars 55 forks source link

version.txt at v0.15.1 contains "0.15.0" #1519

Closed dkg closed 3 years ago

dkg commented 3 years ago

Looks to me like version.txt didn't get updated before making tag v0.15.1, because its contents still contains 0.15.0.

While i could offer a specific fix for the version.txt file, i think the more important update needs to be made in whatever tooling is used to make an rnp release, so that version.txt gets automatically updated before a release.

ronaldtse commented 3 years ago

@dkg thank you for the feedback, you are absolutely right here. We need to update the release phase so that version.txt reflects the actual version.

dkg commented 3 years ago

I'm currently patching it in the sources i've uploaded to debian, which are still stuck in NEW.

ronaldtse commented 3 years ago

Right now the build process reads from version.txt: https://github.com/rnpgp/rnp/blob/ed58f7dad19260448c21dfdab0781d69275d0562/cmake/version.cmake#L78-L81

However, the generation of version.txt is manual, as described in the release workflow document: https://github.com/rnpgp/rnp/blob/master/docs/develop/release-workflow.adoc#update-changelog-and-version

This should be automated in the release process. We should update CPack related code so that the version.txt file is automatically generated at release, but not committed to the actual tag.

Here's my process of building a release for RNP on macOS at the tag v0.15.1 (I didn't do it, but just to demonstrate):

$ git checkout v0.15.1
$ brew bundle
$ . ci/env-macos.sh.inc
$ cmake -B build
-- RNP_VERSION: 0.15.0
-- RNP_VERSION_NCOMMITS: 0
-- RNP_VERSION_GIT_REV: 0
...
$ cmake --build build
$ cpack --config build/CPackSourceConfig.cmake
$ cpack --config build/CPackConfig.cmake

And here you see the problem. cmake/version.cmake is reading version.txt to get 0.15.0. If I remove version.txt and rerun cmake -B build, I get:

$ cmake -B build
-- RNP_VERSION: 0.15.1
-- RNP_VERSION_NCOMMITS: 0
-- RNP_VERSION_GIT_REV: 1af627e
...

And continuing:

$ cpack --config build/CPackSourceConfig.cmake
CPack: Create package using TBZ2
CPack: Install projects
CPack: - Install directory: /Users/me/src/rnp/rnp
CPack: Create package
CPack: - package: /Users/me/src/rnp/rnp/rnp0-0.15.1-Source.tar.bz2 generated.
CPack: Create package using TGZ
CPack: Install projects
CPack: - Install directory: /Users/me/src/rnp/rnp
CPack: Create package
CPack: - package: /Users/me/src/rnp/rnp/rnp0-0.15.1-Source.tar.gz generated.
CPack: Create package using TXZ
CPack: Install projects
CPack: - Install directory: /Users/me/src/rnp/rnp
CPack: Create package

Which is much better.

I believe it would be even better to move the archive release process to GitHub entirely: https://github.community/t/how-to-upload-release-assets-from-multiple-oses-into-one-release/136815/2

To move forward:

  1. Remove version.txt from the release tags and the described process. We don't need them.
  2. Auto-generate version.txt from the CMake release process
  3. Use a GHA release flow, that on pushing a version tag (e.g. v0.16.0), automatically builds and pushes the packed archives to GH Releases.

This way the result will be predictable.

Thoughts?

dkg commented 3 years ago

I would personally prefer if the release tarballs exactly match the contents of the git repository -- it makes it much more straightforward for me to work with the tarballs, as required/encouraged by debian, while also working directly from the upstream revision history (which makes it easier to exchange substantive patches).

I think the process you're describing doesn't do that, so it's not my preference. But if it is what works better for you, I can roll with it.

ni4 commented 3 years ago

It should be my failure, just forgot to update the file before doing a release. As far as I remember version.txt was added to the release branch since we had a problem with ruby-rnp tests, running on a release branch - it was not able to run correctly till there were no version.txt or tag, while tag was pushed afterwards.

As for me, it would be easier to add CI check which works on release branch and checks whether there is a tag with value from version.txt.

ronaldtse commented 3 years ago

The easiest way to resolve @dkg's concern is to maintain a version.txt in the main branch. If we do this we will have to update version.cmake that in the case of a non-release tag, we will continue using the git commit as versioning instead of the value from version.txt. Thoughts?

ni4 commented 3 years ago

@ronaldtse Agree with this approach! Kinda weird to see rnp 0.0.0+git20210604.78403bb while building from sources.