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

[WFCORE-4266] Support MR JAR resource loading from VFSResourceLoader #3636

Merged
merged 1 commit into from Jan 10, 2019

Conversation

dmlloyd
Copy link
Member

@dmlloyd dmlloyd commented Jan 8, 2019

@dmlloyd
Copy link
Member Author

dmlloyd commented Jan 8, 2019

This class does not have unit tests. We might want to have a test plan for this class before merging.

@dmlloyd dmlloyd force-pushed the wfcore-4266 branch 2 times, most recently from 1d8e4da to f1ddea9 Compare January 8, 2019 15:13
@dmlloyd
Copy link
Member Author

dmlloyd commented Jan 8, 2019

I've also pushed up an update where it will only seek out versioned entries if the Multi-Release JAR attribute is present. Note that the spec says that its value doesn't matter. https://github.com/wildfly/wildfly-core/compare/1d8e4da85256d543e42d2ea7b1a7b126f38c2320..f1ddea9

@dmlloyd
Copy link
Member Author

dmlloyd commented Jan 8, 2019

@wildfly-ci

This comment has been minimized.

@dmlloyd
Copy link
Member Author

dmlloyd commented Jan 8, 2019

I do believe that CI is lying about what version it ran with.

@dmlloyd
Copy link
Member Author

dmlloyd commented Jan 8, 2019

Retest this please

@bstansberry
Copy link
Contributor

retest this please

@dmlloyd
Copy link
Member Author

dmlloyd commented Jan 9, 2019

Copy link

@hd42 hd42 left a comment

Choose a reason for hiding this comment

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

With the latest changes, a MRJAR is read correctly from a WAR.

@jmesnil jmesnil added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Jan 10, 2019
@jmesnil jmesnil merged commit 5012d46 into wildfly:master Jan 10, 2019
@dmlloyd dmlloyd deleted the wfcore-4266 branch January 10, 2019 14:01
@@ -123,18 +141,41 @@ public Manifest run() throws IOException {
throw new UndeclaredThrowableException(e);
}
}
// "A multi-release jar file is a jar file that contains a manifest with a main attribute named "Multi-Release", [...]"
multiRelease = manifest != null && manifest.getMainAttributes().containsKey(MULTI_RELEASE_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @dmlloyd, apparently the value of that attribute must be true for the multi-release jar to be considered enabled https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-January/058131.html

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the note. Could you please open a JIRA (it can have "minor" priority)? And a PR if you want to. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks Jaikiran!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
6 participants