Weaving of already compiled classes is skipped when there is no source code

plcarmel-aylo commented 1 week ago

The example given here doesn't work: https://dev-aspectj.github.io/aspectj-maven-plugin/examples/weaveDirectories.html

This is because already compiled classes will be overridden with the configuration they give.

In my use case, I need to compile classes, apply modifications to their bytecode using another plugin entirely, and then weave them. My modifications are lost because they are overridden with the weaving that comes from the source code.

The example here seems to offer a solution: https://dev-aspectj.github.io/aspectj-maven-plugin/examples/includeExclude.html

I tried using an empty source tag, <sources/>, but then the weaving is skipped entirely.

The only work-around that works for me is to exclude part of the source code, using <excludes>...</excludes> for example, while leaving at least one class to avoid the weaving being skipped entirely.

plcarmel-aylo commented 1 week ago

Using <forceAjcCompile>true</forceAjcCompile> solves my problem. Maybe it is just an issue with the documentation.

kriegaex commented 1 week ago

Without a minimal reproducer, it is next to impossible to help you. Can you debug prose? I cannot. If the compiler switch that seems to solve your problem is really the best or even a correct way to address it, I also cannot confirm. I suggest that either @plcarmel-aylo provides an MCVE, or otherwise the question be closed as invalid.

P.S.: You opened the question in this project, but pointed to the documentation of another fork. So, it is not even clear which of the two plugins you are using.

plcarmel-aylo commented 1 week ago

Hi @kriegaex, I use the plugin from the main project, not the one from the fork you created, the existence of which I was not even aware:

            <plugin>
                <groupId>org.codehaus.mojo</groupId>
                <artifactId>aspectj-maven-plugin</artifactId>
                <version>1.15.0</version>
            </plugin>

I understand there might be some drama going on between the two forks, but I am not interested into that. I am simply sharing the experience I have as a first-time user of the plugin.

Your documentation is the one people end-up on when they are using a search engine, and I thank you for providing it, it is very useful. However, I don't think it is reasonable to expect first-time users to understand that it comes from a different fork. Sure, now that I know what to look for, I see that the URL is from a different fork. However, I am just a dumb user trying to make my code work 🙂, that's not something I was looking for.

I will follow-up with an executable example. Thank you for your patience 🙏.

plcarmel-aylo commented 1 week ago

There you go: https://github.com/plcarmel-aylo/aspectj-nosource-mcve

kriegaex commented 1 week ago

the fork you created, the existence of which I was not even aware

Well, it is the Maven plugin recommended by the AspectJ project, because it has a few more more features than the MojoHaus version while being pretty much compatible to it.

I understand there might be some drama going on between the two forks

There was some in the past, but I would say that for the last few years the two projects live in peaceful coexistence, which is for example why I am also trying to answer questions here, even though it is not "my" fork.

My question about which fork you use was just in order to find out which of the two plugins you use, because even though what you want to do should work in both plugins and even in the same way, there is always a chance that you hit a problem which exists only in one of the two projects.

I will follow-up with an executable example

I am going to look into your sample project.

kriegaex commented 1 week ago

Your example does something other than the docs page explains. The result is to be expected like this. For comparison, the docs explain how within a single Maven module you can

compile Java code from source,
compile and weave aspects into the previously compiled Java classes.

Because functionally Ajc is a superset of and therefore basically a drop-in replacement of Javac, Ajc might also compile Java sources one more time, if not instructed not to do that. Normally, this is not a problem.

But what you want to do is, again in a single Maven module,

compile Java code from source,
instrument the compiled classes using another plugin (by running a class just compiled in the previous step - very hacky!),
compile and weave aspects into the previously compiled and instrumented Java classes.

While the first scenario is already something I would not recommend, following the "one module, one artifact" Maven mantra, the second is even more complex. Therefore, you need more complex configuration to achieve all of that in a single module. You found out one way to do that, there are others with slightly different plugin configs.

The cleanest way to achieve what you want is to create 3 modules A, B and C where A just contains and compiles the Java code, B instruments the code in the JAR created by A and C compiles and weaves the aspect into the JAR created by B. The artifact created by C would be what other modules would consume as a dependency, if necessary.

But if you insist to do it in this hacky one-module way and just to show you that many routes lead to Rome, how about

