eclipse-aspectj / aspectj

Other
289 stars 84 forks source link

JDK 20 snapshot available? #242

Closed hensan64 closed 1 year ago

hensan64 commented 1 year ago

Hi

I need support for JDK20.

aspectj-maven-plugin fails with:

[INFO] [INFO] --- aspectj-maven-plugin:1.13.1:compile (compile_with_aspectj) --- [INFO] Showing AJC message detail for messages of types: [error, warning, fail] [ERROR] unrecognized single argument: "-20"

: [ERROR] no sources specified :
kriegaex commented 1 year ago

What exactly from Java 20 do you need?

hensan64 commented 1 year ago

i want it to work with aspectj-maven-plugin that rely on aspectjrt / aspectjtools.

see error message in original message.

kriegaex commented 1 year ago

AspectJ Maven supports any Java source / target version AspectJ supports. Anyway, you didn't answer my question. Which Java 20 features are you trying to use? What stops you from using compliance level 17 or 19 on JDK 20? Why would you need 20?

hensan64 commented 1 year ago

I work corporately, and for security reasons we need to lift Java continuously to get the latest bug fixes. We are using AspectJ in our internal product code and its maven plugin. So we are not looking at features, we are lifting in order not to get stuck on unsupported code. And to provide the latest language features to our users (it is a test framework). And in order to avoid a big bang problem when a new Java LTS is released we are lifting our code continuously to keep pace... And we want to make sure that the latest compliance level works as well...

kriegaex commented 1 year ago

You gave me no reason to prioritise this. You can run the latest JDK and said you don't need any new language features, of which in Java 20 there are only preview features anyway. You didn't mention a single one of them you want to use. Therefore, there is no need to compile with level 20. The big bang argument does not apply either, if you don't upgrade your code to always use the latest preview features.

You really need to give me more than that for me to cut a release just for you. Besides, JDK 20 support has long been fully functional in my developer version. I just want to avoid the overhead to perform and document a release the features of which nobody uses.

hensan64 commented 1 year ago

Ok. You mention a developer version (java-20 branch). Do you make unofficial development releases to Maven ARM:s?

kriegaex commented 1 year ago

Whatever ARM means in this case, I am not sure. But I just pushed a new 1.9.20-SNAPSHOT (exact snapshot version is 1.9.20-20230608.070552-7) from the Java 20 branch to https://oss.sonatype.org/content/repositories/snapshots. You might need the exact snapshot in the future, if I decide to push another snapshot from the main branch without Java 20 stuff. But I will try to remember to always merge other changes into that branch and publish from there. I am working on AspectJ infrequently these days, so it is not going to change a lot, unless I find some free cycles.

Your feedback is welcome.

hensan64 commented 1 year ago

Thanks for supporting. But I still get this error with 1.9.20-SNAPSHOT using target/source 20:

[INFO] --- aspectj-maven-plugin:1.13.1:compile (compile_with_aspectj) @ abc --- [INFO] Showing AJC message detail for messages of types: [error, warning, fail] [ERROR] unrecognized single argument: "-20"

: [ERROR] no sources specified : Do you know where it originates? When I run the very same with target/source 19 it works...
kriegaex commented 1 year ago

Then probably something in your Maven POM is wrong. But that is rather an AspectJ Maven than an AspectJ question. Anyway, I can help you if you post a sample project reproducing the problem on GitHub.

Update: My guess is, that you simply forgot to add a plugin dependency to aspectjtools in the correct version to AspectJ Maven as described in the plugin documentation, section "Usage - Upgrading or downgrading AspectJ".

hensan64 commented 1 year ago

Both dependencies (rt, tools) are set to 1.9.20-SNAPSHOT in the plugin definition.

Keep ticket open until I have troubleshooted more.

kriegaex commented 1 year ago

Just post your POM(s), would you? This is geting tedious. How do you expect anyone to solve your problem with this kind of information hiding? And besides, 1.9.19 is wrong, you need 1.9.20-SNAPSHOT, which I produced for you with Java 20 support. The problem sits in front of the computer. I am closing this issue.

hensan64 commented 1 year ago

Yes, I have run with 1.9.20-SNAPSHOT you provided as well, but in production we use 1.9.19. Just a typo...

Do not assume people are idiots. I am a senior SW developer, but the problem seems obscure.

The reason i have not posted the POM is that it is corporate and complicated.

Running:

mvn -e -X clean test -DskipTests

The aspect-maven-plugin produces this log, and the AJC compiler do not like the '-20' argument:

