dev-aspectj / aspectj-maven-plugin

AspectJ Maven Plugin
MIT License
48 stars 10 forks source link

Any way to exclude a sub-directory? #108

Closed zenbones closed 1 year ago

zenbones commented 1 year ago

Is there a way to exclude a sub-directory, and hopefully a relative sub-directory, as in...

**/target/generated-sources/annotations/**

...or at the very least...

${project.basedir}/target/generated-sources/annotations/**

kriegaex commented 1 year ago

What are you trying to achieve? Please provide a minimal example project.

zenbones commented 1 year ago

I am trying to get all generated sources in a maven build to be excluded from aspectj compilation, in a multi-module project, without having to list every possible sub-directory those generated sources might be in.

The question you might be asking is why? And the answer to that is that I'm using querydsl, and, when upgrading to java 17 and the jakarta namespace, the sources generated by querydsl engender this error when subsequently consumed by the aspectj plugin...

[ERROR] Failed to execute goal dev.aspectj:aspectj-maven-plugin:1.13.1:compile (default) on project smallmind-persistence: AJC compiler errors:
[ERROR] error at (no source information available)
[ERROR] C:\Users\david\Documents\Nutshell\empyrean\aeon\pantheon\org\smallmind\persistence\target\generated-sources\annotations\org\smallmind\persistence\orm\jpa\QTimestampedJPADurable.java:0::0 Internal compiler error: java.lang.Exception: java.lang.StackOverflowError at org.aspectj.org.eclipse.jdt.internal.compiler.apt.dispatch.RoundDispatcher.handleProcessor(RoundDispatcher.java:172)
[ERROR]

I could include the stack trace although it does not look particularly helpful. The sources generated by querydsl look proper and the standard java compiler (openjdk 17.0.5 2022-10-18) does not seem to have a problem with them, and I don't need them processed by aspectj, so I thought I might try excluding them. It didn't seem like I should need to excluded them one by one, but all attempts I've made at providing an excluded directory path in the aspectj plugin config have failed to have the desired effect, and the sources get processed. The other option is finding out why there's a stack overflow, but that would involve forking the project, debugging it and fixing it, and the exclusion seemed simpler.

zenbones commented 1 year ago

I could provide a 'minimal project', but I'm not sure that's necessary to answer a question about how to exclude a directory subtree from aspectj processing, which is really just about configuration options in the plugin. Resolving the error itself would certainly require a minimal project, but it may be easier for me to pull and fix the code as opposed to providing a working example sliced out of my project and hoping you can fix the code.

I am, of course, grateful for any insight you can provide into any of these questions, and I'll provide a PR for any fixes I can make.

kriegaex commented 1 year ago

Excluding the directory would be a workaround, not a solution without knowing the root cause. I am waiting for the sample project.

zenbones commented 1 year ago

While you're waiting, is it possible to answer the initial question? Is there a way to exclude a directory? Or is that knowledge man was not meant to know?

zenbones commented 1 year ago

Underneath the covers the DirectoryScanner is being used to retrieve the file list. It's docs say...

Class for scanning a directory for files/directories which match certain criteria.
These criteria consist of selectors and patterns which have been specified. With the selectors you can select which files you want to have included. Files which are not selected are excluded. With patterns you can include or exclude files based on their filename.
The idea is simple. A given directory is recursively scanned for all files and directories. Each file/directory is matched against a set of selectors, including special support for matching against filenames with include and and exclude patterns. Only files/directories which match at least one pattern of the include pattern list or other file selector, and don't match any pattern of the exclude pattern list or fail to match against a required selector will be placed in the list of files/directories found.
When no list of include patterns is supplied, "**" will be used, which means that everything will be matched. When no list of exclude patterns is supplied, an empty list is used, such that nothing will be excluded. When no selectors are supplied, none are applied.
The filename pattern matching is done as follows: The name to be matched is split up in path segments. A path segment is the name of a directory or file, which is bounded by File.separator ('/' under UNIX, '\' under Windows). For example, "abc/def/ghi/xyz.java" is split up in the segments "abc", "def","ghi" and "xyz.java". The same is done for the pattern against which should be matched.
The segments of the name and the pattern are then matched against each other. When '**' is used for a path segment in the pattern, it matches zero or more path segments of the name.
There is a special case regarding the use of File.separators at the beginning of the pattern and the string to match: When a pattern starts with a File.separator, the string to match must also start with a File.separator. When a pattern does not start with a File.separator, the string to match may not start with a File.separator. When one of these rules is not obeyed, the string will not match.
When a name path segment is matched against a pattern path segment, the following special characters can be used: '*' matches zero or more characters '?' matches one character.
Examples:
"**\*.class" matches all .class files/dirs in a directory tree.
"test\a??.java" matches all files/dirs which start with an 'a', then two more characters and then ".java", in a directory called test.
"**" matches everything in a directory tree.
"**\test\**\XYZ*" matches all files/dirs which start with "XYZ" and where there is a parent directory called test (e.g. "abc\test\def\ghi\XYZ123").
Case sensitivity may be turned off if necessary. By default, it is turned on.
Example of usage:
  String[] includes = { "**\\*.class" };
  String[] excludes = { "modules\\*\\**" };
  ds.setIncludes( includes );
  ds.setExcludes( excludes );
  ds.setBasedir( new File( "test" ) );
  ds.setCaseSensitive( true );
  ds.scan();

  System.out.println( "FILES:" );
  String[] files = ds.getIncludedFiles();
  for ( int i = 0; i < files.length; i++ )
  {
      System.out.println( files[i] );
  }

So the exclude...

**/target/generated-sources/annotations/**/*.java

