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

SHRINKWRAP-505 - Allow addPackage() to be smarter about .war files #99

Closed
wants to merge 1 commit into from

Conversation

bobmcwhirter
Copy link
Contributor

This commit brings about the smartness required for URLPackageScanner
to understand that a URL that represents a .war has its classes
relocated under WEB-INF/lib instead of from the root of the archive.

Part of the change is pre-constructing the Asset passed to the
scanner's Callback, taking into account the location of the underlying
asset, separate from it's actual related class name.

This commit brings about the smartness required for URLPackageScanner
to understand that a URL that represents a .war has its classes
relocated under WEB-INF/lib instead of from the root of the archive.

Part of the change is pre-constructing the Asset passed to the
scanner's Callback, taking into account the location of the underlying
asset, separate from it's actual related class name.
* @return new instance of URLPackageScanner
*/
public static URLPackageScanner newInstance(boolean addRecursively, ClassLoader classLoader, Callback callback) {
public static URLPackageScanner newInstance(boolean addRecursively, ClassLoader classLoader, Callback callback) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

throws IOE, but we catch it and swallow? Need to throw here in the method declar? Would prefer not if we're just going to swallow it or wrap in RuntimeException.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. Can remove from signature. Will do tomorrow. I realized we can swallow and not propagate but forgot to remove from the method.

Sent from my iPhone

On May 31, 2016, at 5:29 PM, Andrew Lee Rubinger notifications@github.com wrote:

In impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/URLPackageScanner.java:

  * @return new instance of URLPackageScanner
  */
  • public static URLPackageScanner newInstance(boolean addRecursively, ClassLoader classLoader, Callback callback) {
  • public static URLPackageScanner newInstance(boolean addRecursively, ClassLoader classLoader, Callback callback) throws IOException {
    throws IOE, but we catch it and swallow? Need to throw here in the method declar? Would prefer not if we're just going to swallow it or wrap in RuntimeException.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@bobmcwhirter
Copy link
Contributor Author

bobmcwhirter commented Jun 1, 2016

@ALRubinger -- So reasoning for the change...

People can have a war project in maven, but with WildFly Swarm still provide a main(). In which case, they also want to programaticly assemble an archive for deployment into Swarm.

We do magic so that the current classloader running main() can see everything in the .war. We actually load it into the current classpath.

But, if subArchive.addPackage() is used, it inspects things with purely cl.getResources( "com/mycorp/foo" ), which will fail, because the thing being asked for is actually findable as "WEB-INF/classes/com/mycorp/foo".

You might think we could then addPackage( "WEB-INF.classes.org.mycorp.foo" ), but then you'd also notice it'd try to create a class named WEB-INF.classes.org.mycorp.foo.Whatever, which is clearly wrong.

Hence the tracking if the 'prefix' in the scanner, to help determine where to search, and what to remove from the class name ("WEB-INF/classes/").

I had to amend the Callback because simply knowing class org.mycorp.foo.Whatever was found is no longer sufficient, because nobody can know where it was actually found. Hence the pushing back of the asset creation to the URLScanner, which is the only thing that knows where it came from (/org/mycorp/foo/Whatever vs /WEB-INF/classes/org/mycorp/foo/Whatever) in addition to its actual name (org.mycorp.foo.Whatever).

The pattern we actually see, in WildFly Swarm, is

archive.addPackage( Main.class.getPackage() )

and I'd hoped the Package object passed in would have some connection to Main.class's classloader, but that appears not to be the case. Within SW, everything becomes a String before any real work is accomplished.

@ALRubinger
Copy link
Member

d0df4ba
0e75f35
11f99f5

@ALRubinger ALRubinger closed this Jun 2, 2016
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