Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support maven-toolchains-plugin #43

Open
jking-roar opened this issue Jul 28, 2015 · 24 comments
Open

Support maven-toolchains-plugin #43

jking-roar opened this issue Jul 28, 2015 · 24 comments
Assignees

Comments

@jking-roar
Copy link

Support specifying a jdk via the maven-toolchains-plugin

After setting up a toolchain for the Java 8 jdk, I got a build failure. I'd like to specify which java to use per module for my complicated build so I can invoke maven once from a parent. When I tried to do so, I got the below error. Looks like gmaven-plus was invoked using the java specified by JAVA_HOME environment variable, or whatever was on my path.

[INFO] --- maven-compiler-plugin:3.3:testCompile (default-testCompile) @ my-project ---
[INFO] Toolchain in maven-compiler-plugin: JDK[/usr/lib/jvm/java-8-oracle]
[INFO] No sources to compile
[INFO]
[INFO] --- gmavenplus-plugin:1.5:testCompile (default) @ my-project ---
[INFO] Using Groovy 2.4.1 to perform testCompile.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.950s (Wall Clock)
[INFO] Finished at: Tue Jul 28 11:43:59 PDT 2015
[INFO] Final Memory: 10M/239M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.codehaus.gmavenplus:gmavenplus-plugin:1.5:testCompile (default) on project my-project: Error occurred while calling a method on a Groovy class from classpath. InvocationTargetException: com/company/project/SomeClass : Unsupported major.minor version 52.0 -> [Help 1]

@keeganwitt
Copy link
Member

It sounds like SomeClass was compiled with Java 8, but then you tried to use it in a module that is compiled with lower version of Java. GMavenPlus uses the maven.compiler.target property (which you can also override by setting targetBytecode explicitly in the <configuration>). If you don't specify, it defaults to 1.5 just like the Maven compiler plugin. It doesn't do anything with your path or JAVA_HOME.

@jking-roar
Copy link
Author

Those have been specified.
I have made a small example that illustrates the issue:
https://github.com/jking-roar/toolchains-gmavenplus-failure

Setting JAVA_HOME to either Java 6 or Java 8 determines the success or failure of the linked project, even though the toolchains plugin is used to specify that Java 8 should be used.

@keeganwitt keeganwitt self-assigned this Jul 28, 2015
@keeganwitt
Copy link
Member

Thank you for the example. I guess I read your first post too fast. I understand what you're asking for now. I've not used toolchains before. I'll do some reading on what I need to do to make that happen. Hopefully it's simple and I can provide a quick turnaround.

@keeganwitt
Copy link
Member

So it looks like reading the JVM version off the selected toolchain is easy. I could set the target bytecode based on that. But that wouldn't help if your JAVA_HOME is set to Java 6 and your selected toolchain is Java 8. It would only work if your JAVA_HOME is set to an equal or higher version than what the toolchain targets. It also would still compile using the Java from your JAVA_HOME.

I think the right way to do this would be to start a new JVM using the path the toolchain provides. But this isn't as simple as I'd hoped. I think I need to do something similar to the forking the Surefire plugin did in their booter client. I'll continue the effort, but given what I've seen, it's unlikely to make it in the next release.

The best I can suggest in the interim to use the higher of the JDKs in the project as your JAVA_HOME, set the maven.compiler.source and maven.compiler.target properties as appropriate for each module, and use animal-sniffer to detect any classpath usages that don't match the targeted version.

@jking-roar
Copy link
Author

Thanks for the update! Unfortunately, my build environment is complicated
enough that the JAVA_HOME variable needs to be the earlier version at the
moment, so the project containing SomeClass is just kicked off in a
separate step.

I look forward to the enhancement
On Jul 28, 2015 6:50 PM, "Keegan Witt" notifications@github.com wrote:

