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

Repackage goal does not lazily resolve "project.build.finalName" anymore #16456

Closed
marcust opened this issue Apr 4, 2019 · 13 comments
Closed
Assignees
Labels
type: regression A regression from a previous release
Milestone

Comments

@marcust
Copy link

marcust commented Apr 4, 2019

I'm using git-commit-id-plugin to build my final name on <build> level like

<finalName>${project.name}-${version.number}</finalName>

and version.number is defined as

<version.number>${git.commit.time}-${git.commit.id.describe-short}</version.number>

That used to work up to now, but 2.1.4 does not evaluate these variables anymore, so I get

project-${git.commit.time}-${git.commit.id.describe-short}.jar
project-20190404-115039-0bfd654.jar

in my target directory, where the one with the unevaluated variables is the actuall repackaged jar.
I tried now half an hour to achieve the same thing any other way and didn't find a solution, I guess that has something to do with #16202. Can we get the old behaviour back?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 4, 2019
@snicoll
Copy link
Member

snicoll commented Apr 4, 2019

It is hard to understand what you’re trying to describe with partial config snippet. Can you share a sample that reproduces what you’re describing?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Apr 4, 2019
@snicoll
Copy link
Member

snicoll commented Apr 4, 2019

Small note, if you're using the finalName property of the plugin, it's deprecated since 2.1.0 in favour of the standard finalName of the build element. If you're using the former, this was working by accident (see also #16457 for more insight).

@marcust
Copy link
Author

marcust commented Apr 4, 2019

Demo is at https://github.com/marcust/spring-boot-issue-16456

So when you do a
mvn clean package
right now, you get in target/

demo-${git.commit.time}-${git.commit.id.describe-short}.jar
demo-20190404-181706-307869c.jar

if you do that with Spring Boot 2.1.3. you get

demo-20190404-181706-307869c-dirty.jar
demo-20190404-181706-307869c-dirty.jar.original

so basically the behaviour of the standard fileName element has changed.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 4, 2019
@snicoll snicoll changed the title Strange finalName behaviour in 2.1.4 Repackage goal does not lazily resolve "project.build.finalName" anymore Apr 5, 2019
@snicoll snicoll added type: regression A regression from a previous release and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Apr 5, 2019
@snicoll snicoll added this to the 2.1.5 milestone Apr 5, 2019
@snicoll
Copy link
Member

snicoll commented Apr 5, 2019

Alright so my attempt at stopping users from specifying a read-only parameter was a bit too agressive. The fact that read-only is completely ignored by Maven is a confirmed bug so we need to find a way to fix #16202.

A revert of 04aadcd will reintroduce the issue so it would be nice if we could detect it and throw an exception instead.

There is also some insight in git-commit-id/git-commit-id-maven-plugin#256

@juergenschuft
Copy link

juergenschuft commented Apr 24, 2019

Hi @snicoll, if I understand your comment #16456 (comment) correctly, the finalName tag in build element works by acciddent. Will it work on purpose in the future (or: may I use it)? Regards, Jürgen

@snicoll
Copy link
Member

snicoll commented Apr 25, 2019

I meant the former, not the latter. I've edited my comment. You're not supposed to use the one on the plugin as its read-only but there is unfortunately a bug in Maven that doesn't enforce that.

Irrespective of that, there is a regression that finalName is not lazily resolved anymore which is what this issue will fix.

@focbenz
Copy link

focbenz commented Apr 25, 2019

I would have also raised an issue but found this thread about finalName being ignore in 2.1.4.RELEASE. In the meantime we have switched the renaming of the artifact to the maven-assembly-plugin. We were unsure why explicitely defining finalName e.g. for multiple executions was removed in 2.1.0 just using the finalName of the build tag.

@snicoll
Copy link
Member

snicoll commented Apr 26, 2019

We were unsure why explicitely defining finalName e.g. for multiple executions was removed in 2.1.0 just using the finalName of the build tag.

For consistency with other core Maven plugins that used to have this property and made them read-only (with the side effects described above), #12608.

@snicoll
Copy link
Member

snicoll commented Apr 27, 2019

I had a chat with @rfscholte about this one and the interesting bit is that it's not supposed to work this way in the maven-jar-plugin either. The intention always was that build/finalName is immutable (as the Maven model should be): a plugin execution should not change the finalName value as we'd have to run that plugin again to be able package the artifact.

It "works" in the jar plugin for the same reason that it used to work in our plugin: there is a finalName property that is read-only with a default value that is lazily resolved (rather than asking the model).

If the intention in Maven is to invalidate that mechanism in the future, we need to figure out what we should do short term for ours. Removing the field means that resolution does not happen and we're calling the maven model (as we should be according to Maven best practices).

@snicoll
Copy link
Member

snicoll commented May 10, 2019

I've created an issue in the maven-jar-plugin issue tracker to see what the general direction will be.

@TheSnoozer
Copy link
Contributor

This issue also appears to be reported in git-commit-id/git-commit-id-maven-plugin#418.

From my understanding this issue which seems to be similar is a problem with spring-boot and some weird maven stuff where final name is assumed immutable, but in fact it happens that some readonly annotation is ignored and thus happens to work in previous versions.

Feel free to ping me if you have any questions - for me it seems just another "funny" maven rabbit hole bug :-)

@snicoll
Copy link
Member

snicoll commented May 10, 2019

Thanks. What you’ve described is already described and discussed in this issue and the maven jar plugin issue I’ve created.

@snicoll snicoll removed the for: team-attention An issue we'd like other members of the team to review label May 13, 2019
@snicoll snicoll self-assigned this May 13, 2019
@snicoll
Copy link
Member

snicoll commented May 13, 2019

Given the state of affairs, I've decided to revert the fix of #16202 which will bring us in par with what the maven-jar-plugin does. This should restore the previous situation, even if the Maven team does not recommend using variables in finalName and will remove such support in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: regression A regression from a previous release
Projects
None yet
Development

No branches or pull requests

6 participants