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

add tests to show Manifest can't be read from build/dist/plugin.jar #15

Merged

Conversation

Gubaer
Copy link
Contributor

@Gubaer Gubaer commented Jun 1, 2022

  • Manifest can be read using JarFile::getManifest()
  • Manifest can't be read using JarInputStream::getManifest() (the way JOSM does it)

Possible reason:

  • MANIFEST entry is at the end of the jar file
  • JarInputStream expects it at the beginning

Copy link
Owner

@floscher floscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gubaer Thank you very much for bringing this to my attention!

It probably has to do with the changes to how the MANIFEST.MF file is generated.

Java generates the manifest files as invalid UTF-8 files (see gradle/gradle#5225). That's why I wrote a custom implementation for creating the manifest files. Maybe the JarInputStream somewhere relies on the files being broken 🤔.

Edit: It looks like your explanation is probably the correct one.

I'll definitively have a look and fix that.

Manifest can be read using JarFile::getManifest()
Manifest can't be read using JarInputStream::getManifest() (the way JOSM does it)

Possible reason: MANIFEST entry is at the end of the jar file, JarInputStream
expects it at the beginning
@floscher floscher force-pushed the bug-cant-read-manifest-using-jarinputstream branch from f93cf37 to b9f5fad Compare June 1, 2022 12:50
@floscher
Copy link
Owner

floscher commented Jun 1, 2022

@Gubaer This should be fixed now and will be released as a new version ASAP. Thanks!

@floscher floscher force-pushed the bug-cant-read-manifest-using-jarinputstream branch from b9f5fad to 5212794 Compare June 2, 2022 08:09
@floscher floscher merged commit 5212794 into floscher:main Jun 2, 2022
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

2 participants