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

Respect order of META-INF/ and META-INF/MANIFEST.MF entries in a JAR file #189

Merged
merged 1 commit into from Dec 30, 2021

Conversation

michael-o
Copy link
Member

The insertion order did not take into account that META-INF/ and/or
META-INF/MANIFEST.MF can or should come first. The code has added it
META-INF/ to directories, thus the actual order of a JAR file was:

META-INF/MANIFEST.MF
META-INF/
META-INF/maven

which does not make sense.

@michael-o
Copy link
Member Author

@hboutemy hboutemy changed the title Respect the order entries in a JAR file Respect order of META-INF/ and META-INF/MANIFEST.MF entries in a JAR file Dec 19, 2021
@hboutemy
Copy link
Member

we need a MJAR issue, because the issue in MNG is triggered by MJAR, with its root cause here in plexus-archiver...

I don't know if we can add a unit test

@hboutemy
Copy link
Member

@michael-o
Copy link
Member Author

I think this issue is everywhere, WAR, shade, assembly, compiler, javadoc, sources and maybe others.

@michael-o
Copy link
Member Author

I think a test is possible since you can use the commons-compress to open a JAR and check for order. I wouldn't be able to create the test next week. Maybe end of. Maybe someone else can provide it?

@plamentotev
Copy link
Member

There is existing test that verifies whether the archive contains META-INF/MANIFEST.MF. I've modified it to verify that the order is respected. @michael-o feel free to squash it in single commitif you think it makes more sense.

The insertion order did not take into account that META-INF/ and/or
META-INF/MANIFEST.MF can or should come first. The code has added it
META-INF/ to directories, thus the actual order of a JAR file was:

META-INF/MANIFEST.MF
META-INF/
META-INF/maven

which does not make sense.

Test supplied by: Plamen Totev <plamen.iv.totev@gmail.com>

This closes #189
@michael-o michael-o closed this in 1d2b764 Dec 30, 2021
@michael-o michael-o merged commit 1d2b764 into master Dec 30, 2021
@michael-o michael-o deleted the respect-jar-entries-order branch December 30, 2021 19:51
@michael-o
Copy link
Member Author

There is existing test that verifies whether the archive contains META-INF/MANIFEST.MF. I've modified it to verify that the order is respected. @michael-o feel free to squash it in single commitif you think it makes more sense.

Thank you, the test is simple and does exactly what we need.

@jorsol
Copy link
Contributor

jorsol commented Jan 3, 2022

Related: Zlika/reproducible-build-maven-plugin#16

https://github.com/openjdk/jdk/blob/03a8d342b86e720d3cba08d540182b4ab161fba3/src/java.base/share/classes/java/util/jar/JarInputStream.java#L78-L81

The jar tool also adds module-info.class as the third entry but I guess plexus-archiver is not affected since the JarToolModularJarArchiver class calls the jar tool directly.

@michael-o
Copy link
Member Author

It is unfortunate that Oracle failed to officially specify that.

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.

None yet

4 participants