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

Reinstate META-INF/maven/${groupId}/${artifactId}/pom.xml in published jar files #14672

Closed
dsyer opened this issue Oct 3, 2018 · 9 comments
Closed
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@dsyer
Copy link
Member

dsyer commented Oct 3, 2018

Somewhere between 2.0.0.M6 and 2.0.0.RELEASE the spring-boot jars stopped having their pom.xml included:

$ jar -tvf ~/.m2/repository/org/springframework/boot/spring-boot/2.0.0.RELEASE/spring-boot-2.0.0.RELEASE.jar | grep pom
$ jar -tvf ~/.m2/repository/org/springframework/boot/spring-boot/2.0.0.M6/spring-boot-2.0.0.M6.jar | grep pom
   104 Mon Nov 06 05:56:56 GMT 2017 META-INF/maven/org.springframework.boot/spring-boot/pom.properties
 13521 Mon Nov 06 05:55:14 GMT 2017 META-INF/maven/org.springframework.boot/spring-boot/pom.xml

Since including the pom.xml is a standard feature of the Maven JAR plugin, it seems like this might be unintended. Maybe something to do with the maven-flatten-plugin (but that first showed up in 2.0.0.M4)?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 3, 2018
@wilkinsona
Copy link
Member

The pom is there in RC2 as well so it seems to have disappeared due to something that changed between RC2 and RELEASE.

@dsyer
Copy link
Member Author

dsyer commented Oct 3, 2018

Looks like probably this:

<addDefaultImplementationEntries>false</addDefaultImplementationEntries>

@wilkinsona
Copy link
Member

I think that one affects the manifest. It's this one that's switching off the pom I think:

<addMavenDescriptor>false</addMavenDescriptor>

@dsyer
Copy link
Member Author

dsyer commented Oct 3, 2018

Yes. Part of this: #11994 (although it looks like maybe unrelated collateral damage to me).

@wilkinsona
Copy link
Member

Removing that line restores the pom and doesn't seem to make any difference to the manifest. This is 2.0.5.RELEASE:

Manifest-Version: 1.0
Implementation-Title: Spring Boot
Automatic-Module-Name: spring.boot
Implementation-Version: 2.0.5.RELEASE
Built-By: Spring
Created-By: Apache Maven 3.5.4
Build-Jdk: 1.8.0_141

And this is 2.0.x with that line removed:

Manifest-Version: 1.0
Implementation-Title: Spring Boot
Automatic-Module-Name: spring.boot
Implementation-Version: 2.0.6.BUILD-SNAPSHOT
Built-By: Spring
Created-By: Apache Maven 3.5.4
Build-Jdk: 1.8.0_181

@wilkinsona wilkinsona added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 3, 2018
@wilkinsona wilkinsona added this to the 2.0.x milestone Oct 3, 2018
@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Oct 3, 2018
@wilkinsona
Copy link
Member

I suspect it's collateral damage too. Flagging for team attention just in case I'm overlooking something.

@wilkinsona wilkinsona changed the title pom.xml missing from jar files Reinstate META-INF/maven/${groupId}/${artifactId}/pom.xml in published jar files Oct 3, 2018
@philwebb
Copy link
Member

philwebb commented Oct 3, 2018

I think I removed it because it seemed a little unnecessary and I wanted to align with what Framework does. Do we need them back for a specific reason?

@philwebb philwebb added status: on-hold We can't start working on this issue yet and removed for: team-attention An issue we'd like other members of the team to review labels Oct 3, 2018
@dsyer
Copy link
Member Author

dsyer commented Oct 3, 2018

Do we need them back for a specific reason?

I was just surprised not to see them since they are "standard". Also I assert their presence in tests in the thin launcher, but I can change that if I have to. I think it is good to have them, in case there are static analysis tools that want to know the transitive dependencies.

@philwebb
Copy link
Member

philwebb commented Oct 3, 2018

I think I'd rather keep them out to align with the jars published by Spring Framework. I guess the build system might play a big part in whether they are there or not. Maven probably adds them, Gradle probably doesn't.

Another reason not to add them back is because we flatten then and remove a few some bits. I've no idea if the flattened or orginal POM gets added, but its certainly easier our end to simply leave them out.

@philwebb philwebb closed this as completed Oct 3, 2018
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: on-hold We can't start working on this issue yet type: task A general task labels Oct 3, 2018
@philwebb philwebb removed this from the 2.0.x milestone Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

4 participants