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

Determine version from git tags #457

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zod
Copy link
Collaborator

@zod zod commented Aug 3, 2022

Instead of manually specifying the version we can use the git tag information using com.palantir.git-version. This gives us sensible version information in branches without manually increasing version numbers.

For android versionCode there also exists gradle-android-git-version but it seems rather unmaintained. I think we can come up with some own version based in the palantir plugin which determines our versionCode if we want to have it auto generated as well.

@zod
Copy link
Collaborator Author

zod commented Nov 14, 2022

I've kept this in draft because I wanted to get some feedback if we want this functionality. I thought this is rather controversial because in the past BRouter releases weren't even tagged.

@zod zod requested review from afischerdev and abrensch November 14, 2022 20:53
@zod zod force-pushed the gradle-git-version branch from 5fa6653 to 09a73fc Compare November 14, 2022 21:33
@zod zod force-pushed the gradle-git-version branch from 09a73fc to 80427ac Compare November 14, 2022 21:53
@zod zod temporarily deployed to BRouter November 14, 2022 21:54 Inactive
Copy link
Contributor

@polyscias polyscias left a comment

Choose a reason for hiding this comment

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

I see that with this change the version becomes something like:

> git describe --tags --always --first-parent
v1.6.3-46-g91459db

That does look to me like a good idea.

build.gradle Outdated Show resolved Hide resolved
@afischerdev
Copy link
Collaborator

I'm not a friend of this feature. But I see that sometimes it could be useful to have an extra counter.

I've been playing around with your branch.

  1. I downloaded a ZIP file. Doesn't work - doesn't have git to get data from it.
  2. a clone on your branch. Does not work - has no tag. Therefore the lib or apk name is specified only by a last commit number.
  3. More personal: my workflow is using the lib-1.6.4 in a lib folder together with lib-1.6.3 - e.g. compare output. Every name change needs a classpath change.

These are all small side effects, but make development more difficult than necessary.

@zod
Copy link
Collaborator Author

zod commented Nov 16, 2022

  1. I downloaded a ZIP file. Doesn't work - doesn't have git to get data from it.

I usually just git clone projects but I guess people who aren't that used to git prefer downloading source archives. There are workarounds if we still want to support building without git information

  1. a clone on your branch. Does not work - has no tag. Therefore the lib or apk name is specified only by a last commit number.

That's intentional. Version increments should be triggered using tags. A version should uniquely identify a source revision. Increasing version numbers in branches for stuff that isn't released yet is bad style because you can no longer determine the revision from the version number.

  1. More personal: my workflow is using the lib-1.6.4 in a lib folder together with lib-1.6.3 - e.g. compare output. Every name change needs a classpath change.

I think this can be easily solved using wildcards or getting the version using ./gradlew -w :printVersion.

These are all small side effects, but make development more difficult than necessary.

We would add a dependency for building with correct version information, which has it's pros and cons. As we had issues in the past with F-Droid builds from development versions I'd like to avoid this issue by either avoiding version increments before releases or automatic versioning using git.

@afischerdev
Copy link
Collaborator

afischerdev commented Nov 17, 2022

  • Zip export:
    I would like to hold this. When there is a workaround it's ok. I haven't tested it now.
  • Git clone without tag
    That's intentional. Yes and it's a good way. But the local result has the useful name 'brouter-80427ac.dirty-all.jar'
  • F-Droid
    The deal with F-Droid is to use the git tag and not the version number.

More personal workflow
To my testing tasks I download every new ZIP file from the BRouter action folder. So my download folder contains this at the moment:
brouter-1.6.3-all.jar
brouter-1.6.4-all.jar
To make sure I use the last versions for comparing.
In the future I will get this content:
brouter-v1.6.3-46-g91459db-all.jar
brouter-v1.6.3-47-g91459ee-all.jar
brouter-v1.6.3-48-g91459ff-all.jar
brouter-v1.6.3-49-g91459dd-all.jar
brouter-v1.6.3-50-g91459aa-all.jar
Pretty clear witch one I have to compare. So I have to consult the repository to see clearer.

Same with creator in gpx files.

To compare something I use a batch e.g. (very short form)
java -cp brouter-1.6.3-all.jar btools.test.test old.gpx
java -cp brouter-1.6.4-all.jar btools.test.test new.gpx
new variant e.g.
java -cp brouter-v1.6.3-50-g91459aa-all.jar btools.test.test old.gpx
java -cp brouter-v1.6.3-47-g91459ee-all.jar btools.test.test new.gpx
Where are the wildcards? The only idea here is to sort it into additional folders.

