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

Build: Forbiddenapis is not running for Java9 SourceSet #29292

Closed
uschindler opened this issue Mar 29, 2018 · 9 comments
Closed

Build: Forbiddenapis is not running for Java9 SourceSet #29292

uschindler opened this issue Mar 29, 2018 · 9 comments
Labels
:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team

Comments

@uschindler
Copy link
Contributor

uschindler commented Mar 29, 2018

Due to a bug in forbiddenapis the forbidden tasks are not enabled for the sourceSet named "java9" in the server module.

The corresponding bug is: policeman-tools/forbidden-apis#138

See also discussion on #29289

I fixed the bug locally and it was working now, the only problem is setup of signatures files for the "java9" sourceSet: We do not have any dependencies, so parsing the default signatures fails:

:server:java9Classes (Thread[Task worker for ':' Thread 3,5,main]) completed. Took 0.0 secs.
:server:forbiddenApisJava9 (Thread[Task worker for ':' Thread 3,5,main]) started.

> Task :server:forbiddenApisJava9 FAILED
Task ':server:forbiddenApisJava9' is not up-to-date because:
  Task has failed previously.
Reading bundled API signatures: jdk-unsafe-1.8
Reading bundled API signatures: jdk-deprecated-1.8
Reading bundled API signatures: jdk-non-portable
Reading bundled API signatures: jdk-system-out
Reading API signatures: jar:file:/C:/Users/Uwe%20Schindler/.gradle/caches/jars-3/f2f51995af2698f84880e96f412cac8d/buildSrc-7.0.0-alpha1-SNAPSHOT.jar!/forbidden/jdk-signatures.txt
Reading API signatures: jar:file:/C:/Users/Uwe%20Schindler/.gradle/caches/jars-3/f2f51995af2698f84880e96f412cac8d/buildSrc-7.0.0-alpha1-SNAPSHOT.jar!/forbidden/es-all-signatures.txt

:server:forbiddenApisJava9 (Thread[Task worker for ':' Thread 3,5,main]) completed. Took 0.538 secs.

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':server:forbiddenApisJava9'.
> Parsing signatures failed: Class 'org.apache.lucene.util.IOUtils' not found on classpath while parsing signature: org.apache.lucene.util.IOUtils

This has to be fixed once forbiddenApis is updated:

  • set targetCompatibility (like done for the compile task). Maybe there is a more generic solution in the build plugin.
  • add missing dependencies to compile classpath, or alternatively remove the signatures that don't work.
@uschindler uschindler changed the title Forbiddenapis is not ran for Java9 SourceSet Forbiddenapis is not running for Java9 SourceSet Mar 29, 2018
@uschindler uschindler changed the title Forbiddenapis is not running for Java9 SourceSet Build: Forbiddenapis is not running for Java9 SourceSet Mar 29, 2018
@uschindler
Copy link
Contributor Author

uschindler commented Mar 29, 2018

The bug was fixed in forbiddenapis. Should come with version 2.6.

Before releasing that version, I'd like to put some more effort in silencing the warnings when parsing signatures where the classes are not on classpath.

@jasontedor jasontedor added the :Delivery/Build Build or test infrastructure label Mar 29, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@uschindler
Copy link
Contributor Author

uschindler commented Mar 29, 2018

Hi @jasontedor,
I figured out how to setup the Java9 sourceset correctly. It works with the actual setup (without forbiddenapis bug fix) only, because the only class that is compiled has no external dependencies and works with compiling against JDK9 runtime only. However, if you create a MR-JAR, the correct compilation setup is:

  • The code in Java9 has to be compiled after the main classes. The reason: The Java9 specific code may refer to other classes from the output of main sourceset. This can be done by adding a dependency to the main's sourceSet output. This is optional for the current case, as the getPid stuff is self-contained.
  • The Java9 sourceset must have the same compile classpath like the main source set, so it also gets the dependencies. This is important for forbiddenapis to work.

I did this locally like this:

diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle
index 5256968..f6d5fc6 100644
--- a/buildSrc/build.gradle
+++ b/buildSrc/build.gradle
@@ -94,7 +94,7 @@ dependencies {
   compile 'com.netflix.nebula:gradle-info-plugin:3.0.3'
   compile 'org.eclipse.jgit:org.eclipse.jgit:3.2.0.201312181205-r'
   compile 'com.perforce:p4java:2012.3.551082' // THIS IS SUPPOSED TO BE OPTIONAL IN THE FUTURE....
-  compile 'de.thetaphi:forbiddenapis:2.5'
+  compile 'de.thetaphi:forbiddenapis:2.6-SNAPSHOT'
   compile 'org.apache.rat:apache-rat:0.11'
   compile "org.elasticsearch:jna:4.5.1"
 }
diff --git a/server/build.gradle b/server/build.gradle
index 7b30f57..3eea0cf 100644
--- a/server/build.gradle
+++ b/server/build.gradle
@@ -43,13 +43,22 @@ if (!isEclipse && !isIdea) {
       java {
         srcDirs = ['src/main/java9']
       }
+      compileClasspath += sourceSets.main.compileClasspath
     }
   }
+
+  dependencies {
+    java9Compile sourceSets.main.output
+  }

   compileJava9Java {
     sourceCompatibility = 9
     targetCompatibility = 9
   }