...should work, but does not. Not sure why it doesn't yet, but the code for retrieving the file list is pretty awful, so I may replace that wholesale with something RegEx, Path and Files based and give that a try. Would certainly require a lot less code to do the same thing.

zenbones commented 1 year ago

I got a fixed version that obeys the exclusions. Passes all bit 2 tests, and removing those for now gave me a version I could run in my project successfully. Unfortunately for me, the stack overflow occurs in non-generated code as well, which I can now see, and now must fix. So you may get that test case yet. Will take me a few days to simplify and test the example project.

kriegaex commented 1 year ago

See? This is exactly the reason I asked for the sample project: because you cannot be sure that the problem is limited to your generated classes. To manually exclude every class in which the error occurs, is not a smart solution either.

I want to see your project and determine, if the configuration is wrong, or if there is a possible bug in the AspectJ weaver or compiler. You are just wasting your time by pursuing the wrong path to solve this problem.

zenbones commented 1 year ago

Well, the excludes should actually exclude and do not, and this is a bug, so I would not call the pursuit a waste of time, even if that fix is not a fix for my underlying issue. I am putting together a project, but, in the meantime, I was able to get a stack trace using a breakpoint and mvnDebug, and it may be of interest...

 org.aspectj.org.eclipse.jdt.internal.compiler.apt.model.TypeMirrorImpl.toString(TypeMirrorImpl.java:93)
 com.querydsl.apt.ExtendedTypeFactory$2.visitBase(ExtendedTypeFactory.java:207)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:252)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:201)
 org.aspectj.org.eclipse.jdt.internal.compiler.apt.model.TypeVariableImpl.accept(TypeVariableImpl.java:77)
 java.compiler@17.0.5/javax.lang.model.util.AbstractTypeVisitor6.visit(AbstractTypeVisitor6.java:95)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:254)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:201)
 org.aspectj.org.eclipse.jdt.internal.compiler.apt.model.TypeVariableImpl.accept(TypeVariableImpl.java:77)
 java.compiler@17.0.5/javax.lang.model.util.AbstractTypeVisitor6.visit(AbstractTypeVisitor6.java:95)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:254)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:201)
 org.aspectj.org.eclipse.jdt.internal.compiler.apt.model.TypeVariableImpl.accept(TypeVariableImpl.java:77)
 java.compiler@17.0.5/javax.lang.model.util.AbstractTypeVisitor6.visit(AbstractTypeVisitor6.java:95)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:254)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:201)
 org.aspectj.org.eclipse.jdt.internal.compiler.apt.model.TypeVariableImpl.accept(TypeVariableImpl.java:77)
 java.compiler@17.0.5/javax.lang.model.util.AbstractTypeVisitor6.visit(AbstractTypeVisitor6.java:95)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:254)
 com.querydsl.apt.ExtendedTypeFactory$2.visitTypeVariable(ExtendedTypeFactory.java:201)

...and so on. This is clearly a somewhat nasty interaction. Not sure who's responsibility it should be to check for this recursion or in what way it should be cut. But I think I can reproduce this in a smaller form.

zenbones commented 1 year ago

I have a test project.

https://github.com/zenbones/showme

With any luck, this should produce the overflow...

mvn clean install

It was as simple as I could get make it while still producing the error.

kriegaex commented 1 year ago

This is an AspectJ compiler problem, see the corresponding bug I created. It is completely unrelated to AspectJ Maven but seems to be related to AspectJ's limited APT support. So I am closing the issue here, please watch the other one.

kriegaex commented 1 year ago

Here is a workaround for you:

  1. Add <phase>process-classes</phase> to your AspectJ Maven <execution> in order to make sure that the regular Maven Compiler Plugin takes care of annotation processing first, before AspectJ Maven kicks in.
  2. Make sure that annotation processing is skipped in AspectJ Maven by adding <proc>none</proc> to the <configuration>.

If your annotation processor would create source code containing aspect code, you probably would also want:

