Skip to content

Conversation

@ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Jul 24, 2017

With Gradle 4.1 and newer JDK versions, we can finally invoke Gradle directly using a JDK9 JAVA_HOME without requiring a JDK8 to "bootstrap" the build. As the thirdPartyAudit task runs within the JVM that Gradle runs in, it needs to be adapted now to be JDK9 aware.

I've tested this PR with Gradle 4.1-rc-1 and JDK9 b178.

Note that this also requires a small backport to the 5.x branch as the build checks out the 5.x branch and invokes a build of that version.

@ywelsch ywelsch added :Delivery/Build Build or test infrastructure >enhancement v6.0.0 labels Jul 24, 2017
@ywelsch ywelsch requested review from nik9000 and rjernst and removed request for rjernst July 24, 2017 12:06
static void configureCompile(Project project) {
project.ext.compactProfile = 'compact3'
project.afterEvaluate {
// fail on all javac warnings
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this comment? I think it has drifted a long way from its home.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found the right place for it again a few lines below: 62a0717

'javax.xml.bind.util.ValidationEventCollector'
]
} else {
thirdPartyAudit.excludes += [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth a comment that these were added to the jre in 9 but we still need to include the jar because we expect to run in 8.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

options.forkOptions.memoryMaximumSize = "1g"
File gradleJavaHome = Jvm.current().javaHome
if (new File(project.javaHome).canonicalPath != gradleJavaHome.canonicalPath) {
options.fork = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that we used to have to fork for java 9 all because gradle ran in java 8, but I think we can still fork if we want to. Do we no longer want to fork?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you said, this extra forking logic was introduced to deal with Java 9 compatibility issues in Gradle. It was overly broad, however, and we should not just fork processes for the fun of it. If, hopefully in the near future, we would require Gradle 4.1 as minimum version for running the build with Java 9, we could remove all this extra logic for distinguishing between the JVM that runs Gradle and the JVM that compiles / runs the tests..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I'm fine with removing the fork. I think it is worth updating the commit message to make it clear that we no longer fork all the time, just if needed.

@ywelsch ywelsch merged commit efd7988 into elastic:master Jul 27, 2017
@ywelsch
Copy link
Contributor Author

ywelsch commented Jul 27, 2017

Thanks @nik9000

ywelsch added a commit that referenced this pull request Jul 27, 2017
With Gradle 4.1 and newer JDK versions, we can finally invoke Gradle directly using a JDK9 JAVA_HOME without requiring a JDK8 to "bootstrap" the build. As the thirdPartyAudit task runs within the JVM that Gradle runs in, it needs to be adapted now to be JDK9 aware.

This commit also changes the `JavaCompile` tasks to only fork if necessary (i.e. when Gradle's JVM and JAVA_HOME's JVM differ).
ywelsch added a commit that referenced this pull request Jul 27, 2017
With Gradle 4.1 and newer JDK versions, we can finally invoke Gradle directly using a JDK9 JAVA_HOME without requiring a JDK8 to "bootstrap" the build. As the thirdPartyAudit task runs within the JVM that Gradle runs in, it needs to be adapted now to be JDK9 aware.

This commit also changes the `JavaCompile` tasks to only fork if necessary (i.e. when Gradle's JVM and JAVA_HOME's JVM differ).
ywelsch added a commit that referenced this pull request Jul 27, 2017
With Gradle 4.1 and newer JDK versions, we can finally invoke Gradle directly using a JDK9 JAVA_HOME without requiring a JDK8 to "bootstrap" the build. As the thirdPartyAudit task runs within the JVM that Gradle runs in, it needs to be adapted now to be JDK9 aware.

This commit also changes the `JavaCompile` tasks to only fork if necessary (i.e. when Gradle's JVM and JAVA_HOME's JVM differ).
@colings86 colings86 added v5.6.0 and removed v5.6.1 labels Aug 4, 2017
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v5.6.0 v6.0.0-beta1 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants