New feature: Determine agent class from manifest

Now, config property 'agentClass' is optional and should rarely be
necessary, e.g. in case of invalid agent JARs without manifests or with
missing 'Agent-Class' manifest entries.

IT SpringBootAspectJ tests both cases.

src/it/RemoveFinalAgent/src/main/java/dev/aspectj/agent/NonManifestRemoveFinalAgent.java

@@ -0,0 +1,22 @@
+package dev.aspectj.agent;
+
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.ClassWriter;
+
+import java.lang.instrument.ClassFileTransformer;
+import java.lang.instrument.IllegalClassFormatException;
+import java.lang.instrument.Instrumentation;
+import java.security.ProtectionDomain;
+
+import static org.objectweb.asm.ClassReader.SKIP_DEBUG;
+import static org.objectweb.asm.ClassReader.SKIP_FRAMES;
+
+/**
+ * Alternate agent class which is not configured in the manifest, bug can be reached by setting the embedder plugin's
+ * {@code javaAgents/agent/agentClass} configuration property
+ */
+public class NonManifestRemoveFinalAgent {
+  public static void premain(String targetClasses, Instrumentation instrumentation) {
+    RemoveFinalAgent.transform(targetClasses, instrumentation);
+  }
+}

src/it/RemoveFinalAgent/src/main/java/dev/aspectj/agent/RemoveFinalAgent.java

@@ -16,7 +16,7 @@ public static void premain(String targetClasses, Instrumentation instrumentation
     transform(targetClasses, instrumentation);
   }
 