<sources>
  <source>
    <basedir>${project.build.sourceDirectory}</basedir>
    <basedir>${project.build.directory}/generated-sources/annotations</basedir>
  </source>
</sources>

But if the generated source code is not aspect code or needed in order to compile aspects into your target classes, I guess you can just skip that part.

kriegaex commented 1 year ago

Another alternative that just came to my mind to remove AspectJ Maven from the showme-baz module and just generate source code there and compile with Maven Compile. Then add an extra module showme-baz-woven, use binary weaving and use showme-baz as a <weaveDependency>. That way, APT would not interfere with aspect weaving, and the classes compiled from the generated source code would automatically be part of aspect weaving, too. The advantage would be that no special compiler config would be necessary for AspectJ Maven (except for the weave dependency, of course).

zenbones commented 1 year ago

Allow me to understand. I'm not sure why aspectj would work in phases other than process-classes, or apt in phases other than generate-sources for that matter. I added the phase into the aspectj-maven-plugin config, which does not, on its own, solve the problem.

            <execution>
              <id>aspect</id>
              <goals>
                <goal>compile</goal>
              </goals>
              <phase>process-classes</phase>
            </execution>
            <execution>
              <id>aspect-tests</id>
              <goals>
                <goal>test-compile</goal>
              </goals>
              <phase>process-test-classes</phase>
            </execution>

Adding...

<proc>none</proc>

...did work. I have a number of annotation driven aspects, and I'd like to make sure that adding proc none does not stop either their generation into the library, or their use within subsequently woven libraries using the generated library as a dependency. All weaving is binary, both the generation of and the use of the aspect library.

zenbones commented 1 year ago

As I've provided a test project, can I ask if you see the problem I brought up originally? If you use an exclude like...

**/target/generated-sources/annotations/**

...is that sub-tree excluded? I think the docs say this should work, but in practice I have not seen that function. Is that just me, or is this a real bug?

kriegaex commented 1 year ago

I'm not sure why aspectj would work in phases other than process-classes

By default, it works in the compile phase, just like Maven Compiler.

or apt in phases other than generate-sources for that matter

I do not see any log output concerning APT on the Maven console. As there is no dedicated plugin in your build which would run in generate-sources, I assume that Maven Compiler takes care of both code generation and compilation in the compile phase, because it detects the META-INF/services/javax.annotation.processing.Processor from querydsl-apt on the classpath.

I added the phase into the aspectj-maven-plugin config, which does not, on its own, solve the problem.

Of course not, because I also instructed you to add <proc>none</proc>.

BTW, if you do not want to move AspectJ Maven to a separate phase, you can also make sure to declare Maven Compiler before AspectJ Maven in your POM in order to make sure that APT and Javac run before AJC, but then you should document it in a comment, because otherwise a co-worker could refactor the POM and switch the plugin execution order within the same phase or simply remove the explicit plugin declaration (because Maven Compiler is declared in the Maven root POM already), making that solution somewhat more brittle and error-prone.

Adding...

<proc>none</proc>

...did work. I have a number of annotation driven aspects, and I'd like to make sure that adding proc none does not stop either their generation into the library, or their use within subsequently woven libraries using the generated library as a dependency. All weaving is binary, both the generation of and the use of the aspect library.

That option just deactivates APT within AspectJ Maven, because Maven Compiler already does it. No problem there.

kriegaex commented 1 year ago

If you use an exclude like...

**/target/generated-sources/annotations/**

...is that sub-tree excluded? I think the docs say this should work, but in practice I have not seen that function.

As far as I have seen, there is no need to exclude it, because AspectJ Maven ignores the directory anyway. It does not know about it, because Maven Compiler created it. On opposite, you would have to manually include it in order for AspectJ Maven to compile those files, see my example above.

Why are you still trying to exclude that directory in the first place? APT on your original classes causes the problem in AspectJ Maven, not those generated classes. They were never the problem, which is why I told you from the beginning to provide the MCVE, so I could find out what was really going on. You did yourself the biggest favour by providing the sample project, otherwise we would have wasted time, trying to exclude something which is completely unrelated to the problem's root cause.

kriegaex commented 1 year ago

For the record: Not only is this not an AspectJ Maven bug, but it is not an AspectJ bug either. Instead, it is an upstream problem in JDT Core and ECJ, see https://github.com/eclipse/org.aspectj/issues/195#issuecomment-1328934301. I.e., it can only be "fixed" in AspectJ after it has been fixed in ECJ by refreshing the AspectJ version of JDT Core.

zenbones commented 1 year ago

Thank you for all that explanation. I think that answers my questions. The only reason I was wondering about the exclude is because that hasn't worked for me, but that's belaboring the point. If I feel strongly I'll open a different ticket on that and I can leave this one alone.