honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Ignore module-info in jar hell checks (#33011) 

* Ignore module-info in JarHell checks
* Add unit test
* integration test to test that jarhell is ran with precommit
This commit is contained in:
Alpar Torok 2018-08-30 11:41:39 +03:00 committed by GitHub
parent 51cbc61135
commit 5cf6e0d4bc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 149 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
buildSrc/build.gradle
View File

@ -176,7 +176,6 @@ if (project != rootProject) {
it.tasks.matching { it.name == 'publishNebulaPublicationToLocalTestRepository'}
}
exclude "**/*Tests.class"
include "**/*IT.class"
testClassesDirs = sourceSets.test.output.classesDirs
classpath = sourceSets.test.runtimeClasspath
inputs.dir(file("src/testKit"))

8
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/JarHellTask.groovy
View File

@ -22,8 +22,8 @@ package org.elasticsearch.gradle.precommit
import com.github.jengelman.gradle.plugins.shadow.ShadowPlugin
import org.elasticsearch.gradle.LoggedExec
import org.gradle.api.file.FileCollection
import org.gradle.api.tasks.Classpath
import org.gradle.api.tasks.OutputFile
/**
* Runs CheckJarHell on a classpath.
*/
@ -35,9 +35,13 @@ public class JarHellTask extends LoggedExec {
* inputs (ie the jars/class files).
*/
@OutputFile
File successMarker = new File(project.buildDir, 'markers/jarHell')
File successMarker
@Classpath
FileCollection classpath
public JarHellTask() {
successMarker = new File(project.buildDir, 'markers/jarHell-' + getName())
project.afterEvaluate {
FileCollection classpath = project.sourceSets.test.runtimeClasspath
if (project.plugins.hasPlugin(ShadowPlugin)) {

10
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
View File

@ -31,7 +31,7 @@ class PrecommitTasks {
/** Adds a precommit task, which depends on non-test verification tasks. */
public static Task create(Project project, boolean includeDependencyLicenses) {
Configuration forbiddenApisConfiguration = project.configurations.create("forbiddenApisCliJar")
project.configurations.create("forbiddenApisCliJar")
project.dependencies {
forbiddenApisCliJar ('de.thetaphi:forbiddenapis:2.5')
}
@ -43,7 +43,7 @@ class PrecommitTasks {
project.tasks.create('forbiddenPatterns', ForbiddenPatternsTask.class),
project.tasks.create('licenseHeaders', LicenseHeadersTask.class),
project.tasks.create('filepermissions', FilePermissionsTask.class),
project.tasks.create('jarHell', JarHellTask.class),
configureJarHell(project),
configureThirdPartyAudit(project)
]
@ -80,6 +80,12 @@ class PrecommitTasks {
return project.tasks.create(precommitOptions)
}
private static Task configureJarHell(Project project) {
Task task = project.tasks.create('jarHell', JarHellTask.class)
task.classpath = project.sourceSets.test.runtimeClasspath
return task
}
private static Task configureThirdPartyAudit(Project project) {
ThirdPartyAuditTask thirdPartyAuditTask = project.tasks.create('thirdPartyAudit', ThirdPartyAuditTask.class)
ExportElasticsearchBuildResourcesTask buildResources = project.tasks.getByName('buildResources')

13
buildSrc/src/test/java/org/elasticsearch/gradle/BuildExamplePluginsIT.java
View File

@ -153,17 +153,4 @@ public class BuildExamplePluginsIT extends GradleIntegrationTestCase {
}
}
private String getLocalTestRepoPath() {
String property = System.getProperty("test.local-test-repo-path");
Objects.requireNonNull(property, "test.local-test-repo-path not passed to tests");
File file = new File(property);
assertTrue("Expected " + property + " to exist, but it did not!", file.exists());
if (File.separator.equals("\\")) {
// Use / on Windows too, the build script is not happy with \
return file.getAbsolutePath().replace(File.separator, "/");
} else {
return file.getAbsolutePath();
}
}
}

8
buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java
View File

@ -40,7 +40,7 @@ public class ExportElasticsearchBuildResourcesTaskIT extends GradleIntegrationTe
.withArguments("buildResources", "-s", "-i")
.withPluginClasspath()
.build();
assertTaskSuccessfull(result, ":buildResources");
assertTaskSuccessful(result, ":buildResources");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml");
@ -61,8 +61,8 @@ public class ExportElasticsearchBuildResourcesTaskIT extends GradleIntegrationTe
.withPluginClasspath()
.build();
assertTaskSuccessfull(result, ":buildResources");
assertTaskSuccessfull(result, ":sampleCopyAll");
assertTaskSuccessful(result, ":buildResources");
assertTaskSuccessful(result, ":sampleCopyAll");
assertBuildFileExists(result, PROJECT_NAME, "sampleCopyAll/checkstyle.xml");
// This is a side effect of compile time reference
assertBuildFileExists(result, PROJECT_NAME, "sampleCopyAll/checkstyle_suppressions.xml");
@ -75,7 +75,7 @@ public class ExportElasticsearchBuildResourcesTaskIT extends GradleIntegrationTe
.withPluginClasspath()
.build();
assertTaskSuccessfull(result, ":sample");
assertTaskSuccessful(result, ":sample");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml");
}

42
buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java Normal file
View File

@ -0,0 +1,42 @@
package org.elasticsearch.gradle.precommit;
import org.elasticsearch.gradle.test.GradleIntegrationTestCase;
import org.gradle.testkit.runner.BuildResult;
import org.gradle.testkit.runner.GradleRunner;
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
public class JarHellTaskIT extends GradleIntegrationTestCase {
public void testJarHellDetected() {
BuildResult result = GradleRunner.create()
.withProjectDir(getProjectDir("jarHell"))
.withArguments("clean", "precommit", "-s", "-Dlocal.repo.path=" + getLocalTestRepoPath())
.withPluginClasspath()
.buildAndFail();
assertTaskFailed(result, ":jarHell");
assertOutputContains(
result.getOutput(),
"Exception in thread \"main\" java.lang.IllegalStateException: jar hell!",
"class: org.apache.logging.log4j.Logger"
);
}
}

31
buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java
View File

@ -9,6 +9,7 @@ import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@ -66,15 +67,24 @@ public abstract class GradleIntegrationTestCase extends GradleUnitTestCase {
}
}
protected void assertTaskSuccessfull(BuildResult result, String taskName) {
protected void assertTaskFailed(BuildResult result, String taskName) {
assertTaskOutcome(result, taskName, TaskOutcome.FAILED);
}
protected void assertTaskSuccessful(BuildResult result, String taskName) {
assertTaskOutcome(result, taskName, TaskOutcome.SUCCESS);
}
private void assertTaskOutcome(BuildResult result, String taskName, TaskOutcome taskOutcome) {
BuildTask task = result.task(taskName);
if (task == null) {
fail("Expected task `" + taskName + "` to be successful, but it did not run");
fail("Expected task `" + taskName + "` to be " + taskOutcome +", but it did not run" +
"\n\nOutput is:\n" + result.getOutput());
}
assertEquals(
"Expected task to be successful but it was: " + task.getOutcome() +
"\n\nOutput is:\n" + result.getOutput() ,
TaskOutcome.SUCCESS,
taskOutcome + "\n\nOutput is:\n" + result.getOutput() ,
taskOutcome,
task.getOutcome()
);
}
@ -109,4 +119,17 @@ public abstract class GradleIntegrationTestCase extends GradleUnitTestCase {
Files.exists(absPath)
);
}
protected String getLocalTestRepoPath() {
String property = System.getProperty("test.local-test-repo-path");
Objects.requireNonNull(property, "test.local-test-repo-path not passed to tests");
File file = new File(property);
assertTrue("Expected " + property + " to exist, but it did not!", file.exists());
if (File.separator.equals("\\")) {
// Use / on Windows too, the build script is not happy with \
return file.getAbsolutePath().replace(File.separator, "/");
} else {
return file.getAbsolutePath();
}
}
}

29
buildSrc/src/testKit/jarHell/build.gradle Normal file
View File

@ -0,0 +1,29 @@
plugins {
id 'java'
id 'elasticsearch.build'
}
dependencyLicenses.enabled = false
dependenciesInfo.enabled = false
forbiddenApisMain.enabled = false
forbiddenApisTest.enabled = false
thirdPartyAudit.enabled = false
namingConventions.enabled = false
ext.licenseFile = file("$buildDir/dummy/license")
ext.noticeFile = file("$buildDir/dummy/notice")
repositories {
mavenCentral()
repositories {
maven {
url System.getProperty("local.repo.path")
}
}
}
dependencies {
// Needed for the JarHell task
testCompile ("org.elasticsearch.test:framework:${versions.elasticsearch}")
// causes jar hell with local sources
compile "org.apache.logging.log4j:log4j-api:${versions.log4j}"
}

7
buildSrc/src/testKit/jarHell/src/main/java/org/apache/logging/log4j/Logger.java Normal file
View File

@ -0,0 +1,7 @@
package org.apache.logging.log4j;
// Jar Hell !
public class Logger {
}

4
libs/core/src/main/java/org/elasticsearch/bootstrap/JarHell.java
View File

@ -255,6 +255,10 @@ public class JarHell {
}
private static void checkClass(Map<String, Path> clazzes, String clazz, Path jarpath) {
if (clazz.equals("module-info") || clazz.endsWith(".module-info")) {
// Ignore jigsaw module descriptions
return;
}
Path previous = clazzes.put(clazz, jarpath);
if (previous != null) {
if (previous.equals(jarpath)) {

22
libs/core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java
View File

@ -76,6 +76,28 @@ public class JarHellTests extends ESTestCase {
}
}
public void testModuleInfo() throws Exception {
Path dir = createTempDir();
JarHell.checkJarHell(
asSet(
makeJar(dir, "foo.jar", null, "module-info.class"),
makeJar(dir, "bar.jar", null, "module-info.class")
),
logger::debug
);
}
public void testModuleInfoPackage() throws Exception {
Path dir = createTempDir();
JarHell.checkJarHell(
asSet(
makeJar(dir, "foo.jar", null, "foo/bar/module-info.class"),
makeJar(dir, "bar.jar", null, "foo/bar/module-info.class")
),
logger::debug
);
}
public void testDirsOnClasspath() throws Exception {
Path dir1 = createTempDir();
Path dir2 = createTempDir();