Should `git.tag` parameter override branch when executing on a branch?

Question

Should `git.tag` parameter override branch when executing on a branch?

l3ender opened this issue 3 years ago · 25 comments

Thank for the great plugin. We have a new use case for a project and are seeing results different than our expectations.

Our maven-git-versioning-extension.xml file (truncated for brevity):

<configuration>
	<refs>
		<ref type="tag">
			<pattern>([0-9]+[.]?)+</pattern>
			<version>${ref}</version>
			<updatePom>true</updatePom>
		</ref>

		<ref type="branch">
			<pattern>.*</pattern>
			<version>${version.release}-SNAPSHOT</version>
		</ref>
	</refs>
</configuration>

If running mvn clean -Dgit.tag=1.1.2 on a detached head, we correctly see the plugin match on tag.

However, if running the same on a branch, the plugin matches on branch and not the provided git.tag parameter. Is this expected or a bug?

It appears the reverse is working as expected: if a tag is checked out and -Dgit.branch=abc is provided, the plugin matches on branch.

Full testing repository can be found here: https://github.com/l3ender/maven-git-version-testing.

Thank you!

Answer 1 · 2021-09-24T06:32:48.000Z

I'll investigate.

Answer 2 · 2021-09-24T10:10:39.000Z

I found the problem: currently tags are only considered in detached mode or <refs considerTagsOnBranches="true"> is set.
But I think I'll slightly change the behavior if you provide a tag (e.g. -Dgit.tag=1.1.2 or GIT_VERSIONING_TAG=1.1.2).

I think following behaviour is reasonable:

if only tag is provided the extension should assume a detached head state
if branch is provided the extension should assume an attached head state

Answer 3 · 2021-09-24T10:59:15.000Z

Thanks for the quick response.

I should mention that this isn't a show-stopping bug for us because in normal usage we will only provide git.tag from our CI environment, which runs on a detached head. But I noticed the behavior when first testing on a normal repo for POC.

And also it seemed a little confusing how git.branch takes precedence when running in context of a checked out tag, while the reverse does not hold true.

We are not using considerTagsOnBranches due to the readme's warning on performance implications.

Answer 4 · 2021-09-24T12:11:33.000Z

Should be fixed, just upgrade to version 7.1.1

Regarding the warning I'll change it to an info icon because this is just a hint incase you facing performing issues while determining git version

Answer 5 · 2021-09-24T13:57:19.000Z

I removed the warning completely, for every 10k tags it will only add about 100 ms extra

Answer 6 · 2021-09-24T13:57:54.000Z

@l3ender could you confirm it works like expected now in version 7.1.1?

Answer 7 · 2021-09-24T14:09:03.000Z

Yes, I can confirm it works if providing only git.tag param when working on a branch.

Perhaps I'm overthinking--or it is a different issue--but why does it work only if passing git.tag and not also git.branch? Using plugin config which has tag ref is ordered higher than branch ref, running mvn clean -Dgit.tag=1.1.2 -Dgit.branch=abc:

on detached head -> matches on branch.
on branch -> matches on branch.
on tag -> matches on branch.

I may be thinking incorrectly, but my assumption is that using the arguments (single or multiple) would result in those taking priority over the working refspec which is checked out.

Thoughts?

Answer 8 · 2021-09-24T15:36:47.000Z

Following table should explain the behaviour I want to achieve. WDYT?

Provided Git Situation	Resulting Git Situation
none	real git situation
only git.tag	HEAD is DETACHED & <git.tag> pointing at HEAD
only git.branch	HEAD is attached to <git.branch> & NO tags pointing at HEAD
git.branch & git.tag	HEAD is attached to <git.branch> & <git.tag> pointing at HEAD

<ref type="tag">configs will be considered if HEAD is detached or <refs considerTagsOnBranches="true"> is set
<ref type="branch"> configs can only be considered if HEAD is attached to a branch

Answer 9 · 2021-09-24T18:15:54.000Z

I'm not quite sure I understand why it matters where HEAD is attached or detached. My thought is more of the following:

if git.branch is provided, it takes precedence over the scenario of HEAD being attached to a branch.
if git.tag is provided, it takes precedence over the scenario of HEAD being attached to a tag.
if both git.branch and git.tag are provided, they both take precedence over scenario of where HEAD is attached, and are evaluated in order according to plugin config.
if neither are provided, evaluation falls back to whatever HEAD is attached to:
- if attached to branch, branch will be used (unless considerTagsOnBranches=true).
- if attached to tag, tag will be used.
- if detached, falls back to to default <rev> entry (e.g. commit).

My thought is that the parameters are used for overriding the actual state of HEAD, so providing them would always take precedence. Am I overthinking it, or misunderstanding use of plugin?

Answer 10 · 2021-09-27T06:22:35.000Z

I other words

if any of git.branch or git.tag is set the real git situation will be overridden (branch and tag).
if only one override option is set this implies the other option set to nothing

Answer 11 · 2021-09-27T06:23:26.000Z

Could you specify a scenario where this behaviour is an issue?

Answer 12 · 2021-09-27T18:41:40.000Z

I think it makes sense that only one of git.branch or git.tag is provided. If providing either overrides the real git situation then I am good.

Will you be creating a github release for version 7.1.1?

