Build: Cleanup precommit task gradle code

This change attempts to simplify the gradle tasks for precommit. One
major part of that is using a "less groovy style", as well as being more
consistent about how tasks are created and where they are configured. It
also allows the things creating the tasks to set up inter task
dependencies, instead of assuming them (ie decoupling from tasks
eleswhere in the build).
This commit is contained in:
Ryan Ernst 2015-12-01 17:08:27 -08:00
parent 9053c9a002
commit d68c6673a2
12 changed files with 269 additions and 140 deletions

View File

@ -62,7 +62,7 @@ class BuildPlugin implements Plugin<Project> {
configureCompile(project)
configureTest(project)
PrecommitTasks.configure(project)
configurePrecommit(project)
}
/** Performs checks on the build environment and prints information about the build environment. */
@ -416,4 +416,11 @@ class BuildPlugin implements Plugin<Project> {
}
return test
}
private static configurePrecommit(Project project) {
Task precommit = PrecommitTasks.create(project, true)
project.check.dependsOn(precommit)
project.test.mustRunAfter(precommit)
project.dependencyLicenses.dependencies = project.configurations.runtime - project.configurations.provided
}
}

View File

@ -18,63 +18,100 @@
*/
package org.elasticsearch.gradle.precommit
import org.gradle.api.DefaultTask
import org.gradle.api.GradleException
import org.gradle.api.InvalidUserDataException
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.*
import org.gradle.api.file.FileCollection
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputDirectory
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.StopActionException
import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.VerificationTask
import java.nio.file.Files
import java.security.MessageDigest
import java.util.regex.Matcher
import java.util.regex.Pattern
class DependencyLicensesTask extends DefaultTask {
static final String SHA_EXTENSION = '.sha1'
static Task configure(Project project, Closure closure) {
DependencyLicensesTask task = project.tasks.create(type: DependencyLicensesTask, name: 'dependencyLicenses')
UpdateShasTask update = project.tasks.create(type: UpdateShasTask, name: 'updateShas')
update.parentTask = task
task.configure(closure)
project.check.dependsOn(task)
return task
}
/**
* A task to check licenses for dependencies.
*
* There are two parts to the check:
* <ul>
* <li>LICENSE and NOTICE files</li>
* <li>SHA checksums for each dependency jar</li>
* </ul>
*
* The directory to find the license and sha files in defaults to the dir @{code licenses}
* in the project directory for this task. You can override this directory:
* <pre>
* dependencyLicenses {
* licensesDir = project.file('mybetterlicensedir')
* }
* </pre>
*
* The jar files to check default to the dependencies from the default configuration. You
* can override this, for example, to only check compile dependencies:
* <pre>
* dependencyLicenses {
* dependencies = project.configurations.compile
* }
* </pre>
*
* Every jar must have a {@code .sha1} file in the licenses dir. These can be managed
* automatically using the {@code updateShas} helper task that is created along
* with this task. It will add {@code .sha1} files for new jars that are in dependencies
* and remove old {@code .sha1} files that are no longer needed.
*
* Every jar must also have a LICENSE and NOTICE file. However, multiple jars can share
* LICENSE and NOTICE files by mapping a pattern to the same name.
* <pre>
* dependencyLicenses {
* mapping from: &#47;lucene-.*&#47;, to: 'lucene'
* }
* </pre>
*/
public class DependencyLicensesTask extends DefaultTask {
private static final String SHA_EXTENSION = '.sha1'
// TODO: we should be able to default this to eg compile deps, but we need to move the licenses
// check from distribution to core (ie this should only be run on java projects)
/** A collection of jar files that should be checked. */
@InputFiles
FileCollection dependencies
public FileCollection dependencies
/** The directory to find the license and sha files in. */
@InputDirectory
File licensesDir = new File(project.projectDir, 'licenses')
public File licensesDir = new File(project.projectDir, 'licenses')
LinkedHashMap<String, String> mappings = new LinkedHashMap<>()
/** A map of patterns to prefix, used to find the LICENSE and NOTICE file. */
private LinkedHashMap<String, String> mappings = new LinkedHashMap<>()
/**
* Add a mapping from a regex pattern for the jar name, to a prefix to find
* the LICENSE and NOTICE file for that jar.
*/
@Input
void mapping(Map<String, String> props) {
String from = props.get('from')
public void mapping(Map<String, String> props) {
String from = props.remove('from')
if (from == null) {
throw new InvalidUserDataException('Missing "from" setting for license name mapping')
}
String to = props.get('to')
String to = props.remove('to')
if (to == null) {
throw new InvalidUserDataException('Missing "to" setting for license name mapping')
}
if (props.isEmpty() == false) {
throw new InvalidUserDataException("Unknown properties for mapping on dependencyLicenses: ${props.keySet()}")
}
mappings.put(from, to)
}
@TaskAction
void checkDependencies() {
// TODO: empty license dir (or error when dir exists and no deps)
public void checkDependencies() {
if (licensesDir.exists() == false && dependencies.isEmpty() == false) {
throw new GradleException("Licences dir ${licensesDir} does not exist, but there are dependencies")
}
if (licensesDir.exists() && dependencies.isEmpty()) {
throw new GradleException("Licenses dir ${licensesDir} exists, but there are no dependencies")
}
// order is the same for keys and values iteration since we use a linked hashmap
List<String> mapped = new ArrayList<>(mappings.values())
@ -127,7 +164,7 @@ class DependencyLicensesTask extends DefaultTask {
}
}
void checkSha(File jar, String jarName, Set<File> shaFiles) {
private void checkSha(File jar, String jarName, Set<File> shaFiles) {
File shaFile = new File(licensesDir, jarName + SHA_EXTENSION)
if (shaFile.exists() == false) {
throw new GradleException("Missing SHA for ${jarName}. Run 'gradle updateSHAs' to create")
@ -143,7 +180,7 @@ class DependencyLicensesTask extends DefaultTask {
shaFiles.remove(shaFile)
}
void checkFile(String name, String jarName, Map<String, Integer> counters, String type) {
private void checkFile(String name, String jarName, Map<String, Integer> counters, String type) {
String fileName = "${name}-${type}"
Integer count = counters.get(fileName)
if (count == null) {
@ -158,10 +195,12 @@ class DependencyLicensesTask extends DefaultTask {
counters.put(fileName, count + 1)
}
static class UpdateShasTask extends DefaultTask {
DependencyLicensesTask parentTask
/** A helper task to update the sha files in the license dir. */
public static class UpdateShasTask extends DefaultTask {
private DependencyLicensesTask parentTask
@TaskAction
void updateShas() {
public void updateShas() {
Set<File> shaFiles = new HashSet<File>()
parentTask.licensesDir.eachFile {
String name = it.getName()

View File

@ -19,10 +19,11 @@
package org.elasticsearch.gradle.precommit
import org.gradle.api.DefaultTask
import org.gradle.api.GradleException
import org.gradle.api.InvalidUserDataException
import org.gradle.api.file.FileCollection
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.OutputFile
import org.gradle.api.tasks.OutputFiles
import org.gradle.api.tasks.SourceSet
import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.util.PatternFilterable
@ -33,14 +34,19 @@ import java.util.regex.Pattern
/**
* Checks for patterns in source files for the project which are forbidden.
*/
class ForbiddenPatternsTask extends DefaultTask {
Map<String,String> patterns = new LinkedHashMap<>()
PatternFilterable filesFilter = new PatternSet()
public class ForbiddenPatternsTask extends DefaultTask {
/** The rules: a map from the rule name, to a rule regex pattern. */
private Map<String,String> patterns = new LinkedHashMap<>()
/** A pattern set of which files should be checked. */
private PatternFilterable filesFilter = new PatternSet()
@OutputFile
File outputMarker = new File(project.buildDir, "markers/forbiddenPatterns")
ForbiddenPatternsTask() {
public ForbiddenPatternsTask() {
description = 'Checks source files for invalid patterns like nocommits or tabs'
// we always include all source files, and exclude what should not be checked
filesFilter.include('**')
// exclude known binary extensions
@ -52,23 +58,28 @@ class ForbiddenPatternsTask extends DefaultTask {
filesFilter.exclude('**/*.crt')
filesFilter.exclude('**/*.png')
// TODO: add compile and test compile outputs as this tasks outputs, so we don't rerun when source files haven't changed
// add mandatory rules
patterns.put('nocommit', /nocommit/)
patterns.put('tab', /\t/)
}
/** Adds a file glob pattern to be excluded */
void exclude(String... excludes) {
public void exclude(String... excludes) {
this.filesFilter.exclude(excludes)
}
/** Adds pattern to forbid */
/** Adds a pattern to forbid. T */
void rule(Map<String,String> props) {
String name = props.get('name')
String name = props.remove('name')
if (name == null) {
throw new IllegalArgumentException('Missing [name] for invalid pattern rule')
throw new InvalidUserDataException('Missing [name] for invalid pattern rule')
}
String pattern = props.get('pattern')
String pattern = props.remove('pattern')
if (pattern == null) {
throw new IllegalArgumentException('Missing [pattern] for invalid pattern rule')
throw new InvalidUserDataException('Missing [pattern] for invalid pattern rule')
}
if (props.isEmpty() == false) {
throw new InvalidUserDataException("Unknown arguments for ForbiddenPatterns rule mapping: ${props.keySet()}")
}
// TODO: fail if pattern contains a newline, it won't work (currently)
patterns.put(name, pattern)
@ -89,14 +100,14 @@ class ForbiddenPatternsTask extends DefaultTask {
Pattern allPatterns = Pattern.compile('(' + patterns.values().join(')|(') + ')')
List<String> failures = new ArrayList<>()
for (File f : files()) {
f.eachLine('UTF-8') { line, lineNumber ->
f.eachLine('UTF-8') { String line, int lineNumber ->
if (allPatterns.matcher(line).find()) {
addErrorMessages(failures, f, (String)line, (int)lineNumber)
addErrorMessages(failures, f, line, lineNumber)
}
}
}
if (failures.isEmpty() == false) {
throw new IllegalArgumentException('Found invalid patterns:\n' + failures.join('\n'))
throw new GradleException('Found invalid patterns:\n' + failures.join('\n'))
}
outputMarker.setText('done', 'UTF-8')
}

View File

@ -0,0 +1,62 @@
/*
* 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.
*/
package org.elasticsearch.gradle.precommit
import org.elasticsearch.gradle.LoggedExec
import org.gradle.api.file.FileCollection
import org.gradle.api.tasks.InputFile
import org.gradle.api.tasks.OutputFile
/**
* Runs CheckJarHell on a classpath.
*/
public class JarHellTask extends LoggedExec {
/**
* We use a simple "marker" file that we touch when the task succeeds
* as the task output. This is compared against the modified time of the
* inputs (ie the jars/class files).
*/
@OutputFile
public File successMarker = new File(project.buildDir, 'markers/jarHell')
/** The classpath to run jarhell check on, defaults to the test runtime classpath */
@InputFile
public FileCollection classpath = project.sourceSets.test.runtimeClasspath
public JarHellTask() {
project.afterEvaluate {
dependsOn(classpath)
description = "Runs CheckJarHell on ${classpath}"
executable = new File(project.javaHome, 'bin/java')
doFirst({
/* JarHell doesn't like getting directories that don't exist but
gradle isn't especially careful about that. So we have to do it
filter it ourselves. */
FileCollection taskClasspath = classpath.filter { it.exists() }
args('-cp', taskClasspath.asPath, 'org.elasticsearch.bootstrap.JarHell')
})
doLast({
successMarker.parentFile.mkdirs()
successMarker.setText("", 'UTF-8')
})
}
}
}

View File

@ -18,16 +18,10 @@
*/
package org.elasticsearch.gradle.precommit
import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApis
import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApisExtension
import de.thetaphi.forbiddenapis.gradle.ForbiddenApisPlugin
import org.gradle.api.GradleException
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.file.FileCollection
import org.gradle.api.plugins.JavaBasePlugin
import org.gradle.api.tasks.Exec
import org.gradle.api.tasks.TaskContainer
/**
* Validation tasks which should be run before committing. These run before tests.
@ -35,36 +29,34 @@ import org.gradle.api.tasks.TaskContainer
class PrecommitTasks {
/** Adds a precommit task, which depends on non-test verification tasks. */
static void configure(Project project) {
List precommitTasks = [
configureForbiddenApis(project),
configureForbiddenPatterns(project.tasks),
configureJarHell(project)]
public static Task create(Project project, boolean includeDependencyLicenses) {
Map precommitOptions = [
name: 'precommit',
group: JavaBasePlugin.VERIFICATION_GROUP,
description: 'Runs all non-test checks.',
dependsOn: precommitTasks
]
Task precommit = project.tasks.create(precommitOptions)
project.check.dependsOn(precommit)
List<Task> precommitTasks = [
configureForbiddenApis(project),
project.tasks.create('forbiddenPatterns', ForbiddenPatternsTask.class),
project.tasks.create('jarHell', JarHellTask.class)]
// delay ordering relative to test tasks, since they may not be setup yet
project.afterEvaluate {
Task test = project.tasks.findByName('test')
if (test != null) {
test.mustRunAfter(precommit)
}
Task integTest = project.tasks.findByName('integTest')
if (integTest != null) {
integTest.mustRunAfter(precommit)
}
// tasks with just tests don't need dependency licenses, so this flag makes adding
// the task optional
if (includeDependencyLicenses) {
DependencyLicensesTask dependencyLicenses = project.tasks.create('dependencyLicenses', DependencyLicensesTask.class)
precommitTasks.add(dependencyLicenses)
// we also create the updateShas helper task that is associated with dependencyLicenses
UpdateShasTask updateShas = project.tasks.create('updateShas', UpdateShasTask.class)
updateShas.parentTask = dependencyLicenses
}
Map<String, Object> precommitOptions = [
name: 'precommit',
group: JavaBasePlugin.VERIFICATION_GROUP,
description: 'Runs all non-test checks.',
dependsOn: precommitTasks
]
return project.tasks.create(precommitOptions)
}
static Task configureForbiddenApis(Project project) {
project.pluginManager.apply('de.thetaphi.forbiddenapis')
private static Task configureForbiddenApis(Project project) {
project.pluginManager.apply(ForbiddenApisPlugin.class)
project.forbiddenApis {
internalRuntimeForbidden = true
failOnUnsupportedJava = false
@ -75,7 +67,7 @@ class PrecommitTasks {
Task mainForbidden = project.tasks.findByName('forbiddenApisMain')
if (mainForbidden != null) {
mainForbidden.configure {
bundledSignatures += ['jdk-system-out']
bundledSignatures += 'jdk-system-out'
signaturesURLs += [
getClass().getResource('/forbidden/core-signatures.txt'),
getClass().getResource('/forbidden/third-party-signatures.txt')]
@ -84,63 +76,11 @@ class PrecommitTasks {
Task testForbidden = project.tasks.findByName('forbiddenApisTest')
if (testForbidden != null) {
testForbidden.configure {
signaturesURLs += [getClass().getResource('/forbidden/test-signatures.txt')]
signaturesURLs += getClass().getResource('/forbidden/test-signatures.txt')
}
}
Task forbiddenApis = project.tasks.findByName('forbiddenApis')
forbiddenApis.group = "" // clear group, so this does not show up under verification tasks
return forbiddenApis
}
static Task configureForbiddenPatterns(TaskContainer tasks) {
Map options = [
name: 'forbiddenPatterns',
type: ForbiddenPatternsTask,
description: 'Checks source files for invalid patterns like nocommits or tabs',
]
return tasks.create(options) {
rule name: 'nocommit', pattern: /nocommit/
rule name: 'tab', pattern: /\t/
}
}
/**
* Adds a task to run jar hell before on the test classpath.
*
* We use a simple "marker" file that we touch when the task succeeds
* as the task output. This is compared against the modified time of the
* inputs (ie the jars/class files).
*/
static Task configureJarHell(Project project) {
File successMarker = new File(project.buildDir, 'markers/jarHell')
Exec task = project.tasks.create(name: 'jarHell', type: Exec)
FileCollection testClasspath = project.sourceSets.test.runtimeClasspath
task.dependsOn(testClasspath)
task.inputs.files(testClasspath)
task.outputs.file(successMarker)
task.executable = new File(project.javaHome, 'bin/java')
task.doFirst({
/* JarHell doesn't like getting directories that don't exist but
gradle isn't especially careful about that. So we have to do it
filter it ourselves. */
def taskClasspath = testClasspath.filter { it.exists() }
task.args('-cp', taskClasspath.asPath, 'org.elasticsearch.bootstrap.JarHell')
})
if (task.logger.isInfoEnabled() == false) {
task.standardOutput = new ByteArrayOutputStream()
task.errorOutput = task.standardOutput
task.ignoreExitValue = true
task.doLast({
if (execResult.exitValue != 0) {
logger.error(standardOutput.toString())
throw new GradleException("JarHell failed")
}
})
}
task.doLast({
successMarker.parentFile.mkdirs()
successMarker.setText("", 'UTF-8')
})
return task
}
}

View File

@ -0,0 +1,65 @@
/*
* 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.
*/
package org.elasticsearch.gradle.precommit
import org.gradle.api.DefaultTask
import org.gradle.api.tasks.TaskAction
import java.nio.file.Files
import java.security.MessageDigest
/**
* A task to update shas used by {@code DependencyLicensesCheck}
*/
public class UpdateShasTask extends DefaultTask {
/** The parent dependency licenses task to use configuration from */
public DependencyLicensesTask parentTask
public UpdateShasTask() {
description = 'Updates the sha files for the dependencyLicenses check'
}
@TaskAction
public void updateShas() {
Set<File> shaFiles = new HashSet<File>()
parentTask.licensesDir.eachFile {
String name = it.getName()
if (name.endsWith(SHA_EXTENSION)) {
shaFiles.add(it)
}
}
for (File dependency : parentTask.dependencies) {
String jarName = dependency.getName()
File shaFile = new File(parentTask.licensesDir, jarName + SHA_EXTENSION)
if (shaFile.exists() == false) {
logger.lifecycle("Adding sha for ${jarName}")
String sha = MessageDigest.getInstance("SHA-1").digest(dependency.getBytes()).encodeHex().toString()
shaFile.setText(sha, 'UTF-8')
} else {
shaFiles.remove(shaFile)
}
}
shaFiles.each { shaFile ->
logger.lifecycle("Removing unused sha ${shaFile.getName()}")
Files.delete(shaFile.toPath())
}
}
}

View File

@ -58,6 +58,7 @@ class RestIntegTestTask extends RandomizedTestingTask {
integTest.testClassesDir = test.testClassesDir
integTest.mustRunAfter(test)
}
integTest.mustRunAfter(project.precommit)
project.check.dependsOn(integTest)
RestSpecHack.configureDependencies(project)
project.afterEvaluate {

View File

@ -56,6 +56,7 @@ class StandaloneTestBasePlugin implements Plugin<Project> {
plusConfigurations = [project.configurations.testRuntime]
}
}
PrecommitTasks.configure(project)
PrecommitTasks.create(project, false)
project.check.dependsOn(project.precommit)
}
}

View File

@ -45,6 +45,7 @@ class StandaloneTestPlugin implements Plugin<Project> {
classpath = project.sourceSets.test.runtimeClasspath
testClassesDir project.sourceSets.test.output.classesDir
}
test.mustRunAfter(project.precommit)
project.check.dependsOn(test)
}
}

View File

@ -117,6 +117,9 @@ forbiddenPatterns {
exclude '**/org/elasticsearch/cluster/routing/shard_routes.txt'
}
// dependency license are currently checked in distribution
dependencyLicenses.enabled = false
if (isEclipse == false || project.path == ":core-tests") {
task integTest(type: RandomizedTestingTask,
group: JavaBasePlugin.VERIFICATION_GROUP,

View File

@ -20,6 +20,7 @@
import org.apache.tools.ant.filters.FixCrLfFilter
import org.apache.tools.ant.taskdefs.condition.Os
import org.elasticsearch.gradle.precommit.DependencyLicensesTask
import org.elasticsearch.gradle.precommit.UpdateShasTask
import org.elasticsearch.gradle.test.RunTask
import org.elasticsearch.gradle.EmptyDirTask
import org.elasticsearch.gradle.MavenFilteringHack
@ -293,13 +294,16 @@ configure(subprojects.findAll { it.name == 'deb' || it.name == 'rpm' }) {
// TODO: dependency checks should really be when building the jar itself, which would remove the need
// for this hackery and instead we can do this inside the BuildPlugin
task check(group: 'Verification', description: 'Runs all checks.') {} // dummy task!
DependencyLicensesTask.configure(project) {
task dependencyLicenses(type: DependencyLicensesTask) {
dependsOn = [dependencyFiles]
dependencies = dependencyFiles
mapping from: /lucene-.*/, to: 'lucene'
mapping from: /jackson-.*/, to: 'jackson'
}
task check(group: 'Verification', description: 'Runs all checks.', dependsOn: dependencyLicenses) {} // dummy task!
task updateShas(type: UpdateShasTask) {
parentTask = dependencyLicenses
}
RunTask.configure(project)

View File

@ -29,9 +29,4 @@ subprojects {
// for local ES plugins, the name of the plugin is the same as the directory
name project.name
}
Task dependencyLicensesTask = DependencyLicensesTask.configure(project) {
dependencies = project.configurations.runtime - project.configurations.provided
}
project.precommit.dependsOn(dependencyLicensesTask)
}