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

PAYARA-1161 added ability to customise contents of Micro uberjar #1569

Merged
merged 1 commit into from
May 10, 2017

Conversation

Cousjava
Copy link
Contributor

@Cousjava Cousjava commented May 2, 2017

No description provided.

@Cousjava
Copy link
Contributor Author

Cousjava commented May 2, 2017

jenkins test please

@payara-ci
Copy link
Contributor

One or more tests have failed

@Cousjava
Copy link
Contributor Author

Cousjava commented May 2, 2017

jenkins test please

@payara-ci
Copy link
Contributor

All tests have passed


JarEntry deploymentEntry = new JarEntry(copyDirectory + file.getPath().replace(basePath, ""));
jos.putNextEntry(deploymentEntry);
Files.copy(file, jos);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that it's not enough that JAR files are copied as is - due to how Payara Micro works since version 171, all loadable JAR files should be uncompressed. Is that right, @smillidge, @Pandrex247 ?
Otherwise an attempt to load compressed JARs (which is almost every 3rd party distributed JAR) would lead to ZIP exceptions like the one reported here: https://groups.google.com/d/msgid/payara-forum/0f0e28fb-0645-4992-bdf0-6201d09d3df0%40googlegroups.com

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would only be necessary if the JARs are to be placed into the MICRO-INF/lib folder. But it wouldn't hurt to convert all JARs to uncompressed JARs, because it would be very rare that users would want to place JARs somewhere else.

Copy link
Contributor

@smillidge smillidge May 6, 2017

Choose a reason for hiding this comment

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

the 3rd party jar does not need to be uncompressed but the zip entry storing it must be if placed in MICRO-INF/runtime or MICRO-INF/lib. However this is only a requirement if running with the nested jar classloader which be default Payara Micro doesn't

See https://github.com/payara/Payara/blob/master/appserver/extras/payara-micro/payara-micro-core/src/main/java/fish/payara/micro/impl/UberJarCreator.java#L191 it's an easy change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the difference between this and deploymentDir (line 242)?

Copy link
Contributor

Choose a reason for hiding this comment

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

@smillidge it's an easy change
I think that the code you linked works only if the original entry is uncompressed, otherwise size != compressed size, throwing an exception as reported in the mailing list. The fix needs to uncompress JAR files if the method is DEFLATED and store them uncompressed, it's not enough to just set the method to STORE.

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm true. I assume the person who asked the question had manually added the entry via zip or something.

Copy link
Contributor

@smillidge smillidge May 8, 2017

Choose a reason for hiding this comment

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

Deployment dir doesn't matter as it is not loaded via the nested classloader. Having the entry in STORED is only for the nested classloader which isn't the default anyway.

@Cousjava
Copy link
Contributor Author

Cousjava commented May 8, 2017

jenkins test please

@Cousjava Cousjava force-pushed the PAYARA-1161-customise-uberjar branch from 97cdd25 to ee429e2 Compare May 8, 2017 15:06
@payara-ci
Copy link
Contributor

All tests have passed

@smillidge smillidge merged commit e615d29 into payara:master May 10, 2017
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.

5 participants