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

4.x: Pico maven plugin combines its own classpath with application #7083

Open
tomas-langer opened this issue Jun 23, 2023 · 2 comments · May be fixed by #8400
Open

4.x: Pico maven plugin combines its own classpath with application #7083

tomas-langer opened this issue Jun 23, 2023 · 2 comments · May be fixed by #8400
Assignees
Labels
4.x Version 4.x bug Something isn't working P2 pico Helidon Injection
Projects
Milestone

Comments

@tomas-langer
Copy link
Member

As the plugin uses service registry and adds helidon-config to the classpath, the registry discovers service that is provided by config.
This may create invalid descriptors for processed applications, as these may not depend on helidon-config at all.
The maven plugin should only use classpath of the application (and not its own classpath) to discover "stuff".
I have worked around this by adding appropriate dependencies to the libraries impacted (oci integration test, pico resource test), and by modified tests in pico resources that expect a certain list of initialized services, but now get more

@tomas-langer tomas-langer added bug Something isn't working pico Helidon Injection labels Jun 23, 2023
@github-actions github-actions bot added this to Triage in Backlog Jun 23, 2023
@m0mus m0mus moved this from Triage to High priority in Backlog Jun 26, 2023
@m0mus m0mus added P2 4.x Version 4.x labels Jun 26, 2023
@trentjeff trentjeff added this to the 4.0.0-M2 milestone Jul 6, 2023
@trentjeff trentjeff moved this from High priority to In Progress in Backlog Jul 6, 2023
@trentjeff
Copy link
Member

This might be harder than it looks, and will likely get us into the "classloader magic" realm.

At first glance, I tried the obvious:

//        ClassLoader prev = Thread.currentThread().getContextClassLoader();
        ClassLoader prev = ClassLoader.getSystemClassLoader();
        URLClassLoader loader = ExecutableClassLoader.create(classpath, prev);

but this yielded type loader conflicts from config as shown:

[ERROR] Pico Maven plugin execution failed
java.util.ServiceConfigurationError: io.helidon.config.spi.ConfigParser: io.helidon.config.PropertiesConfigParser not a subtype
    at java.util.ServiceLoader.fail (ServiceLoader.java:593)
    at java.util.ServiceLoader$LazyClassPathLookupIterator.hasNextService (ServiceLoader.java:1244)
    at java.util.ServiceLoader$LazyClassPathLookupIterator.hasNext (ServiceLoader.java:1273)
    at java.util.ServiceLoader$2.hasNext (ServiceLoader.java:1309)
    at java.util.ServiceLoader$3.hasNext (ServiceLoader.java:1393)
    at java.lang.Iterable.forEach (Iterable.java:74)
    at io.helidon.common.HelidonServiceLoader$Builder.build (HelidonServiceLoader.java:175)
    at io.helidon.config.BuilderImpl.loadParserServices (BuilderImpl.java:525)
    at io.helidon.config.BuilderImpl.buildParsers (BuilderImpl.java:477)
    at io.helidon.config.BuilderImpl.build (BuilderImpl.java:332)
    at io.helidon.config.BuilderImpl.build (BuilderImpl.java:52)
    at io.helidon.pico.maven.plugin.MavenPluginUtils.basicConfig (MavenPluginUtils.java:122)
    at io.helidon.pico.maven.plugin.MavenPluginUtils.picoServices (MavenPluginUtils.java:52)
    at io.helidon.pico.maven.plugin.AbstractApplicationCreatorMojo.innerExecute (AbstractApplicationCreatorMojo.java:252)
    at io.helidon.pico.maven.plugin.AbstractCreatorMojo.execute (AbstractCreatorMojo.java:247)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:137)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:301)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:211)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:165)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:157)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:121)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:127)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:294)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:960)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:293)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:196)
    at jdk.internal.reflect.DirectMethodHandleAccessor.invoke (DirectMethodHandleAccessor.java:104)
    at java.lang.reflect.Method.invoke (Method.java:578)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)

It seems clear that both the maven-plugin and the application being processed will require a common set type types (i.e., from config, pico, etc.). I think the better approach to this is to continue the precedent I've already established in the code with having service type exceptions. Currently the list is empty, but we could always use this to carry (internal) service types for exclusion should this ever become an issue.

In AbstractApplicationCreatorMojo

        // we MUST get the exclusion list prior to building the next loader, since it will reset the service registry
        Set<TypeName> serviceNamesForExclusion = getServiceTypeNamesForExclusion();

@trentjeff trentjeff moved this from In Progress to Sprint Scope in Backlog Jul 6, 2023
@tomas-langer
Copy link
Member Author

Is there maybe a way to find out that the discovered service is already handled by another module that is not the currently processed module (that would be sufficient, and would resolve all such cases).
Otherwise we can add the exclusion to the plugin (explicitly excluding the Config service, and any other we may add to Helidon modules that the maven plugin uses)

@m0mus m0mus modified the milestones: 4.0.0-M2, 4.x Jul 24, 2023
@tomas-langer tomas-langer linked a pull request Mar 6, 2024 that will close this issue
@tomas-langer tomas-langer moved this from Sprint Scope to In Progress in Backlog Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x bug Something isn't working P2 pico Helidon Injection
Projects
Backlog
  
In Progress
Development

Successfully merging a pull request may close this issue.

3 participants