Answer 13 · 2021-09-27T23:22:03.000Z

my bad forgot the git hub release, however its released on maven central already.

Answer 14 · 2021-12-14T10:27:16.000Z

Sorry to continue the discussion, but I am having a hard time understanding the changes of behavior from version 6 to 7.

I have a maven-git-versionning-extension.xml file with the following:

    <refs considerTagsOnBranches="true">
        <ref type="tag">
            <pattern>[0-9]+.[0-9]+.[0-9]+</pattern>
            <version>${ref}</version>
        </ref>

        <ref type="branch">
            <pattern>.+</pattern>
            <version>${ref.slug}-SNAPSHOT</version>
        </ref>
    </refs>

And running mvn release:clean release:prepare -DreleaseVersion=5.4.1 -Dtag=5.4.1 -Dgit.tag=5.4.1 leads to "You don't have a SNAPSHOT project in the reactor projects list."

I've tried the various combinations (branch ref before tag's, -Dgit.branch=master in addition, or alone), but to no avail. Either I am building the SNAPSHOT, or I end up in the above error message. But I can't make the release anymore.

Anything I missed in the doc ?

Answer 15 · 2021-12-14T10:57:33.000Z

@onekiloparsec Strange. If you provide a simplistic project with the latest working extension version, then I'll investigate.

Answer 16 · 2021-12-14T11:04:39.000Z

@onekiloparsec What happens if you just execute mvn help:evaluate -Dexpression=project.version -q -DforceStdout? Does it print the expected version?

Answer 17 · 2021-12-14T15:19:30.000Z

With both versions, I get the same thing: master-SNAPSHOT[IJ]-1-MojoSucceeded-[IJ]-source=CLI-[IJ]-goal=evaluate-[IJ]-id=com.aselta:aselta-core:jar:master-SNAPSHOT-[IJ]-error=

I don't have time right now to prepare a simple project. What I observe though is that changing from version 6.4.2 to 7.1.2 with the same mvn command doesn't yield at all the same result. And the migration isn't clear to me (I am not an expert in Maven stuff).

I don't want to make free criticism, I know how FOSS is time consuming (I do it myself too), and I thank you for your extension. But as for now, we'll stick to v6.

Answer 18 · 2021-12-16T08:02:08.000Z

@onekiloparsec could you try following command mvn help:evaluate -Dgit.tag='1.2.3' -Dexpression=project.version -q -DforceStdout

out put should be 1.2.3

Answer 19 · 2021-12-20T06:07:46.000Z

@onekiloparsec hi, unfortunately I was not able to reproduce your error. If you are able to provide me an example project feel free to open a new separate issue.

Answer 20 · 2022-03-14T16:50:01.000Z

Just for the archive, I can confirm I cannot use the version 7 of the extension, and currently stick to version 6. Various issues:

The mvn help:evaluate command mentioned above is a bit misleading. Since the output in the CI using mvn release:prepare release:perform is different. Moreover, the quotes around 1.2.3 makes the result different too.
I came to the conclusion that mvn release:prepare must resolve to SNAPSHOT but mvn release:perform must resolve to the tag provided. If the prepare resolves to the tag, we end up with the above error message "You don't have a SNAPSHOT project in the reactor projects list"
Unfortunately, I didn't managed to have a working version with the command split. Hence, my Jenkinsfile always has mvn release:prepare release:perform -D... in a single line.
The only way to ensure prepare resolves to SNAPSHOT and perform to the tag, is to not add a flag -Dgit.tag=1.2.3 in addition to -Dtag=1.2.3 -DreleaseVersion=1.2.3 The presence of -Dgit.tag=... forces the resolving to the tag value.
The question of @l3ender above has not been answered: I still don't quite get why it matters where HEAD is attached or detached. As far as I can tell, detached or not, in my context, I just can't have a resolving to SNAPSHOT then tag in my single-line mvn release in my Jenkinsfile.

Answer 21 · 2022-03-15T18:11:01.000Z

I don't think this extension makes sense in combination with release plugin. Furthermore have a look at Maven Release Plugin: The Final Nail in the Coffin, it is a pretty got explanation why you should ditch Maven Release Plugin.

Answer 22 · 2022-03-15T18:16:47.000Z

The question of @l3ender above has not been answered: I still don't quite get why it matters where HEAD is attached or detached.

This idea of this extension is to produce a 'branch version' if HEAD is attached to a branch. If HEAD is not attached and a tag is pointing to HEAD it will result in a 'tag version' (if configured)

Answer 23 · 2022-03-16T16:02:56.000Z

Ah god, thanks very much for your input. I don't know if I am alone, but I was totally unaware of these articles about the Maven Release Plugin (which indeed is a monstruosity). I would have been glad to see it mentioned in the documentation somewhere.

I am not a a senior expert on Maven. Just a VP Engineering building a new team of developers inheriting for a large legacy of multiple technos (it doesn't sounds like a paradise, but we're getting closer every day, one plugin at a time :-) ).

Answer 24 · 2022-03-17T08:39:06.000Z

Glad Ito hear that, if you have further questions regarding maven feel free to open an other issue.

Answer 25 · 2022-03-17T08:39:35.000Z

I'll add the blog post to the README.md