-
Notifications
You must be signed in to change notification settings - Fork 250
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
Improved versioning #738
Improved versioning #738
Conversation
Automatically generated build artifacts for commit 3d161a8 (note: these links will expire after some time):
To test this PR, download and unzip the flatpak application above and then install with:
|
Looking good. My only suggestion for a fallback version is that "3.0.2-derived" or some such notation would be more informative than "unreleased". After the 3.0.3 release the VERSION file would be updated to ""3.0.3-derived". However, this is of marginal importance so feel free to drive on! |
Right, that would make some sense, but be slightly harder to maintain (but might be good to do if we automate that later). In practice, it should not matter much, since the Here's a bit of documentation (that I wanted to write anyway) about how the versioning now works (to be put somewhere): For determining the hamster version, there are currently two interfaces:
To find the actual version number, three options are tried in order:
|
Created wiki page Versioning, for now. |
There is a .bumpversion and setup.py in the root. You may want to review or drop them in this endeavour. |
Automatically generated build artifacts for commit 7f9bdb3 (note: these links will expire after some time):
To test this PR, download and unzip the flatpak application above and then install with:
|
13d177e
to
c01b74c
Compare
Automatically generated build artifacts for commit c01b74c (note: these links will expire after some time):
To test this PR, download and unzip the flatpak application above and then install with:
|
c01b74c
to
4bf6e78
Compare
Automatically generated build artifacts for commit 4bf6e78 (note: these links will expire after some time):
To test this PR, download and unzip the flatpak application above and then install with:
|
Good point, I've removed them. I've also pushed a few other commits and did some more testing, I think this is now ready for final review. @aquaherd, @DirkHoffmann, maybe you could have another review pass over the new commits? If ok, let me handle the merge, there is still a fixup commit to be squashed before merging. |
@@ -44,6 +44,10 @@ | |||
</screenshots> | |||
<translation type="gettext">hamster</translation> | |||
<url type="homepage">https://github.com/projecthamster/hamster/wiki</url> | |||
<releases> | |||
<!-- Provide a (clearly) fake date, otherwise flatpak will ignore the entry --> | |||
<release version="@VERSION@" date="1980-01-01" /> |
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.
date should be an easter egg: "2007-08-01" Toms first commit to the repo
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.
I'm afraid that such a date might actually look like its meaningful, 1980-01-01 is even more clearly invalid/fake. I'm not entirely sure where this date will actually show up, though, but I'm inclined to leave it as is?
I have built a tar release with ./waf dist but the magical substitution from defs.py.in to defs.py did not happen. Does the release pipeline handle this? AFAIK, downstream packagers prefer to download the tar.gz file and do not do a git checkout from github just to get a version string. |
Right, I did not look at that case yet. I think it should (be made to) include the
For at least Debian, I think that is the case, though the previous release did not have a tarball from us, so I'm assuming @rhertzog has used a git export or something like that. Also, the approach with adding a The question is, though, should we (as part of the release process) generate such a tarball (either directly from git, or using |
I just tried |
The problem is the packagers: They only monitor the most recent tag, add that to a github URL and then unpack the github generated tar file, dropping the first folder level that is the only thing that contains a string like v3.0.3. Previously, the 3.0.2 was inside the sources. I have not yet seen a release build so I ight be wrong. I don't see any modification on how the release tar is built here, so I assume it is just a tar from a git tag, that does not include the tag name in the tar file. |
The plan is to write 3.0.3 to |
Not sure, if it is relevant for this issue, and if it is to be fixed on your side or on my test system, but I get these two messages from flatpak when installing Hamster-3.0.2-81-g14262.flatpak:
|
Shouldn't it be "rcN" (with N=1, 2, …)? |
This I fixed today in #739, but this PR is not rebased on top of that yet.
That would make sense for explicit release candidate images, but @GeraldJansen is talking about the fallback |
I think all concerns have been answered. In the interest of progress, I'm going to go ahead and merge this already, so we can prepare for the last steps of the release. If there's anything that still needs to be changed or discussed, before or after the release, feel free to leave more comments. |
Previously, if not installed and no git version could be deduced, a hardcoded 3.0.2 version was used. Now, this fallback version is moved into an external VERSION file and is changed to "unreleased" instead of "3.0.2", to ensure that a proper version is only returned when it is really correct. This also makes it easier to update the version number for releases, and prepares for having a single place in the code to store the release version.
This prepares for using this code from waf, since it can now be imported without additional (gtk) dependencies.
This allows running the src/hamster/version.py file as a standalone script that just outputs the version number and lets wscript call that. This ensures that the exact same version number is used between installed and non-installed versions, and removes the second place where the release version was previously hardcoded. To do this, the version.py code is split into a part that decides the version when installed, and a bit that decides the version when running from the source tree. This is needed since when running the script from waf, sys.path is not set up to allow importing hamster.defs, and trying this might result in importing hamster.defs from a system-wide installed hamster instead of the current source dir. So just skip the installed-version detection entirely when running from waf, it is not relevant anyway.
Released versions omitted the v prefix (to make proper semantic versions), but version numbers autodetected from git would not, leading to discrepancies. This fixes that.
This file is mostly used by packaging tools, such as flatpak, to get info about the application. Since flatpak uses org.gnome.Hamster as the main package id, the metainfo.xml must be named the same for it to be used. Possibly more things need to be renamed for all application ids to be consistent, but it is non-obvious what the best approach is there (see also #725 for details). For now, this pragmatically renames metainfo.xml to ensure it is found by flatpak.
This is now done by waf at install time and ensures that flatpak knows the hamster version of the build.
Without a release date, flatpak ignores the version number, but autodetecting the release date (and carrying it into tarballs) might be too much work, so just hardcode an obviously fake date for now.
This was not actually used in practice and no longer matches the recent changes to the version handling, so remove it.
This was added a long time ago only for the database backend, but seems to be outdated. In general, hamster cannot be installed using setup.py anyway, since it requires dbus service files, gsettings schemas, etc. so waf is used for all that. Remove setup.py which is no longer used anyway. This fixes #581.
Note: For pull requests, the version that is tested is an autogenerated temporary merge commit (which is good, since it tests whether things work *after* merging), but that commit id is also included in the filename which might be bit suprising. To be considered later.
4bf6e78
to
c062980
Compare
This implements the improvements suggested at #608 (comment). Practically, this PR makes the following improvements:
src/hamster/VERSION
instead of being hardcoded in multiple places.unreleased
instead of3.0.2
(i.e. when running from a source checkout or tarball that is not from a released version, and git cannot be used to determine the version, this no longer incorrectly shows3.0.2
).v
prefix stripped, making version numbers consistent between git checkouts and release tarballs.I'm marking this as a draft since I still need to do a bit more testing, and because I found out just before I ran out of time for today, that flatpak seems to insist on having a release date with the version number in
metainfo.xml
and otherwise just ignores the version number entirely (F: Ignoring release element without timestamp or date
). To solve this, we should either:I've implemented 2. for now, but comments are welcome.