Le mer. 16 juin 2021 à 14:04, Romain Manni-Bucau <rmannibu...@gmail.com> a écrit :
> Le mer. 16 juin 2021 à 13:53, Guillaume Nodet <gno...@apache.org> a écrit > : > > > Le mer. 16 juin 2021 à 13:39, Romain Manni-Bucau <rmannibu...@gmail.com> > a > > écrit : > > > > > Le mer. 16 juin 2021 à 12:05, Guillaume Nodet <gno...@apache.org> a > > écrit > > > : > > > > > > > A few plugins that fail with the same problem found by googling a > bit: > > > > https://youtrack.jetbrains.com/issue/IDEA-266556 > > > > https://github.com/projectlombok/lombok/issues/2681 > > > > > > > > > > > > > > > > > > https://community.sonarsource.com/t/maven-sonar-scanner-not-working-with-jdk-16/40699 > > > > > > > > > > > > > > > > > > https://www.bountysource.com/issues/97382407-enunciate-does-not-run-on-jdk16 > > > > That's just the first page on google... so I don't think this is me > > > running > > > > into a corner case. JDK 16 has only been release 2 months ago, and > the > > > > rules have changed quite a bit since JDK 15, so that's not an old > > > problem, > > > > it's a new one. > > > > > > > > > > All these ones are solved with MAVEN_OPTS AFAIK so it is already in - > > > understand we can't have the correct jpms options by default since > we'll > > > never know all plugins. > > > > > > Maybe I'm the only one, but when I see a project with a pom.xml and I > > just > > want to build it, the first thing I try is `mvn install -DskipTests`. > This > > work in most of the case, and I think it should stay that way. That's why > > I'm looking for a way to configure those options per-project. > > > > So yes, that's right, MAVEN_OPTS may solve the problem. But that does > not > > solve my problem which is: > > * how to set up the environment (maven options or jvm options or > > eventually environment variable) > > * with a config per project (not at the maven installation level) > > * with something that can have a bit of logic so that it can depend on > > which jdk version is used > > > > setenv.sh in multimoduleroot/.mvn ;) > > Actually, thinking about it, this also raises a few problems. It makes the life for tools embedding maven much more complicated. I'm wondering if a configuration based solution wouldn't be easier to work with (i.e. without having to interpret or spawn a command shell) ? In felix / karaf, the problem was solved by using property files and property substitution. This could look like: jre-options = ${jre-${java.specification.version}-options:-${jre-default-options}} jre-default-options = [default options] jre-1.8 = [my JDK 1.8 specific options] This kind of configuration allows you to lay out a bit of logic in the configuration. This is just an example of course, but it would be more inlined with the [root]/.mvn/jvm.config and like. Guillaume > > > > > > > > > > > > > > > > > I think adding the --illegal-access=permit option on the command is > > > > sufficient to solve all those problems, but the problem is still the > > > same : > > > > this option will break if using JDK 1.8. This has to be configured > on > > > the > > > > project and not globally. > > > > > > > > > > This will likely break soon so I wouldn't consider it as an option. > > > > > > > See above, I'm not looking for maven to fix the problem. I'm looking for > > maven to provide a way to fix the problem. Which implies, the exact > option > > to configure is up to the user, so the fact that is already deprecated is > > irrelevant. > > > > > > > > > > > > > > > > I'm not sure what you refer to with the setenv.sh script ? is that > > > > something that maven launcher runs prior to launching the JVM ? > > > > > > > > > > Yes, it is often used in servers (thinking to tomcat or karaf). > > > mvn.sh (similarly for .bat flavor) would source bin/setenv.sh (bat) if > > > exists. > > > > > > this script would container any logic the user wants and be usable to > > setup > > > maven env vars before the script reads it. > > > > > > a common (trivial) setup would be (a more advanced can test java > > > --version): > > > > > > bin/setenv.sh: > > > #! /bin/sh > > > epxort MAVEN_OPTS=--illegal-access=permit > > > > > > > > So could we enhance the maven shell script to look for a > > [project-root]/.mvn/setenv.sh and call it ? That would definitely solve > > the problem. > > > > +1 > > > > > > > > > > > > > > > > > Le mer. 16 juin 2021 à 09:55, Romain Manni-Bucau < > > rmannibu...@gmail.com> > > > a > > > > écrit : > > > > > > > > > Le mer. 16 juin 2021 à 08:39, Guillaume Nodet <gno...@apache.org> > a > > > > écrit > > > > > : > > > > > > > > > > > Well, my point was not really to find a workaround, but rather to > > > > start a > > > > > > discussion around "maven jvm customization from files is not well > > > > > > supported for now" ... :-) > > > > > > > > > > > > > > > > Got it, but point is "is it that often needed"? > > > > > Most plugin using toolchain or equivalent can have JVM > customization > > > > > (already) and it is trivial to handle per jvm config through > > profiles. > > > > > So at the end it is only about maven "embedded" plugins and maven > > > itself. > > > > > Last one shouldnt need anything and first ones are solved by > forcing > > a > > > > JVM > > > > > version for the main build (which does not prevent to run tests and > > > > graalvm > > > > > native-image with 50 different JVMs). > > > > > > > > > > The setenv.sh/bat option seems like an acceptable compromise for > > these > > > > > border cases but not sure we need anything more fancy. > > > > > What I would be really against is a jvm.config like solution which > > does > > > > not > > > > > enable to use any configuration (we can't use space for example > right > > > now > > > > > in system properties so if we get the same limitation for env vars > it > > > > would > > > > > be pointless IMHO). > > > > > > > > > > Hope it makes sense. > > > > > > > > > > > > > > > > > > > > > > Le mer. 16 juin 2021 à 07:24, Romain Manni-Bucau < > > > > rmannibu...@gmail.com> > > > > > a > > > > > > écrit : > > > > > > > > > > > > > Le mar. 15 juin 2021 à 23:39, Guillaume Nodet < > gno...@apache.org > > > > > > a > > > > > > écrit > > > > > > > : > > > > > > > > > > > > > > > In my case, it was a test in mvnd which is using some low > level > > > > > > > > reflection. I do agree this is not the best example, because > > an > > > > easy > > > > > > > > workaround can be done by configuring surefire to use a fork. > > > > > However, > > > > > > > if > > > > > > > > a plugin does the same thing, there's no easy way to > workaround > > > it. > > > > > > > > > > > > > > > > > > > > > > Guess the most trivial workaround is to replace the plugin by a > > > > custom > > > > > > exec > > > > > > > one propagating maven current context in an extension and > forking > > > the > > > > > > > plugin run since maven jvm customization from files is not well > > > > > supported > > > > > > > for now or would need some more advanced script which could > > source > > > an > > > > > > > optional setenv to set maven_opts but this last one would test > > java > > > > > > version > > > > > > > which is never great to launch a build and would require to > test > > > args > > > > > > "the > > > > > > > maven way" (phase, plugin:goal, plugin:goal@exec) to bypass it > > > when > > > > > > > possible. > > > > > > > > > > > > > > > > > > > > > > Le mar. 15 juin 2021 à 19:04, Robert Scholte < > > > rfscho...@apache.org > > > > > > > > > > a > > > > > > > > écrit : > > > > > > > > > > > > > > > > > AFAIK the --add-opens is only useful when using the module > > > path. > > > > > > Maven > > > > > > > > > itself and the plugins use the classpath, so I would like > to > > > have > > > > > an > > > > > > > > > example to better understand your issue. > > > > > > > > > > > > > > > > > > > > > > > > > As others explained, --add-opens allow overriding the default > > > > > > > > accessibility. For example if you get a private field on a > JDK > > > > class > > > > > > and > > > > > > > > use setAccessible using reflection, you may run into an > > exception > > > > > that > > > > > > > > would require adding this --add-opens argument on JDK 11 or > > more. > > > > > They > > > > > > > are > > > > > > > > not required on JDK 8 and the annoying thing is that the > option > > > > > causes > > > > > > > the > > > > > > > > JVM to fail. > > > > > > > > > > > > > > > > Using forked JVM from inside plugins does work around the > > > problem, > > > > > but > > > > > > > only > > > > > > > > when that's possible ;-) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > thanks, > > > > > > > > > Robert > > > > > > > > > On 15-6-2021 18:43:35, Guillaume Nodet <gno...@apache.org> > > > > wrote: > > > > > > > > > Hi everyone ! > > > > > > > > > > > > > > > > > > There are some small incompatibilities between JDK around > the > > > > > > supported > > > > > > > > > command line versions. Usually, those do not cause any real > > > > > problems. > > > > > > > > > However, the "--add-opens" are sometimes necessary and only > > > > > supported > > > > > > > on > > > > > > > > > JDK >= 9, as the JVM exits with an error on JDK 8. > > > > > > > > > Some plugins may require the use of those options when > > running > > > on > > > > > JDK > > > > > > > >= > > > > > > > > > 9. What would be the way to solve this ? > > > > > > > > > > > > > > > > > > For mvnd, when the client launches the daemon, the > > > > JDK_JAVA_OPTIONS > > > > > > > > > environment property is used. But it works because we have > > > > control > > > > > on > > > > > > > the > > > > > > > > > client environment. But when you just clone a git repo, > > asking > > > > the > > > > > > user > > > > > > > > to > > > > > > > > > set a specific environment is problematic imho. The > > > > > .mvn/maven.config > > > > > > > or > > > > > > > > > .mvn/jvm.config can't be used to set up environment > variables > > > or > > > > to > > > > > > > > > conditionally set up arguments afaik. > > > > > > > > > > > > > > > > > > Any idea ? > > > > > > > > > > > > > > > > > > -- > > > > > > > > > ------------------------ > > > > > > > > > Guillaume Nodet > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > ------------------------ > > > > > > > > Guillaume Nodet > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > ------------------------ > > > > > > Guillaume Nodet > > > > > > > > > > > > > > > > > > > > > > > -- > > > > ------------------------ > > > > Guillaume Nodet > > > > > > > > > > > > > -- > > ------------------------ > > Guillaume Nodet > > > -- ------------------------ Guillaume Nodet
