Add maven support for agent

[dnestoro]

No description provided.

[alvarosanchez]

This is a good start, but it needs quite some more work. Specially, you need to write tests.

[alvarosanchez]

Besides the inline comments, documentation is missing

[dnestoro]

Style issues will be fixed once main logic gets approval.

[sdeleuze]

Please provide feedback on this first set of questions, thanks!

[sdeleuze]

I tested it with a simple Spring Boot generated on start.spring.io, the project only has the default:

<plugin>
  <groupId>org.graalvm.buildtools</groupId>
  <artifactId>native-maven-plugin</artifactId>
</plugin>

When running mvn clean package it throw the following exception:

[ERROR] Internal error: java.lang.NullPointerException: Cannot invoke "org.codehaus.plexus.util.xml.Xpp3Dom.getName()" because "root" is null -> [Help 1]
org.apache.maven.InternalErrorException: Internal error: java.lang.NullPointerException: Cannot invoke "org.codehaus.plexus.util.xml.Xpp3Dom.getName()" because "root" is null
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:120)
    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.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:77)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:568)
    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)
Caused by: java.lang.NullPointerException: Cannot invoke "org.codehaus.plexus.util.xml.Xpp3Dom.getName()" because "root" is null
    at org.graalvm.buildtools.utils.Xpp3DomParser.getTagByName (Xpp3DomParser.java:52)
    at org.graalvm.buildtools.utils.AgentUtils.collectAgentProperties (AgentUtils.java:123)
    at org.graalvm.buildtools.maven.NativeExtension.lambda$afterProjectsRead$5 (NativeExtension.java:130)
    at java.util.Optional.ifPresent (Optional.java:178)
    at org.graalvm.buildtools.maven.NativeExtension.withPlugin (NativeExtension.java:201)

I then checked the CI run of the latest commit, but looks like checkstyle did not pass:
https://github.com/graalvm/native-build-tools/actions/runs/3688052159/jobs/6242397017

Please fix those issues and ensure your changes are properly tested.

[sdeleuze]

When configuring:

<configuration>
	<agent>
		<enabled>true</enabled>
	</agent>
</configuration>

I see the following error:

GraalVM native-image is missing from your system.
 Make sure that GRAALVM_HOME environment variable is present.

Like the other parts of the plugin, I think it is important that it works without making GRAALVM_HOME mandatory, just with JAVA_HOME defined.

[dnestoro]

When configuring:

<configuration>
	<agent>
		<enabled>true</enabled>
	</agent>
</configuration>

I see the following error:

GraalVM native-image is missing from your system.
 Make sure that GRAALVM_HOME environment variable is present.

Like the other parts of the plugin, I think it is important that it works without making GRAALVM_HOME mandatory, just with JAVA_HOME defined.

I agree with this idea. The only thing I am thinking about is should I do that inside this PR or I should create separate PR. It looks like this is commonly used in many parts of the repo and not only related to the agent. What is your opinion?

[melix]

I should create separate PR. It looks like this is commonly used in many parts of the repo and not only related to the agent. What is your opinion?

Pro-tip: do not mix concerns in a single PR. Smaller, focused PRs are easier to review by peers and more likely to be merged quickly.

[sdeleuze]

If this is not a regression introduced by that PR, we could probably fix it in another PR but IMO we should do that before releasing NBT 0.9.20.

[dnestoro]

I tested plugin with netty project, and discovered two new edge cases. Last commits cover those cases.

[linghengqian]

I tested plugin with netty project, and discovered two new edge cases. Last commits cover those cases.

• Is there a draft PR showing on the netty/netty side? I'm curious when the version of NBT that includes this PR will be released.
• I did additional validation at For Maven Plugin 0.9.20, the native:metadata-copy task cannot be executed normally #403 .