At the end of the development is the final version. Every thing is done. So we can tag it.
But - upps - we need a new sentence in the readme. So we have already a new version 'brouter-v1.6.4-1-g91459aa-all.jar' with the automatic.

I know you weren't too happy with my test variant of the two version names. But I still think it's a good and clear idea. Maybe not compliant with all rules, but understood by all testers. And we don't have many. So why scare some away with unsightly and unclear names?

If I may express one wish, if already version automatic, I would like to see it that way
brouter-1.6.3-46-g91459db-all.jar
brouter-1.6.4-47-g91459ee-all.jar
brouter-1.6.3-48-g91459ff-all.jar
brouter-1.6.3-49-g91459dd-all.jar
brouter-1.6.3-50-g91459aa-all.jar

@zod
Copy link
Collaborator Author

zod commented Nov 20, 2022

  • Git clone without tag
    That's intentional. Yes and it's a good way. But the local result has the useful name 'brouter-80427ac.dirty-all.jar'

I didn't push the tags to my fork. It works when using the abrensch repository (e.g. our GitHub action) or locally using

git clone https://github.com/abrensch/brouter
cd brouter
git remote add zod https://github.com/zod/brouter
git fetch --all
git checkout gradle-git-version
./gradlew -q :printVersion # prints v1.6.3-48-g80427ac

To compare something I use a batch e.g. (very short form) java -cp brouter-1.6.3-all.jar btools.test.test old.gpx java -cp brouter-1.6.4-all.jar btools.test.test new.gpx new variant e.g. java -cp brouter-v1.6.3-50-g91459aa-all.jar btools.test.test old.gpx java -cp brouter-v1.6.3-47-g91459ee-all.jar btools.test.test new.gpx Where are the wildcards? The only idea here is to sort it into additional folders.

The version actually contains more information and an increasing number which shows later commits. You can pick the latest version by sorting, similar to our server.sh

At the end of the development is the final version. Every thing is done. So we can tag it. But - upps - we need a new sentence in the readme. So we have already a new version 'brouter-v1.6.4-1-g91459aa-all.jar' with the automatic.

If you already pushed your tag you just follow the guidelines in the git documentation. This is no different when using those version information. The intention of a tag is to identify a version and should match anyway.

I know you weren't too happy with my test variant of the two version names. But I still think it's a good and clear idea. Maybe not compliant with all rules, but understood by all testers. And we don't have many. So why scare some away with unsightly and unclear names?

I think for testers those names are better because they can determine if they already downloaded this version or some previous version of 1.6.4. Because 1.6.4 is currently a moving target and no version.

If I may express one wish, if already version automatic, I would like to see it that way brouter-1.6.3-46-g91459db-all.jar

Unfortunately that's not possible with com.palantir.git-version, but I think switching to me.qoomon.git-versioning because it's more flexible and doesn't require a workaround for source code archives.

@afischerdev
Copy link
Collaborator

Thanks for the remarks.

The version actually contains more information and an increasing number which shows later commits. You can pick the latest version by sorting, similar to our server.sh

That's a good point. I'll try to find a Windows version of this.
May be something like this
dir ..\lib\*.jar /O-N /B | cmd /q /v:on /c "set/p .=&echo(!.!"

But anyway, this is only useful if you focus on one project.
E.g. comparing results of two projects: both are started at the same point on the origin. Both made one commit and end in
project1 v1.6.3-49-g80427ac
project2 v1.6.3-49-g80427de
or drifting on more commits. Wildcards will not help here. A clear version name will, when is a bounded on a project.

If you already pushed your tag you just follow the guidelines in the git documentation.

Ok, that's useful info.

but I think switching to me.qoomon.git-versioning

Did you try the other one already?
I guess not ZIPable again.

And yes, this way of version number contains more information. But always useful?
What about using this way only on action builds for the master?

@zod zod force-pushed the gradle-git-version branch from ee6aee1 to 72bee72 Compare November 23, 2022 20:20
@zod zod temporarily deployed to BRouter November 23, 2022 20:27 Inactive
@zod
Copy link
Collaborator Author

zod commented Nov 23, 2022

But anyway, this is only useful if you focus on one project. E.g. comparing results of two projects: both are started at the same point on the origin. Both made one commit and end in project1 v1.6.3-49-g80427ac project2 v1.6.3-49-g80427de or drifting on more commits. Wildcards will not help here. A clear version name will, when is a bounded on a project.

How would this work with your current workflow with increasing the version number in a branch? Would another branch/project just use 1.6.5 in this case because the update-version branch already used 1.6.4?

I've switched to me.qoomon.git-versioning which allows specifying own components of the version string and included the branch name. This should keep the version information unique.

Did you try the other one already? I guess not ZIPable again.