[DEBUG] Running : ajc -Xajruntimetarget:1.5 -encoding UTF-8 -20 -classpath ...

I guess it is the maven plugin that produces this AJC call. Is the problem that the plugin adds the '-20' argument or should AJC take it, but somehow AJC is not the latest version (although set so in the pom)?

kriegaex commented 1 year ago

I did not say you are an idiot. I said that I need an MCVE to help you and that information hiding is a bad idea. I also said, the problem sits in front of the computer. That does not mean you are an idiot. It means that you are making a mistake, no more and no less. Can senior developers not make mistakes? BTW, if you are so senior, why don't you know that reproducibility is key to getting better help faster? Why are you ignoring my request to see your POMs or a full MCVE? Give me one, please, or we are done here. If I spend my Sunday afternoon to help you, make it easier for me, would you?

hensan64 commented 1 year ago

Yes, I know reproducibility is important. We deal with that in our own code support. I realize it is difficult to do support via a GitHub Issues tab. It is a little tricky for me when i work corporately and can not publish everything so easily without purging every corporate string. No need to work on Sunday... That is not my expectation.

I will keep on troubleshooting. I have cloned your repos...

But this would help a lot if you can answer during working hours. In the maven log from the aspectj-maven-plugin executing:

mvn -e -X clean test -DskipTests

I get the following log, showing what is called internally in the plugin (I think):

[DEBUG] Running : ajc -Xajruntimetarget:1.5 -encoding UTF-8 -20 -classpath ...

Does AJC support the '-20' argument, or is the version of AJC wrong?

Have a good weekend.

kriegaex commented 1 year ago

MCVE != your original corporate code.

You obviously did not carefully study the link I sent. What I need is a minimal project to reproduce your problem. That should be very easy to produce. Just strip off as much as you can while still reproducing the problem. You can use dummy classes and packages. Every developer should know that. And again, I want to help you. You are simply making it difficult by stubbornly refusing to follow my simple advice to show me your POM. I could have helped you long ago and within one minute. No matter if it is a working day or a Sunday, it is simply a bad idea to ask like this, because you are wasting your own time first and foremost, and as a side effect also mine.

hensan64 commented 1 year ago

