From 67b902671aaceb7a6abad73d20cea3c7b8009639 Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Sun, 21 Oct 2018 01:13:32 +0200 Subject: [PATCH 01/11] convert FilePermissionsTask.groovy to .java --- .../precommit/FilePermissionsTask.groovy | 87 --------------- .../gradle/precommit/FilePermissionsTask.java | 103 ++++++++++++++++++ .../precommit/FilePermissionsTaskTest.java | 87 +++++++++++++++ 3 files changed, 190 insertions(+), 87 deletions(-) delete mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FilePermissionsTask.groovy create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FilePermissionsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FilePermissionsTask.groovy deleted file mode 100644 index d8da9a4207bf7..0000000000000 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FilePermissionsTask.groovy +++ /dev/null @@ -1,87 +0,0 @@ -/* - * 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.GradleException -import org.gradle.api.file.FileCollection -import org.gradle.api.tasks.InputFiles -import org.gradle.api.tasks.OutputFile -import org.gradle.api.tasks.SourceSet -import org.gradle.api.tasks.TaskAction -import org.gradle.api.tasks.util.PatternSet -import org.gradle.api.tasks.util.PatternFilterable -import org.apache.tools.ant.taskdefs.condition.Os - -import java.nio.file.Files -import java.nio.file.attribute.PosixFilePermission -import java.nio.file.attribute.PosixFileAttributeView - -import static java.nio.file.attribute.PosixFilePermission.OTHERS_EXECUTE -import static java.nio.file.attribute.PosixFilePermission.GROUP_EXECUTE -import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE - -/** - * Checks source files for correct file permissions. - */ -public class FilePermissionsTask extends DefaultTask { - - /** A pattern set of which files should be checked. */ - private PatternFilterable filesFilter = new PatternSet() - - @OutputFile - File outputMarker = new File(project.buildDir, 'markers/filePermissions') - - FilePermissionsTask() { - onlyIf { !Os.isFamily(Os.FAMILY_WINDOWS) } - description = "Checks java source files for correct file permissions" - // we always include all source files, and exclude what should not be checked - filesFilter.include('**') - // exclude sh files that might have the executable bit set - filesFilter.exclude('**/*.sh') - } - - /** Returns the files this task will check */ - @InputFiles - FileCollection files() { - List collections = new ArrayList<>() - for (SourceSet sourceSet : project.sourceSets) { - collections.add(sourceSet.allSource.matching(filesFilter)) - } - return project.files(collections.toArray()) - } - - @TaskAction - void checkInvalidPermissions() { - List failures = new ArrayList<>() - for (File f : files()) { - PosixFileAttributeView fileAttributeView = Files.getFileAttributeView(f.toPath(), PosixFileAttributeView.class) - Set permissions = fileAttributeView.readAttributes().permissions() - if (permissions.contains(OTHERS_EXECUTE) || permissions.contains(OWNER_EXECUTE) || - permissions.contains(GROUP_EXECUTE)) { - failures.add("Source file is executable: " + f) - } - } - if (failures.isEmpty() == false) { - throw new GradleException('Found invalid file permissions:\n' + failures.join('\n')) - } - outputMarker.setText('done', 'UTF-8') - } - -} diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java new file mode 100644 index 0000000000000..a8971ae2c2392 --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java @@ -0,0 +1,103 @@ +/* + * 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 java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermission; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import org.apache.tools.ant.taskdefs.condition.Os; +import org.gradle.api.DefaultTask; +import org.gradle.api.GradleException; +import org.gradle.api.file.FileCollection; +import org.gradle.api.plugins.JavaPluginConvention; +import org.gradle.api.tasks.*; +import org.gradle.api.tasks.util.PatternFilterable; +import org.gradle.api.tasks.util.PatternSet; + +/** + * Checks source files for correct file permissions. + */ +public class FilePermissionsTask extends DefaultTask { + + /** + * A pattern set of which files should be checked. + */ + private final PatternFilterable filesFilter = new PatternSet() + // we always include all source files, and exclude what should not be checked + .include("**") + // exclude sh files that might have the executable bit set + .exclude("**/*.sh"); + + @OutputFile + private File outputMarker = new File(getProject().getBuildDir(), "markers/filePermissions"); + + public FilePermissionsTask() { + setDescription("Checks java source files for correct file permissions"); + } + + /** + * Returns the files this task will check + */ + @InputFiles + public FileCollection files() { + SourceSetContainer sourceSets = getProject().getConvention().getPlugin(JavaPluginConvention.class).getSourceSets(); + Object[] fileTreeStream = sourceSets.stream() + .map(sourceSet -> sourceSet.getAllSource().matching(filesFilter)) + .toArray(); + return getProject().files(fileTreeStream); + } + + @TaskAction + public void checkInvalidPermissions() throws IOException { + if (Os.isFamily(Os.FAMILY_WINDOWS)) { + throw new StopExecutionException(); + } + + List failures = new ArrayList(); + for (File f : files()) { + PosixFileAttributeView fileAttributeView = Files.getFileAttributeView(f.toPath(), PosixFileAttributeView.class); + Set permissions = fileAttributeView.readAttributes().permissions(); + if (permissions.contains(PosixFilePermission.OTHERS_EXECUTE) + || permissions.contains(PosixFilePermission.OWNER_EXECUTE) + || permissions.contains(PosixFilePermission.GROUP_EXECUTE)) { + failures.add("Source file is executable: " + f); + } + } + + if (!failures.isEmpty()) { + throw new GradleException("Found invalid file permissions:\n" + String.join("\n", failures)); + } + + Files.write(outputMarker.toPath(), "done".getBytes("UTF-8")); + } + + public File getOutputMarker() { + return outputMarker; + } + + public void setOutputMarker(File outputMarker) { + this.outputMarker = outputMarker; + } +} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java new file mode 100644 index 0000000000000..50af9bd4ce71f --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java @@ -0,0 +1,87 @@ +/* + * 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 java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.util.List; + +import org.elasticsearch.gradle.test.GradleUnitTestCase; +import org.gradle.api.GradleException; +import org.gradle.api.Project; +import org.gradle.api.plugins.JavaPlugin; +import org.gradle.testfixtures.ProjectBuilder; +import org.junit.Rule; +import org.junit.rules.TemporaryFolder; + +import static java.util.Collections.singletonMap; + +public class FilePermissionsTaskTest extends GradleUnitTestCase { + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + public void testCheckPermissionsWhenAnExecutableFileExists() throws Exception { + Project project = createProject(); + + FilePermissionsTask filePermissionsTask = createTask(project); + File outputMarker = temporaryFolder.newFile(); + filePermissionsTask.setOutputMarker(outputMarker); + + File file = new File(project.getProjectDir(), "src/main/java/Code.java"); + file.getParentFile().mkdirs(); + file.createNewFile(); + file.setExecutable(true); + + try { + filePermissionsTask.checkInvalidPermissions(); + } catch (GradleException e) { + assertEquals(true, e.getMessage().startsWith("Found invalid file permissions")); + } + } + + + public void testCheckPermissionsWhenNoExecutableFileExists() throws Exception { + Project project = createProject(); + + FilePermissionsTask filePermissionsTask = createTask(project); + File outputMarker = temporaryFolder.newFile(); + filePermissionsTask.setOutputMarker(outputMarker); + + File file = new File(project.getProjectDir(), "src/main/java/Code.java"); + file.getParentFile().mkdirs(); + file.createNewFile(); + + filePermissionsTask.checkInvalidPermissions(); + + List result = Files.readAllLines(outputMarker.toPath(), Charset.forName("UTF-8")); + assertEquals("done", result.get(0)); + } + + private Project createProject() throws IOException { + Project project = ProjectBuilder.builder().withProjectDir(temporaryFolder.newFolder()).build(); + project.getPlugins().apply(JavaPlugin.class); + return project; + } + private FilePermissionsTask createTask(Project project) { + FilePermissionsTask task = (FilePermissionsTask) project.task(singletonMap("type", FilePermissionsTask.class), "filePermissionsTask"); + return task; + } +} \ No newline at end of file From 411492c97bbfe46fdad3e4198b7da97a5ab8f8c8 Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Mon, 22 Oct 2018 08:29:55 +0200 Subject: [PATCH 02/11] append missing end newline --- .../elasticsearch/gradle/precommit/FilePermissionsTaskTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java index 50af9bd4ce71f..ec5e9be1573ee 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java @@ -84,4 +84,4 @@ private FilePermissionsTask createTask(Project project) { FilePermissionsTask task = (FilePermissionsTask) project.task(singletonMap("type", FilePermissionsTask.class), "filePermissionsTask"); return task; } -} \ No newline at end of file +} From 22f81b6857ee408c5c57724ccc4243896f21290d Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Mon, 22 Oct 2018 13:17:27 +0200 Subject: [PATCH 03/11] expand imports --- .../elasticsearch/gradle/precommit/FilePermissionsTask.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java index a8971ae2c2392..d91326107917d 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java @@ -32,7 +32,11 @@ import org.gradle.api.GradleException; import org.gradle.api.file.FileCollection; import org.gradle.api.plugins.JavaPluginConvention; -import org.gradle.api.tasks.*; +import org.gradle.api.tasks.InputFiles; +import org.gradle.api.tasks.OutputFile; +import org.gradle.api.tasks.SourceSetContainer; +import org.gradle.api.tasks.StopExecutionException; +import org.gradle.api.tasks.TaskAction; import org.gradle.api.tasks.util.PatternFilterable; import org.gradle.api.tasks.util.PatternSet; From 2b8db2509e3e59b4435ec9514646e8aebc5f38cf Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Mon, 22 Oct 2018 13:45:45 +0200 Subject: [PATCH 04/11] rename test --- ...lePermissionsTaskTest.java => FilePermissionsTaskTests.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename buildSrc/src/test/java/org/elasticsearch/gradle/precommit/{FilePermissionsTaskTest.java => FilePermissionsTaskTests.java} (97%) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java similarity index 97% rename from buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java rename to buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java index ec5e9be1573ee..802f36d82f68d 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTest.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java @@ -34,7 +34,7 @@ import static java.util.Collections.singletonMap; -public class FilePermissionsTaskTest extends GradleUnitTestCase { +public class FilePermissionsTaskTests extends GradleUnitTestCase { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); From 1d40b3690e04c047b39f0c25ca282a74fcaecc44 Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Mon, 22 Oct 2018 14:37:36 +0200 Subject: [PATCH 05/11] fix checkstyle --- .../gradle/precommit/FilePermissionsTaskTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java index 802f36d82f68d..77a1aab98d2c8 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java @@ -81,7 +81,8 @@ private Project createProject() throws IOException { return project; } private FilePermissionsTask createTask(Project project) { - FilePermissionsTask task = (FilePermissionsTask) project.task(singletonMap("type", FilePermissionsTask.class), "filePermissionsTask"); + FilePermissionsTask task = (FilePermissionsTask) project.task(singletonMap("type", FilePermissionsTask.class), + "filePermissionsTask"); return task; } } From 93f89fe8c5f95a65b45e7cbc2fae577f39f2578a Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Mon, 22 Oct 2018 21:38:24 +0200 Subject: [PATCH 06/11] various fixes --- .../gradle/precommit/FilePermissionsTask.java | 37 +++++++++++-------- .../precommit/FilePermissionsTaskTests.java | 11 +++--- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java index d91326107917d..f32a9390ed458 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java @@ -23,9 +23,9 @@ import java.nio.file.Files; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFilePermission; -import java.util.ArrayList; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.apache.tools.ant.taskdefs.condition.Os; import org.gradle.api.DefaultTask; @@ -61,16 +61,28 @@ public FilePermissionsTask() { setDescription("Checks java source files for correct file permissions"); } + private static boolean isExecutableFile(File file) { + try { + Set permissions = Files.getFileAttributeView(file.toPath(), PosixFileAttributeView.class) + .readAttributes() + .permissions(); + return permissions.contains(PosixFilePermission.OTHERS_EXECUTE) + || permissions.contains(PosixFilePermission.OWNER_EXECUTE) + || permissions.contains(PosixFilePermission.GROUP_EXECUTE); + } catch (IOException e) { + throw new IllegalStateException("unable to read the file " + file + " attributes", e); + } + } + /** * Returns the files this task will check */ @InputFiles - public FileCollection files() { + public FileCollection getFiles() { SourceSetContainer sourceSets = getProject().getConvention().getPlugin(JavaPluginConvention.class).getSourceSets(); - Object[] fileTreeStream = sourceSets.stream() + return getProject().files(sourceSets.stream() .map(sourceSet -> sourceSet.getAllSource().matching(filesFilter)) - .toArray(); - return getProject().files(fileTreeStream); + .toArray()); } @TaskAction @@ -78,17 +90,10 @@ public void checkInvalidPermissions() throws IOException { if (Os.isFamily(Os.FAMILY_WINDOWS)) { throw new StopExecutionException(); } - - List failures = new ArrayList(); - for (File f : files()) { - PosixFileAttributeView fileAttributeView = Files.getFileAttributeView(f.toPath(), PosixFileAttributeView.class); - Set permissions = fileAttributeView.readAttributes().permissions(); - if (permissions.contains(PosixFilePermission.OTHERS_EXECUTE) - || permissions.contains(PosixFilePermission.OWNER_EXECUTE) - || permissions.contains(PosixFilePermission.GROUP_EXECUTE)) { - failures.add("Source file is executable: " + f); - } - } + List failures = getFiles().getFiles().stream() + .filter(FilePermissionsTask::isExecutableFile) + .map(file -> "Source file is executable: " + file) + .collect(Collectors.toList()); if (!failures.isEmpty()) { throw new GradleException("Found invalid file permissions:\n" + String.join("\n", failures)); diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java index 77a1aab98d2c8..ebb99552f19e0 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java @@ -29,11 +29,10 @@ import org.gradle.api.Project; import org.gradle.api.plugins.JavaPlugin; import org.gradle.testfixtures.ProjectBuilder; +import org.junit.Assert; import org.junit.Rule; import org.junit.rules.TemporaryFolder; -import static java.util.Collections.singletonMap; - public class FilePermissionsTaskTests extends GradleUnitTestCase { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -52,8 +51,9 @@ public void testCheckPermissionsWhenAnExecutableFileExists() throws Exception { try { filePermissionsTask.checkInvalidPermissions(); + Assert.fail("the check should have failed because of the executable file permission"); } catch (GradleException e) { - assertEquals(true, e.getMessage().startsWith("Found invalid file permissions")); + assertTrue(e.getMessage().startsWith("Found invalid file permissions")); } } @@ -80,9 +80,8 @@ private Project createProject() throws IOException { project.getPlugins().apply(JavaPlugin.class); return project; } + private FilePermissionsTask createTask(Project project) { - FilePermissionsTask task = (FilePermissionsTask) project.task(singletonMap("type", FilePermissionsTask.class), - "filePermissionsTask"); - return task; + return project.getTasks().create("filePermissionsTask", FilePermissionsTask.class); } } From 7c259f1eb1d665d89090b1107fe9c99736245236 Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Mon, 22 Oct 2018 21:40:39 +0200 Subject: [PATCH 07/11] task annotation on getter --- .../org/elasticsearch/gradle/precommit/FilePermissionsTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java index f32a9390ed458..aa6176bca9aee 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java @@ -54,7 +54,6 @@ public class FilePermissionsTask extends DefaultTask { // exclude sh files that might have the executable bit set .exclude("**/*.sh"); - @OutputFile private File outputMarker = new File(getProject().getBuildDir(), "markers/filePermissions"); public FilePermissionsTask() { @@ -102,6 +101,7 @@ public void checkInvalidPermissions() throws IOException { Files.write(outputMarker.toPath(), "done".getBytes("UTF-8")); } + @OutputFile public File getOutputMarker() { return outputMarker; } From d1f51c77992f7e902b2a5c034f6f63469762f9a2 Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Fri, 26 Oct 2018 11:24:40 +0200 Subject: [PATCH 08/11] remove setter on outputMarker + simplify source file retrieval --- .../gradle/precommit/FilePermissionsTask.java | 11 ++++++----- .../gradle/precommit/FilePermissionsTaskTests.java | 5 +---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java index aa6176bca9aee..6adcb78c4ff7a 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java @@ -31,6 +31,7 @@ import org.gradle.api.DefaultTask; import org.gradle.api.GradleException; import org.gradle.api.file.FileCollection; +import org.gradle.api.file.FileTree; import org.gradle.api.plugins.JavaPluginConvention; import org.gradle.api.tasks.InputFiles; import org.gradle.api.tasks.OutputFile; @@ -79,9 +80,11 @@ private static boolean isExecutableFile(File file) { @InputFiles public FileCollection getFiles() { SourceSetContainer sourceSets = getProject().getConvention().getPlugin(JavaPluginConvention.class).getSourceSets(); - return getProject().files(sourceSets.stream() + return sourceSets.stream() .map(sourceSet -> sourceSet.getAllSource().matching(filesFilter)) - .toArray()); + .reduce(FileTree::plus) + .orElse(null) + ; } @TaskAction @@ -98,6 +101,7 @@ public void checkInvalidPermissions() throws IOException { throw new GradleException("Found invalid file permissions:\n" + String.join("\n", failures)); } + outputMarker.getParentFile().mkdirs(); Files.write(outputMarker.toPath(), "done".getBytes("UTF-8")); } @@ -106,7 +110,4 @@ public File getOutputMarker() { return outputMarker; } - public void setOutputMarker(File outputMarker) { - this.outputMarker = outputMarker; - } } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java index ebb99552f19e0..1ebee004d5c94 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java @@ -41,8 +41,6 @@ public void testCheckPermissionsWhenAnExecutableFileExists() throws Exception { Project project = createProject(); FilePermissionsTask filePermissionsTask = createTask(project); - File outputMarker = temporaryFolder.newFile(); - filePermissionsTask.setOutputMarker(outputMarker); File file = new File(project.getProjectDir(), "src/main/java/Code.java"); file.getParentFile().mkdirs(); @@ -62,8 +60,6 @@ public void testCheckPermissionsWhenNoExecutableFileExists() throws Exception { Project project = createProject(); FilePermissionsTask filePermissionsTask = createTask(project); - File outputMarker = temporaryFolder.newFile(); - filePermissionsTask.setOutputMarker(outputMarker); File file = new File(project.getProjectDir(), "src/main/java/Code.java"); file.getParentFile().mkdirs(); @@ -71,6 +67,7 @@ public void testCheckPermissionsWhenNoExecutableFileExists() throws Exception { filePermissionsTask.checkInvalidPermissions(); + File outputMarker = new File(project.getBuildDir(), "markers/filePermissions"); List result = Files.readAllLines(outputMarker.toPath(), Charset.forName("UTF-8")); assertEquals("done", result.get(0)); } From 6be51cd6e800d96e7432fec1509f7ee0124cfb8a Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Fri, 26 Oct 2018 11:26:39 +0200 Subject: [PATCH 09/11] formatting --- .../elasticsearch/gradle/precommit/FilePermissionsTask.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java index 6adcb78c4ff7a..b491637c6e959 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java @@ -83,8 +83,7 @@ public FileCollection getFiles() { return sourceSets.stream() .map(sourceSet -> sourceSet.getAllSource().matching(filesFilter)) .reduce(FileTree::plus) - .orElse(null) - ; + .orElse(null); } @TaskAction From bd5e9a60bf832c459a7cd81569493137581fdb02 Mon Sep 17 00:00:00 2001 From: Vincent Boulaye Date: Fri, 26 Oct 2018 11:33:51 +0200 Subject: [PATCH 10/11] add test case when no files are found --- .../precommit/FilePermissionsTaskTests.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java index 1ebee004d5c94..4d73a548d8e35 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/FilePermissionsTaskTests.java @@ -53,9 +53,22 @@ public void testCheckPermissionsWhenAnExecutableFileExists() throws Exception { } catch (GradleException e) { assertTrue(e.getMessage().startsWith("Found invalid file permissions")); } + file.delete(); } + public void testCheckPermissionsWhenNoFileExists() throws Exception { + Project project = createProject(); + + FilePermissionsTask filePermissionsTask = createTask(project); + + filePermissionsTask.checkInvalidPermissions(); + + File outputMarker = new File(project.getBuildDir(), "markers/filePermissions"); + List result = Files.readAllLines(outputMarker.toPath(), Charset.forName("UTF-8")); + assertEquals("done", result.get(0)); + } + public void testCheckPermissionsWhenNoExecutableFileExists() throws Exception { Project project = createProject(); @@ -70,6 +83,9 @@ public void testCheckPermissionsWhenNoExecutableFileExists() throws Exception { File outputMarker = new File(project.getBuildDir(), "markers/filePermissions"); List result = Files.readAllLines(outputMarker.toPath(), Charset.forName("UTF-8")); assertEquals("done", result.get(0)); + + file.delete(); + } private Project createProject() throws IOException { From 3c0b5c86be32cf9b6ae5e7eef808591a2deaa252 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Fri, 26 Oct 2018 14:44:23 +0300 Subject: [PATCH 11/11] Avoid returning null --- .../elasticsearch/gradle/precommit/FilePermissionsTask.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java index b491637c6e959..100c3a22700ad 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java @@ -35,6 +35,7 @@ import org.gradle.api.plugins.JavaPluginConvention; import org.gradle.api.tasks.InputFiles; import org.gradle.api.tasks.OutputFile; +import org.gradle.api.tasks.SkipWhenEmpty; import org.gradle.api.tasks.SourceSetContainer; import org.gradle.api.tasks.StopExecutionException; import org.gradle.api.tasks.TaskAction; @@ -78,12 +79,13 @@ private static boolean isExecutableFile(File file) { * Returns the files this task will check */ @InputFiles + @SkipWhenEmpty public FileCollection getFiles() { SourceSetContainer sourceSets = getProject().getConvention().getPlugin(JavaPluginConvention.class).getSourceSets(); return sourceSets.stream() .map(sourceSet -> sourceSet.getAllSource().matching(filesFilter)) .reduce(FileTree::plus) - .orElse(null); + .orElse(getProject().files().getAsFileTree()); } @TaskAction