So it looks like reading the JVM version off the selected toolchain is
easy. I could set the target bytecode based on that. But that wouldn't help
if your JAVA_HOME is set to Java 6 and your selected toolchain is Java 8.
It would only work if your JAVA_HOME is set to an equal or higher version
than what the toolchain targets. It also would still compile using the Java
from your JAVA_HOME.

I think the right way to do this would be to start a new JVM using the
path the toolchain provides. But this isn't as simple as I'd hoped. I think
I need to do something similar to the forking the Surefire plugin did in
their booter client. I'll continue the effort, but given what I've seen,
it's unlikely to make it in the next release.

The best I can suggest in the interim to use the higher of the JDKs in the
project as your JAVA_HOME, set the maven.compiler.source and
maven.compiler.target properties as appropriate for each module, and use
animal-sniffer https://github.com/mojohaus/animal-sniffer to detect any
classpath usages that don't match the targeted version.


Reply to this email directly or view it on GitHub
#43 (comment).

@leonard84
Copy link

@keeganwitt we have switched our build to use toolchains, only GMavenPlus is lacking support for it.

The main idea, is that you can run the maven with a different Java than what you are using for compilation. This is especially helpful if you are working in an environment where you need to use different JDKs for different projects. With toolchains you don't have to constantly swich your JAVA_HOME per project.

I've taken a look at the AbstractCompileMojo and it seems that you are doing in-process compilation instead of forking a new jvm. To get the toolchain to work, you would have to fork a new JVM.

See how it is integrated into the maven-compiler-plugin:
https://github.com/apache/maven-compiler-plugin/blob/adea58fb05c8fe819c18c44d10cfadd6cddf12d7/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java#L388
https://github.com/apache/maven-compiler-plugin/blob/adea58fb05c8fe819c18c44d10cfadd6cddf12d7/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java#L568

@keeganwitt
Copy link
Member

keeganwitt commented Feb 28, 2020

This has been on my to-do list way longer than I would have liked, sorry about that.

You're right, we'd have to fork the JVM. I'd been reading the code in maven-compiler-plugin and maven-surefire-plugin as examples. The hard part isn't reading the toolchain, it's using it to re-invoke the plugin with the correct java command. That's the part I still haven't worked out yet (though I haven't sunk a ton of time into it yet). The compiler plugin has exposed the compiler itself as a toolchain. I think it'd be simpler not to do that (yet) and maybe do something more like what surefire does.

@sebthom
Copy link

sebthom commented Aug 20, 2022

Toolchain awareness would be really appreciated. I too just encountered a situation where I need this. The exec-maven-plugin also supports this. Maybe you can have a look at their implementation:
https://github.com/mojohaus/exec-maven-plugin/blob/master/src/main/java/org/codehaus/mojo/exec/ExecMojo.java
https://www.mojohaus.org/exec-maven-plugin/examples/example-exec-using-toolchains.html

@keeganwitt
Copy link
Member

I'm sorry this hasn't been done. I've had a hard time figuring out the proper way to implement this and getting the time to actually do it.

From what I can tell in my research, there are no utility classes to manage spawning the new java process. I'm gonna have to build it from scratch. It's astonishing to me this is the case, but if there's a better way, I haven't found it yet.

@sebthom
Copy link

sebthom commented Aug 20, 2022

Yeah, would be nice the maven toolchains plugin would provide an AbstractToolchainAwareMavenPlugin with all the boilerplate.

Here is another example where a plugin was made toolchain aware, maybe it helps tbroyer/gwt-maven-plugin@315176c

This may make general process spawning a bit easier: https://github.com/brettwooldridge/NuProcess

@keeganwitt
Copy link
Member

Thank you for the links. What I've been trying to find an example of (and haven't so far) is how one would launch Maven again with the same arguments, using the JDK specified in the toolchain.

@InYourHead
Copy link

Any updates on this topic? I just spend some time to find this issue in my project

@keeganwitt
Copy link
Member