Yes, I've moved the version number to gradle.properties to treat it as fallback if the plugin can't determine a version number.

And yes, this way of version number contains more information. But always useful? What about using this way only on action builds for the master?

What's the harm of adding this information?

You can disable it locally and overwrite the version information by using

VERSIONING_DISABLE=true ./gradlew :version -Pversion=1.6.4 -q # prints 1.6.4

@afischerdev
Copy link
Collaborator

I've switched to me.qoomon.git-versioning

Great, I had a test with all variantes of cloning, all worked as expected. And ZIP was not breaking.
But one thing:
BRouter version is in all variantes is null. This doesn't work. Control over build reports standard output in server folder.
The creator in gpx is set 'normal'.
... creator="BRouter-0.0.0-489-gradle-git-version-gba9a8f6" ...
Great idea to add the repo name, that helps a lot

You can disable it locally and overwrite the version information by using

That are great news as well.

@zod
Copy link
Collaborator Author

zod commented Nov 30, 2022

Great, I had a test with all variantes of cloning, all worked as expected. And ZIP was not breaking. But one thing: BRouter version is in all variantes is null. This doesn't work.

It works if you follow the instructions above.

Control over build reports standard output in server folder.

I have no idea what that means.

@afischerdev
Copy link
Collaborator

Control over build reports standard output in server folder.

I have no idea what that means.

It means that I have controlled the junit reports.
It shows that the version is null for the server.

It works if you follow the #457 (comment).

I did all three export ideas. No version.

@rkflx rkflx mentioned this pull request Jul 17, 2023
@rkflx
Copy link
Collaborator

rkflx commented Jul 17, 2023

+1 for the concept (I only tested it locally, cannot comment on effects on the CI).

BRouter-Web might use a SemVer-compatible version check in the future. For this too keep working with this PR could you please change from hyphen (-) to plus (+) when appending build metadata, see 9. and 10. in spec. Otherwise e.g. 1.6.3-46-g91459db will be considered a prerelease version of 1.6.3 (which would not be the case for 1.6.3+46-g91459db).

brouter-core/src/main/java/btools/router/OsmTrack.java Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
@rkflx
Copy link
Collaborator

rkflx commented Aug 20, 2023

Conversation continued from #605:

@rkflx do you like to finish the #457 first?

@afischerdev I'm not sure what you mean by that and why you are @-mentioning me.

@rkflx When I wrote this I think I remembered you are a friend of git driven version numbers. And may be you like something to do in that direction.

I still don't understand. This very PR does just that, and I already mentioned that it worked just fine in my testing. Nevertheless it has been blocked since nearly 9 months by your objections, despite the author going out of their way to accommodate your concerns. It is not for me to "finish" something here.

I did all three export ideas. No version.

@afischerdev Is this still an issue for you?


@zod Are you still interested in this? If so, I'd like to encourage you to rebase on master, to bump the fallback version in gradle.properties (probably to 1.7.3), to implement the recent suggestions (static and +) and to un-draft. If not, kindly let me know and I'll see what I can do.

@afischerdev
Copy link
Collaborator

@afischerdev Is this still an issue for you?

I remember a fallback when git information is not present. Was working well.
But I'll test that again.

@zod zod force-pushed the gradle-git-version branch from 6c607e8 to 81a85dc Compare September 19, 2023 20:49
@zod zod temporarily deployed to BRouter September 19, 2023 20:49 — with GitHub Actions Inactive
@zod zod force-pushed the gradle-git-version branch from 81a85dc to 8fd348f Compare September 19, 2023 20:50
@zod zod temporarily deployed to BRouter September 19, 2023 20:50 — with GitHub Actions Inactive
Copy link
Collaborator

@afischerdev afischerdev left a comment

Choose a reason for hiding this comment

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

Thanks for the update
My tests:

  • download your version as zip
    generates a brouter-1.7.3-all.jar
  • cloned version with the recommended steps
    generates a brouter-1.7.3+8-gradle-git-version-g8fd348f-all.jar

Both have in RouteServerTest a 'BRouter null'
Please see Standard output at ../brouter-server/build/reports/tests/test/classes/btools.server.RouteServerTest.html

But I think we can ignore that. More tests with the generated jars bring out the expected version in standard output and gpx export.

@rkflx
Copy link
Collaborator

rkflx commented Sep 22, 2023

LGTM

(The only additional nit I noticed: Now that we include branch names in build results, we might want to improve sorting in server.sh a bit. I'll look into that and possibly submit a follow-up PR after this one has landed.)

@zod Thank you for rebasing. Note that merging the PR is blocked as long as it still is in draft state.

@zod zod marked this pull request as ready for review September 22, 2023 20:29
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.

4 participants