+
+  forbiddenApisJava9 {
+    targetCompatibility = 9
+  }

   jar {
     into('META-INF/versions/9') {

This makes the build work - as said before the extra dependency to main's output is optional, but may be required soon:

> Task :server:java9Classes
Skipping task ':server:java9Classes' as it has no actions.

:server:java9Classes (Thread[Task worker for ':' Thread 3,5,main]) completed. Took 0.0 secs.
:server:forbiddenApisJava9 (Thread[Task worker for ':' Thread 3,5,main]) started.

> Task :server:forbiddenApisJava9
Task ':server:forbiddenApisJava9' is not up-to-date because:
  Input property 'classpath' file C:\Users\Uwe Schindler\Projects\lucene\es\elasticsearch\server\cli\build\distributions\elasticsearch-cli-7.0.0-alpha1-SNAPSHOT.jar has changed.
Reading bundled API signatures: jdk-unsafe-9
Reading bundled API signatures: jdk-deprecated-9
Reading bundled API signatures: jdk-non-portable
Reading bundled API signatures: jdk-system-out
Reading API signatures: jar:file:/C:/Users/Uwe%20Schindler/.gradle/caches/jars-3/f2f51995af2698f84880e96f412cac8d/buildSrc-7.0.0-alpha1-SNAPSHOT.jar!/forbidden/jdk-signatures.txt
Reading API signatures: jar:file:/C:/Users/Uwe%20Schindler/.gradle/caches/jars-3/f2f51995af2698f84880e96f412cac8d/buildSrc-7.0.0-alpha1-SNAPSHOT.jar!/forbidden/es-all-signatures.txt
Loading classes to check...
Scanning classes for violations...
Scanned 1 class file(s) for forbidden API invocations (in 0.26s), 0 error(s).

:server:forbiddenApisJava9 (Thread[Task worker for ':' Thread 3,5,main]) completed. Took 0.618 secs.
:server:forbiddenApisMain (Thread[Task worker for ':' Thread 3,5,main]) started.

> Task :server:forbiddenApisMain
Task ':server:forbiddenApisMain' is not up-to-date because:
  Input property 'classpath' file C:\Users\Uwe Schindler\Projects\lucene\es\elasticsearch\server\cli\build\distributions\elasticsearch-cli-7.0.0-alpha1-SNAPSHOT.jar has changed.
Reading bundled API signatures: jdk-unsafe-1.8
Reading bundled API signatures: jdk-deprecated-1.8
Reading bundled API signatures: jdk-non-portable
Reading bundled API signatures: jdk-system-out
Reading API signatures: jar:file:/C:/Users/Uwe%20Schindler/.gradle/caches/jars-3/f2f51995af2698f84880e96f412cac8d/buildSrc-7.0.0-alpha1-SNAPSHOT.jar!/forbidden/jdk-signatures.txt
Reading API signatures: jar:file:/C:/Users/Uwe%20Schindler/.gradle/caches/jars-3/f2f51995af2698f84880e96f412cac8d/buildSrc-7.0.0-alpha1-SNAPSHOT.jar!/forbidden/es-all-signatures.txt
Reading API signatures: jar:file:/C:/Users/Uwe%20Schindler/.gradle/caches/jars-3/f2f51995af2698f84880e96f412cac8d/buildSrc-7.0.0-alpha1-SNAPSHOT.jar!/forbidden/es-server-signatures.txt
Loading classes to check...
Scanning classes for violations...
Scanned 5538 class file(s) for forbidden API invocations (in 8.47s), 0 error(s).

:server:forbiddenApisMain (Thread[Task worker for ':' Thread 3,5,main]) completed. Took 9.193 secs.

As you see, it also scans the Java 9 classes with correct bundled signatures.

@uschindler
Copy link
Contributor Author

uschindler commented Mar 29, 2018

I improved the Java9 config a bit more. Now its expressed without hacks using configurations and inheritance:

// we want to keep the JDKs in our IDEs set to JDK 8 until minimum JDK is bumped to 9 so we do not include this source set in our IDEs
if (!isEclipse && !isIdea) {
  sourceSets {
    java9 {
      java {
        srcDirs = ['src/main/java9']
      }
    }
  }
  
  configurations {
    java9Compile.extendsFrom(compile)
  }
  
  dependencies {
    java9Compile sourceSets.main.output
  }

  compileJava9Java {
    sourceCompatibility = 9
    targetCompatibility = 9
  }
  
  forbiddenApisJava9 {
    targetCompatibility = 9
  }

  jar {
    metaInf {
      into 'versions/9'
      from sourceSets.java9.output
    }
    manifest.attributes('Multi-Release': 'true')
  }
}

This changes:

  • each sourceSet by default defines a configuration. To not add all dependencies a second time or use a compileClassPath hack, we just make this new configuration extend the compile configuration.
  • we also add the main's output as dependency (this is still optional, but should be done to be future-proof and to mimic what MR-JARs are doing)
  • I also used the JAR tasks metaInf config closure, so we don't need to hardcode the path name. This is the recommended way to add stuff to META-INF in Gradle. I found another place (BuildPlugin) where the licenses are added, should look the same (@rjernst).

I think this can already applied to the Java 9 build, just the forbiddenApisJava9 part commented out, until the bug is fixed.

@uschindler
Copy link
Contributor Author

Hi: Here is the diff of my whole work for correctly setting up the Java9 source sets. It also fixes the issues with the delayed project evaluation that @jasontedor tried to fix after the builds failed. This now uses projects.tasks.withType() to fix this completely and does not rely on task names.

master...uschindler:issues/29292

I can create a PR that does not yet contain the forbiddenapis update if you want to apply it for fixing the bugs in the Java 9 setup.

@jasontedor
Copy link
Member

Please do @uschindler.

uschindler added a commit to uschindler/elasticsearch that referenced this issue Mar 30, 2018
…nd fix checkstyle task that was partly broken because delayed setup of Java9 sourcesets. This also cleans packaging of META-INF. It also prepares forbiddenapis 2.6 upgrade (see elastic#29292)
@uschindler
Copy link
Contributor Author

Here is the PR: #29312

rjernst pushed a commit that referenced this issue Apr 3, 2018
Correctly setup classpath/dependencies and fix checkstyle task that was partly broken because delayed setup of Java9 sourcesets. This also cleans packaging of META-INF. It also prepares forbiddenapis 2.6 upgrade

relates #29292
rjernst pushed a commit that referenced this issue Apr 3, 2018
Correctly setup classpath/dependencies and fix checkstyle task that was partly broken because delayed setup of Java9 sourcesets. This also cleans packaging of META-INF. It also prepares forbiddenapis 2.6 upgrade

relates #29292
@rjernst
Copy link
Member

rjernst commented Apr 18, 2018

Fixed by #29312

@rjernst rjernst closed this as completed Apr 18, 2018
@uschindler
Copy link
Contributor Author

Hi, thanks @rjernst! I had unfortunately no time to release a new forbiddenapis version, but once this is done, I will send another PR - including adding the commented out config option.

@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
Projects
None yet
Development

No branches or pull requests

5 participants