renaming the aspect source file to MessageToUppercase.aj (not .java), which automatically leads to Maven Compiler skipping it but AspectJ Maven still finding it,
refactoring the Maven Compiler options to use outputDirectory and proc instead of compilerArgs,
configuring AJ Maven to only compile the aspect, eliminating the need for forceAjcCompile?

  <build>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.13.0</version>
        <executions>
          <execution>
            <id>default-compile</id>
            <configuration>
              <outputDirectory>${project.build.directory}/unwoven-classes</outputDirectory>
              <proc>full</proc>
            </configuration>
          </execution>
        </executions>
      </plugin>

      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>exec-maven-plugin</artifactId>
        <version>3.5.0</version>
        <executions>
          <execution>
            <id>replace-message</id>
            <phase>compile</phase>
            <goals>
              <goal>java</goal>
            </goals>
            <configuration>
              <mainClass>org.example.ReplaceMessage</mainClass>
              <addOutputToClasspath>false</addOutputToClasspath>
              <additionalClasspathElements>
                <additionalClasspathElement>${project.build.directory}/unwoven-classes</additionalClasspathElement>
              </additionalClasspathElements>
              <arguments>
                <argument>${project.build.directory}/unwoven-classes/org/example/Main.class</argument>
              </arguments>
            </configuration>
          </execution>
          <execution>
            <!-- You should expect FUBAR! in the output if already compiled classes were not overwritten by weaving. -->
            <id>perform-test</id>
            <goals>
              <goal>java</goal>
            </goals>
            <configuration>
              <mainClass>org.example.Main</mainClass>
            </configuration>
          </execution>
        </executions>
      </plugin>

      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>aspectj-maven-plugin</artifactId>
        <version>1.15.0</version>
        <executions>
          <execution>
            <goals>
              <goal>compile</goal>
            </goals>
          </execution>
        </executions>
        <configuration>
          <source>11</source>
          <complianceLevel>11</complianceLevel>
          <showWeaveInfo>true</showWeaveInfo>
          <verbose>true</verbose>
          <sources>
            <source>
              <basedir>${project.build.sourceDirectory}</basedir>
              <includes>
                <include>**/*.aj</include>
              </includes>
            </source>
          </sources>
          <weaveDirectories>
            <weaveDirectory>${project.build.directory}/unwoven-classes</weaveDirectory>
          </weaveDirectories>
        </configuration>
      </plugin>
    </plugins>
  </build>

The build log then says, just like you wish:

[INFO] --- compiler:3.13.0:compile (default-compile) @ aspectj-nosource-mcve ---
[INFO] Recompiling the module because of changed source code.
[INFO] Compiling 2 source files with javac [debug release 11] to target\unwoven-classes
[INFO] 
[INFO] --- exec:3.5.0:java (replace-message) @ aspectj-nosource-mcve ---
[INFO] 
[INFO] --- aspectj:1.15.0:compile (default) @ aspectj-nosource-mcve ---
[INFO] Showing AJC message detail for messages of types: [error, warning, fail]
[INFO] Join point 'method-execution(java.lang.String org.example.Main.message())' in Type 'org.example.Main' (Main.java:10) advised by around advice from 'org.example.MessageToUppercase' (MessageToUppercase.aj:11)
[INFO] 
[INFO] --- exec:3.5.0:java (perform-test) @ aspectj-nosource-mcve ---
FUBAR!
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

P.S.: Your byte code instrumentation step should really be a separate Maven plugin, just like Maven Compiler, AJ Maven, Byte-Buddy Maven and similar plugins.

plcarmel-aylo commented 1 week ago

Thank you for your help @kriegaex. I will use your suggestions for the "hacky" one-module approach, and maybe use multiple modules eventually.

I am not sure if it is practical for me to have additional modules for unwoven classes and for instrumented code. I develop a solution for a project that already has a lot of maven modules, and the instrumentation will be applied to most of them. I think that my solution would not be very popular with my team if it was increasing the number of modules significantly. What I do is already perceived as being some sort of black magic, so I don't want to push my luck. The aspect itself will be in a separate module, however, because it will be reused many times.

Well, it is the Maven plugin recommended by the AspectJ project, because it has a few more more features than the MojoHaus version while being pretty much compatible to it.

Unfortunately, we are discouraged from using dependencies or plugins that are not well-known. Using a plugin that has the org.codehaus.mojo groupId is a much easier sell for me. However, if there is an official recommendation I can point to, I should be allowed to use your version.

P.S.: Your byte code instrumentation step should really be a separate Maven plugin, just like Maven Compiler, AJ Maven, Byte-Buddy Maven and similar plugins.

I agree, and it is in fact a separate plugin in my real code. I tried to come-up with something as simple as possible to for the example.

kriegaex commented 1 week ago

If the aspect is in a separate module, your configuration will look a bit differently, because you need to use aspectLibraries. But it will also be cleaner, because you do not need to worry about the aspect being compiled by both Maven Compiler and AJ Maven, which is cleaner and simplifies the plugin configuration.

If the custom byte code transformer is a separate plugin, it will also simplify configuration.

If I had known those two facts, I would have suggested another solution. Anyway, let us end the discussion here. I think, we can agree on the maintainers of this project closing the issue, because it contains questions rather than a bug or new feature description. Probably, Stack Overflow would have been a better place to ask. Good luck to you and thanks for the interesting question.

plcarmel-aylo commented 1 week ago

Thank you for taking the time. I got my answers, now its up to you and the maintainers to decide whether adjustments are needed based on the experience I've had as a first time user.

I still think the documentation could be improved to clarify these aspects. I really thought there was an issue, that's why I am here. In fact, I still do think that skipping weaving when there are no source specified is a buggy feature. If it was removed, forceAjcCompile would not be needed at all.

kriegaex commented 1 week ago

I still do think that skipping weaving when there are no source specified is a buggy feature.

But it is not skipped! Weaving is done, otherwise "HELLO!" would not be printed but "Hello!". You got the configuration wrong, using AJ Maven to compile everything from scratch again after Maven Exec has manipulated the code in unwoven-classes. You simply got the order wrong. The problem really sits in front of the computer. That is why you see "HELLO!" instead of "FUBAR!" as you expect.

kriegaex commented 1 week ago

Oh, I know now what you mean. You mean this if you explicitly specify <sources/>, telling AJ Maven that there are no source files, resulting in "[WARNING] No sources found skipping aspectJ compile". Well, isn't that to be expected? The warning even tells you that due to your manual configuration no sources for compilation are found.

I think, in that special case it is OK that if you want to force weaving because you have one or more <weaveDirectories>, you ought to tell the plugin to <forceAjcCompile>true</forceAjcCompile>.

But anyway, you already said that the aspect is in an aspect library and the transformer in a Maven plugin. In that case, you will not need any complicated configuration or extra class file directories. Even if you do the ugly manual byte code manipulation with ReplaceMessage, you could simply put the exec:java step doing that either in a later phase after compileor just reorder the plugins and do it in the compile phase but after aspect weaving.

Just try this simple configuration, getting rid of Maven Compiler config, custom directories or forced compilation:

  <build>
    <plugins>
      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>aspectj-maven-plugin</artifactId>
        <version>1.15.0</version>
        <executions>
          <execution>
            <goals>
              <goal>compile</goal>
            </goals>
          </execution>
        </executions>
        <configuration>
          <source>11</source>
          <complianceLevel>11</complianceLevel>
          <showWeaveInfo>true</showWeaveInfo>
          <verbose>true</verbose>
        </configuration>
      </plugin>
      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>exec-maven-plugin</artifactId>
        <version>3.5.0</version>
        <executions>
          <execution>
            <id>replace-message</id>
            <phase>compile</phase>
            <goals>
              <goal>java</goal>
            </goals>
            <configuration>
              <mainClass>org.example.ReplaceMessage</mainClass>
              <addOutputToClasspath>false</addOutputToClasspath>
              <additionalClasspathElements>
                <additionalClasspathElement>${project.build.outputDirectory}</additionalClasspathElement>
              </additionalClasspathElements>
              <arguments>
                <argument>${project.build.outputDirectory}/org/example/Main.class</argument>
              </arguments>
            </configuration>
          </execution>
          <execution>
            <!-- Should print "FUBAR!" -->
            <id>perform-test</id>
            <goals>
              <goal>java</goal>
            </goals>
            <configuration>
              <mainClass>org.example.Main</mainClass>
            </configuration>
          </execution>
        </executions>
      </plugin>
    </plugins>
  </build>

</project>

See? I just moved Maven Exec behind AJ Maven, and both their executions in the compile phase will be executed in the lexical order of their corresponding plugin definitions in the POM. But this is still hacky compared to the separation-of-concerns method I suggested to you.

[INFO] --- compiler:3.13.0:compile (default-compile) @ aspectj-nosource-mcve ---
[INFO] Recompiling the module because of changed source code.
[INFO] Compiling 3 source files with javac [debug release 11] to target\classes
[INFO] 
[INFO] --- aspectj:1.15.0:compile (default) @ aspectj-nosource-mcve ---
[INFO] Showing AJC message detail for messages of types: [error, warning, fail]
CLASSPATH component C:\Program Files\JetBrains\IntelliJ IDEA 2024.2\plugins\maven\lib\maven3\boot\plexus-classworlds.license: java.util.zip.ZipException: zip END header not found
[INFO] Join point 'method-execution(java.lang.String org.example.Main.message())' in Type 'org.example.Main' (Main.java:9) advised by around advice from 'org.example.MessageToUppercase' (MessageToUppercase.java:11)
[INFO] 
[INFO] --- exec:3.5.0:java (replace-message) @ aspectj-nosource-mcve ---
[INFO] 
[INFO] --- exec:3.5.0:java (perform-test) @ aspectj-nosource-mcve ---
FUBAR!
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

BTW, if you want to deactivate Javac compilation via Maven Compiler completely, you can set the phase for execution default-compile to none explicitly to get rid of compiler:3.13.0:compile completely:

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.13.0</version>
        <executions>
          <execution>
            <id>default-compile</id>
            <phase>none</phase>
          </execution>
        </executions>
      </plugin>

Similarly, you can do it for execution default-testCompile, if for some reason AJ Maven is also supposed to compile or weave anything for your tests.

mojohaus / aspectj-maven-plugin

Weaving of already compiled classes is skipped when there is no source code #217