-  private static void transform(String targetClasses, Instrumentation instrumentation) {
+  static void transform(String targetClasses, Instrumentation instrumentation) {
     //noinspection Convert2Lambda
     instrumentation.addTransformer(
       new ClassFileTransformer() {

src/it/SpringBootAspectJ/pom.xml

@@ -44,12 +44,14 @@
                 <agent>
                   <groupId>org.aspectj</groupId>
                   <artifactId>aspectjweaver</artifactId>
-                  <agentClass>org.aspectj.weaver.loadtime.Agent</agentClass>
+                  <!-- Optional parameter, let the plugin extract the correct value from the agent manifest -->
+                  <!--<agentClass>org.aspectj.weaver.loadtime.Agent</agentClass>-->
                 </agent>
                 <agent>
                   <groupId>dev.aspectj</groupId>
                   <artifactId>remove-final-agent</artifactId>
-                  <agentClass>dev.aspectj.agent.RemoveFinalAgent</agentClass>
+                  <!-- Optional parameter, overriding agent manifest value -->
+                  <agentClass>dev.aspectj.agent.NonManifestRemoveFinalAgent</agentClass>
                   <agentArgs>dev.aspectj.FirstComponent,dev.aspectj.SecondComponent</agentArgs>
                 </agent>
               </javaAgents>

src/it/SpringBootAspectJ/verify.groovy

@@ -39,6 +39,12 @@ assert removeEmbeddedJarLines.grep(~/.*BOOT-INF\/lib\/remove-final-agent-.*\.jar
 // Inspect Spring Boot runtime output
 // --------------------------------------------------------
 
+// AspectJ weaver is started via 'Agent-Class' manifest attribute auto-detection
+assert logLines.contains('Starting agent org.aspectj.weaver.loadtime.Agent with arguments null')
+
+// Remove final agent is started via 'agentClass' config property
+assert logLines.contains('Starting agent dev.aspectj.agent.NonManifestRemoveFinalAgent with arguments dev.aspectj.FirstComponent,dev.aspectj.SecondComponent')
+
 // AspectJ load-time weaving (LTW) happens
 List<String> weaveInfoLines = logLines.grep(~/.*weaveinfo.*/)
 assert weaveInfoLines.size() == 4
@@ -47,7 +53,7 @@ assert weaveInfoLines.grep(~/.*field-set\(java.lang.Double dev.aspectj.FirstComp
 assert weaveInfoLines.grep(~/.*field-set\(java.lang.Integer dev.aspectj.SecondComponent.field2\).*/).size() == 1
 assert weaveInfoLines.grep(~/.*field-set\(boolean dev.aspectj.SecondComponent.field3\).*/).size() == 1
 
-// Remove final agent is active
+// Remove final agent does its job, removing 'final' modifiers from specified target classes
 assert logLines.grep(~/.*Remove final agent seems to be inactive.*/).size() == 0
 assert logLines.contains('[Remove Final Agent] Removing final from class dev.aspectj.FirstComponent')
 assert logLines.contains('[Remove Final Agent] Removing final from class dev.aspectj.SecondComponent')

src/main/java/dev/aspectj/maven/agent_embedder/AgentEmbedderMojo.java

@@ -91,6 +91,7 @@
 )
 public class AgentEmbedderMojo extends AbstractMojo {
   public static final String MANIFEST_PATH = "META-INF/MANIFEST.MF";
+  public static final String HEADER_AGENT_CLASS = "Agent-Class";
 
   /**
    * Host file system the mojo works on. Override for testing.
@@ -110,10 +111,10 @@ public class AgentEmbedderMojo extends AbstractMojo {
    *     {@code agentPath} for a possible workaround.
    *   </li>
    *   <li>
-   *     {@code agentClass}: The agent class containing its {@code premain} method. A future version of this plugin
-   *     might be able to read the value directly from the agent's manifest, but currently you need to specify it
-   *     explicitly. Just inspect the agent JAR's <i>META-INF/MANIFEST.MF</i> file and copy the fully qualified class
-   *     name from its {@code Premain-Class} attribute.
+   *     {@code agentClass}: The agent class containing a {@code premain} launcher method. This property is optional,
+   *     because by default the plugin extracts the value from the agent JAR's {@code Agent-Class} manifest attribute.
+   *     In the rare occasion that you wish to override the manifest value (e.g., the manifest does not exist or does
+   *     not contain the correct value), set this property.
    *   </li>
    *   <li>
    *     {@code agentArgs}: An optional argument string for the java agent. When using {@code -javaagent} on the JVM
@@ -182,8 +183,8 @@ public class AgentEmbedderMojo extends AbstractMojo {
    * of their classpath dependencies in folder <i>BOOT-INF/lib</i>, from where they are loaded using a special
    * classloader that can read nested JARs. Agent JARs defined as dependencies, e.g. <i>aspectjweaver-x.y.z.jar</i>, can
    * also be found there, if the user did not exclude them during the build. After having expanded an agent JAR into the
-   * containing JAR's root folder, the classes exist twice in the same JAR - unpacked and as a nested JAR. This is
-   * not necessarily a big problem, but bloats the JAR.
+   * containing JAR's root folder, the classes exist twice in the same JAR - unpacked and as a nested JAR. This is not
+   * necessarily a big problem, but bloats the JAR.
    * <p>
    * This option, if active, makes the plugin search for nested JARs matching the names of artifacts described by
    * {@code javaAgents}. For each agent, the first nested JAR found is deleted.
@@ -212,7 +213,6 @@ public void execute() throws MojoExecutionException {
     try (FileSystem jarFS = getZipFS(artifactPath, false)) {
       if (jarFS == null)
         throw new MojoExecutionException("Cannot open artifact JAR file");
-      new ManifestUpdater(jarFS).update();
       embedLauncherAgent(jarFS);
       getLog().info("Embedding java agents");
       for (JavaAgentInfo agent : javaAgents) {
@@ -225,8 +225,9 @@ public void execute() throws MojoExecutionException {
         if (agentJarLocation == null)
           throw new MojoExecutionException("Java agent JAR for " + agent + " not found");
         getLog().info("Processing java agent " + agentJarLocation);
-        unpackAgentJar(jarFS, agentJarLocation);
+        unpackAgentJar(agent, jarFS, agentJarLocation);
       }
+      new ManifestUpdater(jarFS).update();
     }
     catch (IOException | NoExecutableJarException e) {
       throw new MojoExecutionException("Error while embedding java agents", e);
@@ -272,7 +273,7 @@ protected String adjustPathSeparatorToHostFS(String path, FileSystem targetFS) {
     return path.replace(fromSeparator, toSeparator);
   }
 
-  protected void unpackAgentJar(FileSystem jarFS, String agentPath) throws IOException, MojoExecutionException {
+  protected void unpackAgentJar(JavaAgentInfo agentInfo, FileSystem jarFS, String agentPath) throws IOException, MojoExecutionException {
     Path agentJarPath = hostFS.getPath(agentPath);
     final boolean externalJarFound = Files.exists(agentJarPath);
     Path embeddedAgentJarPath = null;
@@ -300,6 +301,9 @@ protected void unpackAgentJar(FileSystem jarFS, String agentPath) throws IOExcep
 
     //noinspection RedundantCast: newFileSystem is overloaded in more recent JDKs
     try (FileSystem javaAgentFS = getZipFS(agentJarPath, false)) {
+      Objects.requireNonNull(javaAgentFS);
+      configureJavaAgentClass(agentInfo, agentJarPath, javaAgentFS);
+
       try (
         Stream<Path> files = Files.find(
           javaAgentFS.getPath("/"), Integer.MAX_VALUE,
@@ -332,6 +336,49 @@ protected void unpackAgentJar(FileSystem jarFS, String agentPath) throws IOExcep
     }
   }
 
+  /**
+   * Determine and configure the java agent class for a given agent.
+   * <p>
+   * By default, read it from the agent info given in the plugin configuration. If unset, try to extract it from the
+   * agent JAR's manifest, reading its {@code Agent-Class} attribute. If neither of the two is specified, throw a
+   * {@link MojoExecutionException}.
+   * <p>
+   * Note: The method does not check if the determined class actually exists in the agent JAR.
+   */
+  protected void configureJavaAgentClass(JavaAgentInfo agentInfo, Path agentJarPath, FileSystem javaAgentFS)
+    throws IOException, MojoExecutionException
+  {
+    getLog().debug("Configuring java agent class for " + agentInfo);
+
+    getLog().debug("Reading agent manifest from path " + agentJarPath);
+    Manifest javaAgentManifest = new Manifest(Files.newInputStream(javaAgentFS.getPath("/" + MANIFEST_PATH)));
+
+    String manifestAgentClass = javaAgentManifest.getMainAttributes().getValue(HEADER_AGENT_CLASS);
+    manifestAgentClass = manifestAgentClass == null ? "" : manifestAgentClass.trim();
+    getLog().debug("Agent class from manifest: " + manifestAgentClass);
+
+    agentInfo.setAgentClass(agentInfo.getAgentClass() == null ? "" : agentInfo.getAgentClass().trim());
+    getLog().debug("Agent class from plugin configuration: " + agentInfo.getAgentClass());
+
+    if (manifestAgentClass.isEmpty()) {
+      if (agentInfo.getAgentClass().isEmpty())
+        throw new MojoExecutionException(
+          "Agent class for " + agentInfo + " neither configured nor found in agent manifest"
+        );
+      getLog().warn(
+        "Agent class name not found in agent manifest, using configured value '" +
+          agentInfo.getAgentClass() + "'. Attention: JAR does not seem to be a regular java agent."
+      );
+    }
+    else if (agentInfo.getAgentClass().isEmpty()) {
+      getLog().debug("Using agent class '" + manifestAgentClass + "' found in manifest");
+      agentInfo.setAgentClass(manifestAgentClass);
+    }
+    else {
+      getLog().debug("Using agent class '" + agentInfo.getAgentClass() + "' found in plugin configuration");
+    }
+  }
+
   public class ManifestUpdater {
     public static final String MANIFEST_HEADER_MAIN_CLASS = "Main-Class";
     public static final String MANIFEST_HEADER_LAUNCHER_AGENT = "Launcher-Agent-Class";

src/main/java/dev/aspectj/maven/agent_embedder/JavaAgentInfo.java

@@ -2,6 +2,8 @@
 
 import org.apache.maven.artifact.Artifact;
 
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
 import java.util.Objects;
 
 public class JavaAgentInfo {
@@ -44,6 +46,17 @@ public String getAgentClass() {
     return agentClass;
   }
 
+  /**
+   * In contrast to other agent info properties, the agent class needs to be updateable, because the property is
+   * optional and can be determined by scanning the agent's manifest, extracting its {@code Agent-Class} attribute in
+   * the process. See {@link AgentEmbedderMojo#configureJavaAgentClass(JavaAgentInfo, Path, FileSystem)}.
+   *
+   * @param agentClass java agent main class
+   */
+  public void setAgentClass(String agentClass) {
+    this.agentClass = agentClass;
+  }
+
   public String getAgentArgs() {
     return agentArgs;
   }

src/test/groovy/dev/aspectj/maven/agent_embedder/AgentEmbedderMojoTest.groovy

@@ -72,8 +72,8 @@ class AgentEmbedderMojoTest extends Specification {
 
     when:
     try (FileSystem targetJarFS = fsTool.getTargetJarFS(false)) {
-      mojo.unpackAgentJar(targetJarFS, fsTool.agentJarLocation1)
-      mojo.unpackAgentJar(targetJarFS, fsTool.agentJarLocation2)
+      mojo.unpackAgentJar(mojo.javaAgents[0], targetJarFS, fsTool.agentJarLocation1)
+      mojo.unpackAgentJar(mojo.javaAgents[1], targetJarFS, fsTool.agentJarLocation2)
     }
     // Refresh meta data after FS operation
     targetFSInfo = fsTool.targetFSInfo