Use args file instead of classpath shortening#34227
Conversation
|
Tests should go into Did not see another test for |
|
Thank you for your proposed contribution! This PR has a valid DCO. The relevant team for this area will confirm the design of the implementation choices and follow up on what tests should be added or changed. |
|
This is a good idea, but we need to continue to support the pathing jar on Java 8. We should be able to use an argfile for all Java 9+. We won't get to this for 9.1. If you're willing to make more changes, let us know. |
In case 
|
|
If I do "Update branch" on GitHub, the DCO test fails. Should I ignore that - or should I use the command line to keep this branch recent? I am assuming, it will be at least 2026 until this is re-considered, but I am planning to check back weekly (at least monthly) and keep the branch recent to avoid effort for integrating. |
Yes, I am willing to make more changes. I pushed updates two weeks ago adressing Java 8 compatibiltiy. |
|
With today's version of the patch (w/ latest |
|
Any chance of getting this in the next Gradle version? This is really annoying |
Signed-off-by: Oliver Kopp <kopp.dev@gmail.com>
Signed-off-by: Oliver Kopp <kopp.dev@gmail.com>
Signed-off-by: Oliver Kopp <kopp.dev@gmail.com>
Signed-off-by: Oliver Kopp <kopp.dev@gmail.com>
Signed-off-by: Oliver Kopp <kopp.dev@gmail.com>
Signed-off-by: Oliver Kopp <kopp.dev@gmail.com>
big-guy
left a comment
There was a problem hiding this comment.
Thanks for your patience. This should be in 9.5
| return shortenedArguments; | ||
| } catch (IOException e) { | ||
| LOGGER.info("Pathing JAR could not be created, Gradle cannot shorten the command line.", e); | ||
| if (javaModuleDetector == null) { |
There was a problem hiding this comment.
I thought this looked suspicious and I realize now what's going on...
This can only be null in some very specific situations, which wouldn't be encountered with a JavaExec task or someone using javaexec {}.
For regular use, this is always non-null, so this doesn't detect whether or not the underlying Java executable will support arg files or not. We end up using arg files no matter what.
I added an integ test that fails when running with Java 8 + shortening: https://ge.gradle.org/s/dpuamnzel2m5o/tests/task/:language-java:embeddedIntegTest/details/org.gradle.api.tasks.JavaExecWithLongCommandLineIntegrationTest/succeeds%20with%20long%20command%20line%20using%20Java%208%20toolchain?top-execution=1
I looked around how we could do this properly. We have a JvmMetadataDetector service which would tell us the version of Java by probing the JVM, but there appear to be a couple of problems with that:
- We might be using JavaExecHandleBuilder itself to probe
- We're dealing with the executable as just a String
- There are a lot of hoops to get this service down here
But I found a way around this, which is to see that we have --module or --module-path and assume we can use arg files (because only Java 9+ supports those args).
That does mean that you can do something strange:
- Have a project that's producing a module
- Configure a JavaExec to use Java 8
Gradle would detect that to run the project, we need to use module-path and that would fail once we start Java 8. It would be nicer if Gradle realized the configured JVM was incompatible before even trying to start it. But that's a problem for another day.
Thank you so much for the follow-up! I was also struggling with JVM detection and "guessed" some approach. Nice workaround with |
Fixes #34003
(Duplicate issue #34226)
Context
./gradlew :jabgui:rundoes not work at two persons of our team.Contributor Checklist
<subproject>/src/integTest) to verify changes from a user perspective.<subproject>/src/test) to verify logic../gradlew sanityCheck../gradlew <changed-subproject>:quickTest.Reviewing cheatsheet
Before merging the PR, comments starting with