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

Make Classworld setup more alike to vanilla Maven #784

Merged
merged 1 commit into from
Feb 14, 2023

Conversation

oehme
Copy link
Contributor

@oehme oehme commented Jan 30, 2023

Use the plexus Launcher to start the daemon server, just like we would
launch a normal Maven process.

This improves compatbility with any extensions or plugins that assume that
their ClassLoader is a ClassRealm.

@oehme
Copy link
Contributor Author

oehme commented Jan 30, 2023

FYI the builds are failing since bf892cd - Seems those artifacts aren't there yet. I'll rebase this branch on an earlier commit to get some CI feedback in the meantime.

@oehme oehme force-pushed the oehme/maven-like-classworld branch from b5be6d5 to 364ef8f Compare January 31, 2023 09:46
@oehme oehme marked this pull request as draft January 31, 2023 11:38
@oehme oehme force-pushed the oehme/maven-like-classworld branch from 364ef8f to 5b45eee Compare February 6, 2023 12:01
@@ -473,6 +472,7 @@ DefaultPlexusContainer container() throws Exception {

List<File> extClassPath = Stream.of(
Environment.MVND_EXT_CLASSPATH.asString().split(","))
.filter(s -> s != null && !s.isEmpty())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The missing empty filter here lead to the empty String being added to the extClassPath list, which made the classloader structure slightly different from vanilla Maven in some scenarios. Not a big deal in practice, but it broke some of our tests, so I decided to fix it for consistency :)

@oehme oehme requested a review from gnodet February 6, 2023 12:03
@oehme oehme marked this pull request as ready for review February 6, 2023 12:03
@oehme oehme force-pushed the oehme/maven-like-classworld branch from 5b45eee to 1678fcc Compare February 6, 2023 12:04
Use the plexus Launcher to start the daemon server, just like we would
launch a normal Maven process.

This improves compatbility with any extensions or plugins that assume that
their ClassLoader is a ClassRealm.
@oehme oehme force-pushed the oehme/maven-like-classworld branch from 1678fcc to a4a6f81 Compare February 6, 2023 12:57
@oehme
Copy link
Contributor Author

oehme commented Feb 8, 2023

@gnodet this is ready for another look

@gnodet gnodet merged commit 1bcfc29 into apache:master Feb 14, 2023
@lppedd
Copy link

lppedd commented Feb 25, 2023

@oehme hi! Just passing by and wondering, in which case a plugin/extension would want to explicitly check for a class loader type? Did you think about this because of some real world use case?

@oehme
Copy link
Contributor Author

oehme commented Feb 26, 2023

Yes, the Gradle Enterprise Maven extension does some "importRealm" magic, so that other extensions (written by customers) can access its API no matter how they were registered (ext/lib, maven.ext.classpath, .mvn/extensions.xml)

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.

3 participants