${aspectj.version} = 1.9.20-SNAPSHOT

        <plugin>
            <groupId>dev.aspectj</groupId>
            <artifactId>aspectj-maven-plugin</artifactId>
            <version>${aspectj.maven.plugin.version}</version>
            <configuration>
                <complianceLevel>${compiler.compliance.java.version}</complianceLevel>
                <includes>
                    <include>**/*.java</include>
                    <include>**/*.aj</include>
                </includes>
                <excludes>
                    <exclude>**/generated-sources/annotations/**</exclude>
                </excludes>
                <aspectDirectory>src/main/aspect</aspectDirectory>
                <testAspectDirectory>src/test/aspect</testAspectDirectory>
                <XaddSerialVersionUID>false</XaddSerialVersionUID>
                <aspectLibraries>
                    <aspectLibrary>
                        <groupId>com.xxx</groupId>
                        <artifactId>xxx</artifactId>
                    </aspectLibrary>
                    <aspectLibrary>
                        <groupId>com.xxx</groupId>
                        <artifactId>xxx</artifactId>
                    </aspectLibrary>
                </aspectLibraries>
            </configuration>
            <dependencies>
                <dependency>
                    <groupId>org.apache.logging.log4j</groupId>
                    <artifactId>log4j-slf4j-impl</artifactId>
                    <version>${log4j2.version}</version>
                </dependency>
                <dependency>
                    <groupId>org.apache.logging.log4j</groupId>
                    <artifactId>log4j-core</artifactId>
                    <version>${log4j2.version}</version>
                </dependency>
                <dependency>
                    <groupId>org.apache.logging.log4j</groupId>
                    <artifactId>log4j-api</artifactId>
                    <version>${log4j2.version}</version>
                </dependency>
                <dependency>
                    <groupId>org.apache.logging.log4j</groupId>
                    <artifactId>log4j-1.2-api</artifactId>
                    <version>${log4j2.version}</version>
                </dependency>
                <dependency>
                    <groupId>org.aspectj</groupId>
                    <artifactId>aspectjrt</artifactId>
                    <version>${aspectj.version}</version>
                </dependency>
                <dependency>
                    <groupId>org.aspectj</groupId>
                    <artifactId>aspectjtools</artifactId>
                    <version>${aspectj.version}</version>
                </dependency>
                <dependency>
                    <groupId>com.xxx</groupId>
                    <artifactId>xxx</artifactId>
                    <version>${xxx.version}</version>
                </dependency>
            </dependencies>
            <executions>
                <execution>
                    <id>compile_with_aspectj</id>
                    <goals>
                        <goal>compile</goal>
                    </goals>
                </execution>
            </executions>
        </plugin>
hensan64 commented 1 year ago

Sorry for obstructing the process...

kriegaex commented 1 year ago

See? That was not so painful, copying & pasting a POM, was it?

Two obvious things I canot help noticing at first glance:

  1. You did not specify either complianceLevel or a combination of source and target. Without that, the compiler will fall back to ancient versions like 1.5 or even older.

  2. I have no idea why you are specifying so many irrelevant plugin dependencies. You really only need to override org.aspectj:aspectjtools. Just remove the others, please. They are not helping, the plugin does not need them.

There might be more wrong, but let us do this step by step, shall we? Just fix these two first, then please report back.

hensan64 commented 1 year ago
    <java.version>20</java.version>
    <compiler.source.java.version>${java.version}</compiler.source.java.version>
    <compiler.target.java.version>${java.version}</compiler.target.java.version>
    <compiler.compliance.java.version>${java.version}</compiler.compliance.java.version>

It works with java.version=19 so we are not running with 1.5.

Trying it out...

kriegaex commented 1 year ago

Those settings compiler.* are just some random Maven properties. In order for them to have any effect, you need to actually use them in the plugin configuration, e.g.

<complianceLevel>${java.version}</complianceLevel>

or

<complianceLevel>${compiler.compliance.java.version}</complianceLevel>

Maybe you configured them for Maven Compiler, but forgot to do so for AspectJ Maven.

hensan64 commented 1 year ago

Excuse: It is a big product so this is actually not my code. I am just working on uplift testing of JDK20...

Yes, complianceLevel is already set in the pom.xml code above

Testing your suggestions...

hensan64 commented 1 year ago

<complianceLevel>${compiler.compliance.java.version}</complianceLevel>

Is already set since before

kriegaex commented 1 year ago

Oh, I see. Sorry to have overlooked it before. The plugin config is a bit messy, the compliance level is above the in-/excludes, not grouped with the rest of the actual settings. Then, maybe you have a multi-project situation and one POM overrides some inherited config. Maybe it is something like pluginManagement vs. plugins situation. Without an MCVE, I can only speculate, but this is not a quiz show. I would much prefer to analyze rather than guess. So please, provide an MCVE. I am 100% sure that if the plugin is configured the way I told you to, it works with Java 20.

As a developer supporting another one, put yourself in my shoes for a minute: Could you debug a bunch of log statements or an incomplete POM snippet? The problem is most probably in the part of your project I do not see. Hence my "information hiding" remark. This will be the last time I am asking for it. Not even the POM, which is already way less than an MCVE, was complete. Do yourself a favour, prepare an MCVE, push it to GitHub and be happy that I will be able to easily solve your problem within 5 minutes.

hensan64 commented 1 year ago

I have full sympathy for a difficult support situation 😊.

Yes, we have a multi module maven project and pluginManagement and parents and stuff.

While fixing an MCVE, can you answer my question below, which to me is at the core of things...

hensan64 commented 1 year ago

In the maven log from the aspectj-maven-plugin executing:

mvn -e -X clean test -DskipTests

I get the following log, showing what is called internally in the plugin (I think):

[DEBUG] Running : ajc -Xajruntimetarget:1.5 -encoding UTF-8 -20 -classpath ...

Does AJC support the '-20' argument, or is the version of AJC wrong?

[INFO] --- aspectj-maven-plugin:1.13.1:compile (compile_with_aspectj) @ abc --- [INFO] Showing AJC message detail for messages of types: [error, warning, fail] [ERROR] unrecognized single argument: "-20"

kriegaex commented 1 year ago

Does AJC support the '-20' argument, or is the version of AJC wrong?

It does. No more questions before posting the MCVE, otherwise I am going to lock this issue. No offence meant.

hensan64 commented 1 year ago

Just got back from a party...

First: I understand your urgency to get an MCVE. If I were in your shoes I would have requested the same. But for our project it is a major task that will take a lot of time to create properly (if it shall be executable) since our tool is so large and complex. A maven multi module project with 30 sub modules or so... Possible, but the next step...

INSTEAD: I focused on going through your code, both the aspectj-maven-plugin and aspectj and found the following.

image image image

In class org.aspectj.org.eclipse.jdt.internal.compiler.batch.Main, in which the complianceLevel is checked, working for 19 but not for 20 which is the behavior I have been talking about.

Also, there is more code not prepared for JDK20.

image
kriegaex commented 1 year ago

Maybe next time, check the java-20 branch.

https://github.com/eclipse-aspectj/eclipse.jdt.core/blob/ad34fa4ba45bfb08403b5f6a0a6af1d4cf13c1dd/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java#L3227-L3229

    case "20": //$NON-NLS-1$
    case "20.0": //$NON-NLS-1$
      return CompilerOptions.VERSION_20;

https://github.com/eclipse-aspectj/eclipse.jdt.core/blob/ad34fa4ba45bfb08403b5f6a0a6af1d4cf13c1dd/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/classfmt/ClassFileConstants.java#L139-L142

  int MAJOR_VERSION_20 = 64;

  int MAJOR_VERSION_0 = 44;
  int MAJOR_LATEST_VERSION = MAJOR_VERSION_20;

The above is what is contained in the snapshot version. If you see something else, you are not using the snapshot. Go fix your POM. Like I said, the problem sits in front of the computer. I suggest you stop wasting your time with fruitless debugging and share either an MCVE or at least a zipped set of full POMs, if you are incapable of creating an MCVE. You can find some private contact channels on my Stack Overflow profile.

$ cat Foo.java
public class Foo {
  public static void main(String[] args) {
    System.out.println("Hello world");
  }
}

$ java -cp .m2/repository/org/aspectj/aspectjtools/1.9.20-SNAPSHOT/aspectjtools-1.9.20-SNAPSHOT.jar org.aspectj.tools.ajc.Main -20 Foo.java

$ javap -v Foo.class | grep major
  major version: 64
hensan64 commented 1 year ago

MCVE: https://github.com/hensan64/aspectj-jdk20-ts.git

kriegaex commented 1 year ago
$ mvn verify

...
[INFO] Reactor Summary for aspectj-jdk20-ts 0.0.1-SNAPSHOT:
[INFO] 
[INFO] aspectj-jdk20-ts ................................... SUCCESS [  1.194 s]
[INFO] module1 ............................................ SUCCESS [  4.463 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
...

So?

hensan64 commented 1 year ago

Good information. I do not get this result. I will see what can be different in our environment... Could it be that you have a newer version of the snapshot, then what i get from Nexus/Artifactory?

kriegaex commented 1 year ago

Maybe. It could be related to its refresh/caching settings, or maybe your local ones. Snapshots should always be checked for updates.

kriegaex commented 1 year ago

I am not using any repository managers in my current environment, so I simply added this to your POM:

<repositories>
    <repository>
        <id>ossrh-snapshots</id>
        <url>https://oss.sonatype.org/content/repositories/snapshots</url>
        <releases>
            <enabled>false</enabled>
        </releases>
        <snapshots>
            <enabled>true</enabled>
            <updatePolicy>always</updatePolicy>
        </snapshots>
    </repository>
</repositories>
kriegaex commented 1 year ago

You also do not need aspectjrt as a plugin dependency, only as a regular build dependency. It does not hurt, but is superfluous, because aspectjtools is a superset of aspectjrt. The compiler contains the weaver classes, the weaver contains the runtime classes.

kriegaex commented 1 year ago

Maybe building once with -U helps, but only if your Nexus/Artifactory is transparent for update requests.

hensan64 commented 1 year ago

It works after clearing the ".m2/repositories" cache... (I have used -U but it did not help)... A corrupt cache somehow. It seems the problem was we simply used 2 different versions of 1.9.20-SNAPSHOT which of course confused things... I apologize for the inconvenience...

MCVE: Lesson learned.

Question: What is your delivery strategy? To only release aspectjtools for Java LTS but not Java STS?

kriegaex commented 1 year ago

Next time, I suggest to only delete what you wish to refresh. If you have a multi-GB cache like me, it will save a lot of time for not having to re-download everything. Just delete the org/aspectj directory or, even more precise, just search for 1.9.20-SNAPSHOT subdirectories within and delete those. Normally, it should not be necessary to delete the cache at all, unless something is corrupt. Snapshots should be refreshed automatically, unless you change the default setting in your settings.xml or repository manager.

Since I took over creating AspectJ releases, I released an AJ version for each JDK release. JDK 20 is the first one I skipped, but maybe I will publish a release anyway, even though it is late now. What stopped me was my lack of free cycles. The coding was done long ago, right after JDK 20 was published, but I should also write some short release notes and move add/move a few more tests. On top of working, I was also travelling a lot lately (also mostly work-related). But for JDK 21 latest, there will be a new AJ release.