Skip to content
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

Add a build-number input to the AIO workflow #1935

Open
wants to merge 3 commits into
base: maintenance/gramps60
Choose a base branch
from

Conversation

stevenyoungs
Copy link
Contributor

Add a build-number input to the AIO workflow

When DEV_VERSION is not True, set the appbuild to the build-number input

@stevenyoungs stevenyoungs changed the base branch from master to maintenance/gramps60 February 7, 2025 00:02
@stevenyoungs
Copy link
Contributor Author

stevenyoungs commented Feb 7, 2025

@Nick-Hall can you run an AIO build on this branch please.
Then I would like a second build with gramps\version.py modified to DEV_VERSION=False and the workflow build-number input set to "beta1" (or whatever you would normally use)

@stevenyoungs
Copy link
Contributor Author

Observation: For DEV_VERSION=True
AIO sets
appbuild="r$(git rev-list --count HEAD)-$(git rev-parse --short HEAD)"
Debian sets
appbuild=$(git rev-parse --short $GITHUB_SHA)

should these be aligned?

@fxtmtrsine
Copy link

fxtmtrsine commented Feb 7, 2025

Minor detail for the beta release can it say that instead of "a new maintenance release"
gramps6message

should it be showing "r1" seems to mean release build 1 I'm guessing maybe should be beta1 build 1 "b1"?

@stevenyoungs
Copy link
Contributor Author

stevenyoungs commented Feb 7, 2025

should it be showing "r1" seems to mean release build 1 I'm guessing maybe should be beta1 build 1 "b1"?

The "r1" part comes from this piece of code
r$(git rev-list --count HEAD)
which is the number of git commits on the branch

With this PR

  • For a non-development build (DEV_VERSION=False)
    it will say "6.0.0-beta1" where "beta1" is whatever Nick types in when he runs a non-development build.
  • For a development build (DEV_VERSION=True)
    the version number will be the same as it is today: "6.0.0-r1-5a76f63" in your screenshot

Here's the first page of the installation wizard with this PR applied, DEV_VERSION=False and a build-number of "beta1"
image

Minor detail for the beta release can it say that instead of "a new maintenance release"

I'll take a look...

