You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/03/16 20:11:23 UTC
[GitHub] [maven] mthmulders opened a new pull request #456: Run GitHub actions integration tests with Java 16
mthmulders opened a new pull request #456:
URL: https://github.com/apache/maven/pull/456
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-804705606
@mthmulders that's what the test does yes. The most important is the execution classloader which should rely on the plugin loader, can be done with a .java but must be executed in a mojo classloader (so exec-maven-plugin does a bit too much to test that accurately so not sure about a .java).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] mthmulders edited a comment on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
mthmulders edited a comment on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-806169235
I gave BeanShell a try this evening, but I'm greeted with
> Class: javax.enterprise.inject.Instance not found in namespace
So, to summarise:
> @mthmulders that's what the test does yes. The most important is the execution classloader which should rely on the plugin loader, can be done with a .java but must be executed in a mojo classloader (so exec-maven-plugin does a bit too much to test that accurately so not sure about a .java).
If I understand correctly, we can't run this test using the exec-maven-plugin.
From my experience, we can't run it with JUnit and we can't run it with Bsh either. I'm afraid we're running out of options....
I think there are not much other options left than moving to [Groovy 4.0.0-alpha-2](https://github.com/apache/groovy/blob/1136910170fd100fb2d792db95ec08b935f29530/versions.properties#L20) and hoping not every new Java release will see this issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] mthmulders commented on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
mthmulders commented on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-800605050
The `MavenITmng7045DropUselessAndOutdatedCdiApiTest` test appears to use Groovy. That test fails with
```
Caused by: java.lang.IllegalArgumentException: Unsupported class file major version 61
at groovyjarjarasm.asm.ClassReader.<init> (ClassReader.java:189)
at groovyjarjarasm.asm.ClassReader.<init> (ClassReader.java:170)
at groovyjarjarasm.asm.ClassReader.<init> (ClassReader.java:156)
at groovyjarjarasm.asm.ClassReader.<init> (ClassReader.java:277)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] mthmulders commented on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
mthmulders commented on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-805247010
I've tried to rewrite the Groovy script to a JUnit test. The test succeeds with both Maven 3.6.3 and 4.0.0-alpha-1-SNAPSHOT:
[INFO] Running org.apache.maven.its.mng7045.FindMethodFromDependencyTest
InstanceSource: (file:/Users/maarten/.m2/repository/javax/enterprise/cdi-api/2.0/cdi-api-2.0.jar <no signer certificates>)
Note that I switched the Geronimo dependency for the javax.enterprise one, as the Geronimo one gives
java.lang.NoClassDefFoundError: javax/inject/Provider
at org.apache.maven.its.mng7045.FindMethodFromDependencyTest.findInstanceStreamMethod(FindMethodFromDependencyTest.java:9)
The test runs using Surefire 3.0.0-M5 and JUnit 4.13.2.
But that route doesn't help us, as we can't determine whether CDI 1.0 "leaked in" or not. To be continued - but given that I know nothing about bsh or how to run it from Maven, it will probably take a lot longer. If anyone else does, feel free to chime in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-800611518
Groovy and/or ASM nees to be updated. @rmannibucau , can you help?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] mthmulders commented on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
mthmulders commented on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-812486116
> But the [ASM changelog](https://asm.ow2.io/versions.html) says Java 17 support was only added with ASM 9.1. That version is only included in [Groovy 4.0.0-alpha-2](https://github.com/apache/groovy/blob/1136910170fd100fb2d792db95ec08b935f29530/versions.properties#L20).
Turns out ASM 9.1 is not in Groovy 4.0.0-alpha-2 either - it is only in HEAD. According to [the Groovy issue tracker](https://issues.apache.org/jira/browse/GROOVY-9943), it may end up in Groovy 3.0.8 and/or 4.0.0-alpha-3. Both of them are not released at this point.
Therefore, I will create a separate merge request to add Java 17-ea to the matrix, and this merge request will only add Java 16. We can then merge this one on short notice, and merge the other one when there is a suitable Groovy version available.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] mthmulders commented on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
mthmulders commented on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-804701641
> @mthmulders I'd move MavenITmng7045DropUselessAndOutdatedCdiApiTest to pure java or bsh I think to avoid this issue each 6 months maybe
I could give that a try, but before I do that, let me make sure I understand the test. It verifies if the `javax.enterprise.inject.Instance` class has a method `stream`. It should have that method; if it does not, the test should fail.
This could also be implemented with a Java source file that we try to compile - if compilation fails, the test should also fail. Did I understand it correctly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] mthmulders commented on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
mthmulders commented on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-804683539
The offending test has a Groovy script in the POM. It uses the [gmavenplus-plugin](https://search.maven.org/artifact/org.codehaus.gmavenplus/gmavenplus-plugin/1.11.0/maven-plugin) with an inline dependency on Groovy 3.0.7, which is the latest version of Groovy.
According to [the Groovy build](https://github.com/apache/groovy/blob/a43f54cb59f19218a9cbc9edb3741dbd4a2e2e77/build.gradle#L137), that version of Groovy uses ASM 9.0.
But the [ASM changelog](https://asm.ow2.io/versions.html) says Java 17 support was only added with ASM 9.1. That version is only included in [Groovy 4.0.0-alpha-2](https://github.com/apache/groovy/blob/1136910170fd100fb2d792db95ec08b935f29530/versions.properties#L20). Do we want to move to 4.0.0-alpha-2 for that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] mthmulders commented on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
mthmulders commented on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-806169235
I gave BeanShell a try this evening, but I'm greeted with `Class: javax.enterprise.inject.Instance not found in namespace`.
> @mthmulders that's what the test does yes. The most important is the execution classloader which should rely on the plugin loader, can be done with a .java but must be executed in a mojo classloader (so exec-maven-plugin does a bit too much to test that accurately so not sure about a .java).
If I understand correctly, we can't run this test using the exec-maven-plugin.
From my experience, we can't run it with JUnit and we can't run it with Bsh either. I'm afraid we're running out of options....
I think there are not much other options left than moving to [Groovy 4.0.0-alpha-2](https://github.com/apache/groovy/blob/1136910170fd100fb2d792db95ec08b935f29530/versions.properties#L20) and hoping not every new Java release will see this issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] mthmulders merged pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
mthmulders merged pull request #456:
URL: https://github.com/apache/maven/pull/456
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #456: Run GitHub actions integration tests with Java 16
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #456:
URL: https://github.com/apache/maven/pull/456#issuecomment-804685141
@mthmulders I'd move MavenITmng7045DropUselessAndOutdatedCdiApiTest to pure java or bsh I think to avoid this issue each 6 months maybe
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org