miniver icon indicating copy to clipboard operation
miniver copied to clipboard

miniver does not use latest tag for version

Open kmpf opened this issue 3 years ago • 6 comments

Another issue I came across, was that the version used for the package build by miniver was not based on the latest git tag:

$ git tag
v1.0.1
v1.0.1p
v1.0.1p1
v1.0.2
v1.0.3
v1.1
v2.0.0
v2.0.0rc1

Given these tags, the resulting package name was:

  • <package-name>-1.0.3.dev11+ge33b9565.dirty

I have to admit, that the tag names are not that well chosen, but the error still seems strange.

kmpf avatar Apr 14 '21 08:04 kmpf

I've seen this and haven't spent enough time to pinpoint.

However, I believe it has to do with branches. Possibly the tag was created on a now non-existent branch?

chinghwayu avatar Apr 19 '21 20:04 chinghwayu

@kmpf This is probably because you are querying for the version from a commit that does not have the later tags (v1.1, v2.0.0 etc.) as ancestors.

If you link to the repo in question and what commit you're pointing at then we can debug further.

here's the logic that determines the latest version.

We first try

git describe --long --always --first-parent

then, if that errored out, we try

git describe --long --always

The man page for git describe says the following about the --first-parent flag:

--first-parent Follow only the first parent commit upon seeing a merge commit. This is useful when you wish to not match tags on branches merged in the history of the target commit.

So as @chinghwayu says this is probably related to having tags that are on different branches.

jbweston avatar Apr 25 '21 03:04 jbweston

Hi, sorry for the late reply. I work on a non-public version, but I tested it with the master of https://github.com/fraunhofer-izi/uap

I think the behaviour has to do with the difference in annotated and unannotated tags. After cloning the above mentioned repo I did:

$ git describe --long --always --first-parent
a7980c51
$ git describe --long --always
v1.0.3-1413-ga7980c51
$ git describe --long --always --tags
v2.0.0rc2-76-ga7980c51

I don't know which kind if behaviour you want miniver to have. You could provide an option to use unannotated tags, maybe?

kmpf avatar May 10 '21 20:05 kmpf

Just realized there already is a patch to add the --tags switch. The documentation did not trigger my attention, because I did not see the error mentioned there.

kmpf avatar May 11 '21 06:05 kmpf

I don't know which kind if behaviour you want miniver to have. You could provide an option to use unannotated tags, maybe?

Just realized there already is a patch to add the --tags switch. The documentation did not trigger my attention, because I did not see the error mentioned there.

Ah, yes... ran into this as well.

What if git describe was executed twice, with and without --tags? If results are different, print a warning. If a switch such as -u is used, execute only once with --tags.

chinghwayu avatar May 14 '21 16:05 chinghwayu

I wanted to use miniver for my project, but then I just hit the issue of forced usage of --first-parent. I don't think it's a good idea to deviate from the git defaults and hardcode in a different approach, with no option to change it. Consider this very common scenario:

 ●─╮ [develop] Merge branch 'master' into develop
 │ ∙ [master] <1.1> publish a hotfix
 ∙ │ do some development
 ◎─╯ <1.0> initial commit

In this case, git describe outputs 1.1-2-g445f312, which is correct, because my codebase is based on version 1.1. However, git describe --first-parent (which is what miniver does by default) outputs 1.0-2-g445f312, which is incorrect and misleading. When I see my program (running on current develop) claiming that it's 1.0+something, it seems like the hotfix is not included. This also brings inconsistency between the program (using miniver) and e.g. tarball name/prefix generated through git describe).

I feel like the default approach should be changed, or at least an option to set this should be offered (perhaps as an argument to miniver install, if possible), ideally both.

kparal avatar Feb 23 '22 09:02 kparal