I've reached out to a few folks for some advice on how to implement this. I've got personal things going on right now that won't give me the free time to try to look at this again for a month or so likely. Sorry about this.

@keeganwitt
Copy link
Member

keeganwitt commented Jul 5, 2023

I never heard anything back. What I'm trying to figure out is how to invoke Maven again with the specified JVM, but with all the same parameters and to make sure to invoke the same Maven as well. But I haven't seen any example of this. Perhaps that's not even the right approach.

@keeganwitt keeganwitt reopened this Jul 5, 2023
@bmarwell
Copy link
Contributor

Can do it, but I would like to see both #265 and #267 fixed first.

@bmarwell
Copy link
Contributor

@jking-roar we could add a groovy toolchain.
Currently, the method org.codehaus.groovy.control.CompilationUnit#compile is being used for compilation.
Using a fork argument and/or toolchains, we can use groovyc instead. This can be combined with the JAVA_HOME / JDK from the toolchains.

Is this what you had in mind?

That would give:

  • default (as-is)
    use included groovy library/dependency and call CompilationUnit#compile as before.
  • configuration -> fork=true
    use groovyc from path instead
  • toolchain GROOVY_HOME set
    use groovyc from toolchain

@keeganwitt
Copy link
Member

keeganwitt commented Oct 3, 2023

Consolidating a Slack discussion here.

One concern I have with using groovyc is that it's not a regular binary. It's a Shell script (with a Windows batch script version as well). This might make it a bit brittle. Also, the script has logic for identifying the path to the java binary to invoke. Now if the Maven toolchain is able to set something like JAVA_HOME, then maybe that's not a problem.

As an implementation note, I think the reason the plugin uses CompilationUnit.compile() is that's what the original GMaven plugin did, as well as that's what the groovyc Ant task uses. The groovyc Bash script launches FileSystemCompiler. But offhand, I don't think that should be problematic.

@bmarwell
Copy link
Contributor

bmarwell commented Oct 4, 2023

Ok. Then we could do it.
I will speak to the maven core team whether there is already a toolchain name, or whether we need to create a new one.

@bmarwell
Copy link
Contributor

bmarwell commented Oct 6, 2023

Here is an update, I do not want to "restrict" it to the slack channel.

So, in #maven in ASF slack they sad "whatever we feel is correct" for choosing the toolchain name and their parameters.
Does the goovy dist have a special name?
jdk is used for the JDK, I think just groovy for the Groovy distribution is fine.
we can use installDir as additional Parameter.

Also, groovyc calls startGroovy. It uses standard 'JAVA_HOME` if set. When called from maven, it is (by 100%).
See here: https://github.com/apache/groovy/blob/46e3631daf68168b52df60298309764427b92af1/src/bin/startGroovy#L96-L115

That said, we can start 😃

@keeganwitt
Copy link
Member

I think this will mostly work. But there are some options not parsed by FileSystemCompiler, such as parallelParse that we'll have to figure out some way to continue to support.

@bmarwell
Copy link
Contributor

This got me thinking. We could also use a library from a toolchain. However, I do not see the point in that, because we could just as easy use define a property for the groovy dependencies and use -Dgroovy.version=xxx on the command line when invoking maven.

@jking-roar can you explain your use case once more?

@keeganwitt
Copy link
Member

I was thinking about this yesterday too and started wondering if we really needed a new toolchain. The typical use case here is just which java to use, so shouldn't that use the existing jdk toolchain?

@mfriedenhagen
Copy link

IMO it should use the existing Java toolchain. Right now you get problems, when the Maven process is started with JDK 8 but you use JDK 17 for the toolchain. Then the Java classes are compiled with target 17 but the groovy compiler now runs with JDK 8 from the Maven process and is unable to use the Java class files.

@bmarwell
Copy link
Contributor

I see!

That's still running in <fork> mode with groovyc. You'd lose some of the options. I documented the Feature request for the missing features in my PR, see the link to the ASF JIRA over there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants