From 40e4305618bc82139adc773f1d8a15a6852ae7e0 Mon Sep 17 00:00:00 2001 From: Thomas Glaeser Date: Fri, 31 Jul 2020 22:11:44 -0400 Subject: [PATCH 1/8] Fix for issue 654 ... copy the original file to the tmp location just to remember the file attributes --- .../main/java/com/diffplug/gradle/spotless/SpotlessApply.java | 2 +- .../java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java index b93ac6f2a8..a815d5c33c 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java @@ -68,7 +68,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { File originalSource = new File(getProject().getProjectDir(), path); try { getLogger().debug("Copying " + fileVisitDetails.getFile() + " to " + originalSource); - Files.copy(fileVisitDetails.getFile().toPath(), originalSource.toPath(), StandardCopyOption.REPLACE_EXISTING); + Files.copy(fileVisitDetails.getFile().toPath(), originalSource.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); } catch (IOException e) { throw new RuntimeException(e); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index f3f359c09d..ad12745672 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.Comparator; import org.gradle.api.GradleException; @@ -80,6 +81,8 @@ private void processInputFile(Formatter formatter, File input) throws IOExceptio throw new IllegalStateException("Every file has a parent folder."); } Files.createDirectories(parentDir); + // Need to copy the original file to the tmp location just to remember the file attributes + Files.copy(input.toPath(), output.toPath(), StandardCopyOption.COPY_ATTRIBUTES); dirtyState.writeCanonicalTo(output); } } From 001998446b3cd6c72e699fe0c04a14918f451c76 Mon Sep 17 00:00:00 2001 From: Thomas Glaeser Date: Fri, 31 Jul 2020 23:12:19 -0400 Subject: [PATCH 2/8] Fix for issue 654 ... fixing java.nio.file.FileAlreadyExistsException --- .../java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index ad12745672..23c0679558 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -82,7 +82,7 @@ private void processInputFile(Formatter formatter, File input) throws IOExceptio } Files.createDirectories(parentDir); // Need to copy the original file to the tmp location just to remember the file attributes - Files.copy(input.toPath(), output.toPath(), StandardCopyOption.COPY_ATTRIBUTES); + Files.copy(input.toPath(), output.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); dirtyState.writeCanonicalTo(output); } } From d537dd4067d1cdec7b22fa983c94bc0ea17ca403 Mon Sep 17 00:00:00 2001 From: Thomas Glaeser Date: Sat, 1 Aug 2020 14:07:28 -0400 Subject: [PATCH 3/8] Fix for issue 654 ... adding unit test --- .../diffplug/gradle/spotless/PaddedCellTaskTest.java | 2 ++ .../java/com/diffplug/spotless/ResourceHarness.java | 11 ++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java index 27dd125d81..572b7fb174 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java @@ -157,6 +157,8 @@ public void paddedCellApplyCheck() throws Exception { assertFile(converge.file).hasContent(""); // converge -> converges assertFile(diverge.file).hasContent("CCC"); // diverge -> no change + assertFileAttributesEqual(wellbehaved.file, wellbehaved.outputFile); + // After apply, check should pass wellbehaved.check(); cycle.check(); diff --git a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java index f436ce16ff..87587343b7 100644 --- a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 DiffPlug + * Copyright 2016-2020 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,8 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.AclEntry; +import java.nio.file.attribute.AclFileAttributeView; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -166,6 +168,13 @@ protected void assertOnResources(FormatterFunc step, String unformattedPath, Str Assert.assertEquals(expected, formatted); } + /** Reads file attributes from the output file and compares it to the attributes from the input file. */ + protected void assertFileAttributesEqual(File input, File output) throws IOException { + List inputAttributes = Files.getFileAttributeView(input.toPath(), AclFileAttributeView.class).getAcl(); + List outputAttributes = Files.getFileAttributeView(output.toPath(), AclFileAttributeView.class).getAcl(); + Assertions.assertThat(outputAttributes).isEqualTo(inputAttributes); + } + @CheckReturnValue protected ReadAsserter assertFile(String path) throws IOException { return new ReadAsserter(newFile(path)); From 63d85791496347ce66ba3b5034b102670cf00d6d Mon Sep 17 00:00:00 2001 From: Thomas Glaeser Date: Sat, 1 Aug 2020 15:12:02 -0400 Subject: [PATCH 4/8] Fix for issue 654 ... again, many of the types from Java package 'java.nio.file.attribute' are not fs agnostic; as we prefer not having fs specific conditions, we limit ourselvs to basic attribute --- .../com/diffplug/spotless/ResourceHarness.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java index 87587343b7..84fc93eff6 100644 --- a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java @@ -22,10 +22,9 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.attribute.AclEntry; -import java.nio.file.attribute.AclFileAttributeView; import java.util.ArrayList; import java.util.Arrays; +import java.util.LinkedList; import java.util.List; import java.util.function.Consumer; import java.util.function.UnaryOperator; @@ -169,9 +168,15 @@ protected void assertOnResources(FormatterFunc step, String unformattedPath, Str } /** Reads file attributes from the output file and compares it to the attributes from the input file. */ - protected void assertFileAttributesEqual(File input, File output) throws IOException { - List inputAttributes = Files.getFileAttributeView(input.toPath(), AclFileAttributeView.class).getAcl(); - List outputAttributes = Files.getFileAttributeView(output.toPath(), AclFileAttributeView.class).getAcl(); + protected void assertFileAttributesEqual(File input, File output) { + List inputAttributes = new LinkedList<>(); + List outputAttributes = new LinkedList<>(); + inputAttributes.add(Files.isReadable(input.toPath())); + inputAttributes.add(Files.isWritable(input.toPath())); + inputAttributes.add(Files.isExecutable(input.toPath())); + outputAttributes.add(Files.isReadable(output.toPath())); + outputAttributes.add(Files.isWritable(output.toPath())); + outputAttributes.add(Files.isExecutable(output.toPath())); Assertions.assertThat(outputAttributes).isEqualTo(inputAttributes); } From c1747c0c43386b072c9f827879fea24d846d9aee Mon Sep 17 00:00:00 2001 From: Thomas Glaeser Date: Sat, 1 Aug 2020 15:27:31 -0400 Subject: [PATCH 5/8] Fix for issue 654 ... add entry to change log --- plugin-gradle/CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 2deba8579a..489e1ab984 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -3,6 +3,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`). ## [Unreleased] +* Depending on the file system, executing `gradle spotlessApply` might change permission on the changed files from `644` to `755`; fixes ([#656](https://github.com/diffplug/spotless/pull/656)) * Bump default ktfmt from 0.15 to 0.16, and remove duplicated logic for the --dropbox-style option ([#642](https://github.com/diffplug/spotless/pull/648)) ## [5.1.0] - 2020-07-13 From 597c3dd4c8207c2e799dea9205c2197a92ac208d Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Aug 2020 22:24:00 -0700 Subject: [PATCH 6/8] Revert "Fix for issue 654 ... again, many of the types from Java package 'java.nio.file.attribute' are not fs agnostic; as we prefer not having fs specific conditions, we limit ourselvs to basic attribute" This reverts commit 63d85791496347ce66ba3b5034b102670cf00d6d. --- .../com/diffplug/spotless/ResourceHarness.java | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java index 84fc93eff6..87587343b7 100644 --- a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java @@ -22,9 +22,10 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.AclEntry; +import java.nio.file.attribute.AclFileAttributeView; import java.util.ArrayList; import java.util.Arrays; -import java.util.LinkedList; import java.util.List; import java.util.function.Consumer; import java.util.function.UnaryOperator; @@ -168,15 +169,9 @@ protected void assertOnResources(FormatterFunc step, String unformattedPath, Str } /** Reads file attributes from the output file and compares it to the attributes from the input file. */ - protected void assertFileAttributesEqual(File input, File output) { - List inputAttributes = new LinkedList<>(); - List outputAttributes = new LinkedList<>(); - inputAttributes.add(Files.isReadable(input.toPath())); - inputAttributes.add(Files.isWritable(input.toPath())); - inputAttributes.add(Files.isExecutable(input.toPath())); - outputAttributes.add(Files.isReadable(output.toPath())); - outputAttributes.add(Files.isWritable(output.toPath())); - outputAttributes.add(Files.isExecutable(output.toPath())); + protected void assertFileAttributesEqual(File input, File output) throws IOException { + List inputAttributes = Files.getFileAttributeView(input.toPath(), AclFileAttributeView.class).getAcl(); + List outputAttributes = Files.getFileAttributeView(output.toPath(), AclFileAttributeView.class).getAcl(); Assertions.assertThat(outputAttributes).isEqualTo(inputAttributes); } From c6d63f0e131aaa7d5035a34318ea5d3f4ef930f8 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Aug 2020 22:24:00 -0700 Subject: [PATCH 7/8] Revert "Fix for issue 654 ... adding unit test" This reverts commit d537dd4067d1cdec7b22fa983c94bc0ea17ca403. --- .../diffplug/gradle/spotless/PaddedCellTaskTest.java | 2 -- .../java/com/diffplug/spotless/ResourceHarness.java | 11 +---------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java index 572b7fb174..27dd125d81 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java @@ -157,8 +157,6 @@ public void paddedCellApplyCheck() throws Exception { assertFile(converge.file).hasContent(""); // converge -> converges assertFile(diverge.file).hasContent("CCC"); // diverge -> no change - assertFileAttributesEqual(wellbehaved.file, wellbehaved.outputFile); - // After apply, check should pass wellbehaved.check(); cycle.check(); diff --git a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java index 87587343b7..f436ce16ff 100644 --- a/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java +++ b/testlib/src/main/java/com/diffplug/spotless/ResourceHarness.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2020 DiffPlug + * Copyright 2016 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,8 +22,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.attribute.AclEntry; -import java.nio.file.attribute.AclFileAttributeView; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -168,13 +166,6 @@ protected void assertOnResources(FormatterFunc step, String unformattedPath, Str Assert.assertEquals(expected, formatted); } - /** Reads file attributes from the output file and compares it to the attributes from the input file. */ - protected void assertFileAttributesEqual(File input, File output) throws IOException { - List inputAttributes = Files.getFileAttributeView(input.toPath(), AclFileAttributeView.class).getAcl(); - List outputAttributes = Files.getFileAttributeView(output.toPath(), AclFileAttributeView.class).getAcl(); - Assertions.assertThat(outputAttributes).isEqualTo(inputAttributes); - } - @CheckReturnValue protected ReadAsserter assertFile(String path) throws IOException { return new ReadAsserter(newFile(path)); From 4c3a399e7a597bb3e41854d948f1b8392965964a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 4 Aug 2020 22:41:58 -0700 Subject: [PATCH 8/8] Create a failing test to reproduce #654. --- .../gradle/spotless/FilePermissionsTest.java | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FilePermissionsTest.java diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FilePermissionsTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FilePermissionsTest.java new file mode 100644 index 0000000000..63f86f902b --- /dev/null +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FilePermissionsTest.java @@ -0,0 +1,62 @@ +/* + * Copyright 2020 DiffPlug + * + * Licensed 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 com.diffplug.gradle.spotless; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermissions; + +import org.assertj.core.api.AbstractStringAssert; +import org.assertj.core.api.Assertions; +import org.junit.Test; + +import com.diffplug.spotless.FileSignature; + +public class FilePermissionsTest extends GradleIntegrationHarness { + @Test + public void spotlessApplyShouldPreservePermissions() throws IOException { + if (FileSignature.machineIsWin()) { + // need unix filesystem + return; + } + setFile("build.gradle").toLines( + "buildscript { repositories { mavenCentral() } }", + "plugins {", + " id 'com.diffplug.spotless'", + "}", + "", + "spotless {", + " java {", + " target file('test.java')", + " googleJavaFormat('1.2')", + " }", + "}"); + setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); + + Path path = rootFolder().toPath().resolve("test.java"); + Files.setPosixFilePermissions(path, PosixFilePermissions.fromString("rwxr--r--")); + assertPermissions(path).isEqualTo("rwxr--r--"); + + gradleRunner().withArguments("spotlessApply").build(); + assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); + assertPermissions(path).isEqualTo("rwxr--r--"); + } + + private AbstractStringAssert assertPermissions(Path path) throws IOException { + return Assertions.assertThat(PosixFilePermissions.toString(Files.getPosixFilePermissions(path))); + } +}