aio/build.sh Outdated
@@ -142,7 +146,11 @@ cp /mingw64/share/icons/hicolor/scalable/places/*.svg /mingw64/share/icons/gnome
# build gramps
rm -rf dist aio/dist
python setup.py bdist_wheel
appbuild="r$(git rev-list --count HEAD)-$(git rev-parse --short HEAD)"
if `grep -q '^DEV_VERSION\s*=\s*True' gramps/version.py`; then
appbuild="r$(git rev-list --count HEAD)-$(git rev-parse --short HEAD)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git describe is perhaps clearer and definitely simpler. Today's gramps60 HEAD produces v6.0.0-beta1-27-gf5e551c3f, composed of the tag, the number of commits following the tag, and the shortened sha1 of the commit. If the commit is tagged git describe returns just the tag, e.g.:

> git describe v6.0.0-beta1
v6.0.0-beta1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nick-Hall do you always tag before you do the release build? If so git describe looks like a good improvement

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We should probably use git describe instead of our existing get_git_revision script. The Gramps version reported from the command line and shown in the About dialog is wrong.

> python Gramps.py -v
6.0.0-beta2-149fb610ca

I bump the version number after each release and this is misleading.

I'll investiage.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus build.sh just uses VERSION_TUPLE so it loses the beta1 part making it even more misleading.

master's VERSION_TUPLE is 6.0.0 and its last tag is v5.2.4; neither is really right. In that case maybe "$(git branch)-$(git rev-list --count HEAD)-$(git rev-parse --short HEAD)" would be most correct... but perhaps we don't build AIOs from master so it doesn't matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "beta1" part is injected in via a workflow input and used if DEV_VERSION in version.py is False

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using git describe works well to provide a better description of development versions . See PR #1938.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "beta1" part is injected in via a workflow input and used if DEV_VERSION in version.py is False

That misses the point. 6.0.0-r1-5a76f63 as a dev build tag is misleading because there is no 6.0.0 yet.

@Nick-Hall Nick-Hall mentioned this pull request Feb 7, 2025
@stevenyoungs
Copy link
Contributor Author

stevenyoungs commented Feb 7, 2025

The table below shows the results of running ./build.sh false beta2 locally

DEV_VERSION Installer filename Image
True GrampsAIO-6.0.0-v6.0.0-beta1-20-g79c390cf1_win64.exe image
False GrampsAIO-6.0.0-beta2_win64.exe image

Note: By default the workflow checkout action only fetches a single commit. See https://github.com/actions/checkout/blob/v4/README.md
So we can either increase the fetch-depth or not use git describe

@Nick-Hall
Copy link
Member

The development version should be: GrampsAIO-6.0.0-beta1-20-g79c390cf1_win64.exe - You have an extra v6.0.0 that isn't needed.

The production version would ideally have the build number added: GrampsAIO-6.0.0-beta2-1_win64.exe

@Nick-Hall Nick-Hall added this to the v6.0 milestone Feb 7, 2025
@stevenyoungs
Copy link
Contributor Author

stevenyoungs commented Feb 7, 2025

You have an extra v6.0.0 that isn't needed.

That comes from git describe which gives
v6.0.0-beta1-17-g8e53577de
when run from the branch upstream/maintenance/gramps60

v6.0.0-beta1 is the tag on commit 5a76f63

When I run git describe on master I get
v5.2.4-281-gef5512c43
which whilst technically correct, it probably not useful

@stevenyoungs
Copy link
Contributor Author

Minor detail for the beta release can it say that instead of "a new maintenance release"

@fxtmtrsine I've simply removed the " -- a new maintenance release" suffix.
I don't think it adds anything useful, even in a maintenance release. Let me know if you feel differently and I can take another look.

@jralls
Copy link
Member

jralls commented Feb 7, 2025

When I run git describe on master I get v5.2.4-281-gef5512c43 which whilst technically correct, it probably not useful

That's why I suggested special-casing it with the branch.

@Nick-Hall
Copy link
Member

You could get the version qualifier with:

grep "^VERSION_QUALIFIER" gramps/version.py | cut -d\" -f2

For a development release you could get the appbuild with:

git describe | grep -o '[^-]*-[^-]*$'

and use the $2 parameter for production releases.

@stevenyoungs stevenyoungs force-pushed the aio-appbuild branch 3 times, most recently from e3265d4 to e98731f Compare February 11, 2025 22:26
@Nick-Hall
Copy link
Member

Nick-Hall commented Feb 11, 2025

@stevenyoungs It would still be useful to add a build number to a non-development release. I created 3 Windows builds for 6.0.0-beta1.

For DEV_VERSION use "<branch_name>-<short_commit_id>"
else use "<VERSION_QUALIFIER>-<build-number>"
@stevenyoungs stevenyoungs force-pushed the aio-appbuild branch 2 times, most recently from 1ca405f to aba610e Compare February 11, 2025 23:07
@stevenyoungs
Copy link
Contributor Author

stevenyoungs commented Feb 11, 2025

I've revised the approach based on earlier feedback. For DEV_VERSION=True it uses "<branch_name>-<short_commit_id>". For other builds it uses "<VERSION_QUALIFIER>-"

The table below shows the output for both values of DEV_VERSION on a branch called aio-appbuild. The workflow Build number input was set to 1 for both builds. gramps/version.py had the following for both builds:
VERSION_TUPLE = (6, 0, 0)
VERSION_QUALIFIER = "-beta2"

DEV_VERSION Installer filename Image
True GrampsAIO-6.0.0-aio-appbuild-e98731f_win64.exe image
False GrampsAIO-6.0.0-beta2-1_win64.exe image

@Nick-Hall this is ready for review. I ran both builds in my own github repo but it would be good to get an AIO build run in the official repo too.

@jralls
Copy link
Member

jralls commented Feb 12, 2025

VERSION_QUALIFIER = "-beta2"

This is OK for beta releases and the like but I'm not comfortable with modifying Gramps code to reflect a packaging change. I'd be a lot happierf it can work by looking up something in the bundle, maybe an environment variable.

It would still be useful to add a build number to a non-development release. I created 3 Windows builds for 6.0.0-beta1.

I think you mean that it took you three tries to get a working bundle and that somebody else told you that the first two were broken so you had to change the build number. If something changes in Gramps code then it really deserves a new tag and therefor a release number, e.g. v6.0.0-beta1a or -beta2.

@stevenyoungs
Copy link
Contributor Author

This is OK for beta releases and the like but I'm not comfortable with modifying Gramps code to reflect a packaging change. I'd be a lot happierf it can work by looking up something in the bundle, maybe an environment variable.

For a full release version, VERSION_QUALIFIER = "". The workflow has an input called Build number which defaults to 1. So the initial release build would produce a file called GrampsAIO-6.0.0-1_win64.exe. This mirrors the 5.2.4 AIO release GrampsAIO-5.2.4-1_win64.exe. If the packaging has to change, a different build number input can be provided to the workflow e.g. 2 and the name then becomes GrampsAIO-6.0.0-2_win64.exe. I think this meets your suggestion of not getting this value from the repo itself. Let me know if I missed something

If something changes in Gramps code then it really deserves a new tag and therefor a release number, e.g. v6.0.0-beta1a or -beta2.

This is what happens for the filename. Instead of beta1a it is 6.0.0-beta1-1, 6.0.0-beta1-2 and 6.0.0-beta1-3 etc. The current AIO build is GrampsAIO-6.0.0-beta1-3_win64.exe. These were a bit more than re-runs of the workflow - there were new commits in the repo. albeit in AIO only files. I don't see any tags to prove that though.

@Nick-Hall
Copy link
Member

To create GrampsAIO-6.0.0-beta1-2 I created a branch off the v6.0.0-beta1 release tag called aio-6.0.0-beta1. It contains the single commit ba2c71f which changes the AIO build script to use cx-Freeze 3.8 (3.9 contained a bug). When 3.10 was released, I created GrampsAIO-6.0.0-beta1-3 from the original aio-6.0.0-beta1 release tag again.

The problem was that the About dialog didn't distinguish between the three releases. Perhaps the build number would be better described as an AIO release number.

@Nick-Hall
Copy link
Member

it would be good to get an AIO build run in the official repo too.

I think that this is good enough for me to use for the next beta release.

@emyoulation
Copy link
Contributor

emyoulation commented Feb 12, 2025

With Paul finding time for Gramps again, can the loss of the AIO pinning "Gramps Console" and GrampsW app Icons to the Start Menu be reviewed?

6.0beta1 is not adding the icons. You have to search after installing and Pin them to the Start menu (and Taskbar, but that's less frustrating.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants