From e7a879d709e40604b53020a9eee64f177ed8e22e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 11 Oct 2024 15:14:43 -0700 Subject: [PATCH 1/8] First shot at fixing the configuration cache / remote build cache conflict. --- .../spotless/ConfigurationCacheHack.java | 87 +++++++++++++++++++ .../java/com/diffplug/spotless/Formatter.java | 4 + .../FormatterStepSerializationRoundtrip.java | 20 +++-- .../gradle/spotless/SpotlessTask.java | 22 +++-- 4 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java diff --git a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java new file mode 100644 index 0000000000..9962bd2a54 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java @@ -0,0 +1,87 @@ +/* + * Copyright 2024 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.spotless; + +import java.io.IOException; +import java.util.AbstractList; +import java.util.ArrayList; +import java.util.Collection; + +/** + * Gradle requires three things: + * - Gradle defines cache equality based on your serialized representation + * - Combined with remote build cache, you cannot have any absolute paths in your serialized representation + * - Combined with configuration cache, you must be able to roundtrip yourself through serialization + * + * These requirements are at odds with each other, as described in these issues + * - Gradle issue to define custom equality https://github.com/gradle/gradle/issues/29816 + * - Spotless plea for developer cache instead of configuration cache https://github.com/diffplug/spotless/issues/987 + * - Spotless cache miss bug fixed by this class https://github.com/diffplug/spotless/issues/2168 + * + * The point of this class is to create containers which can optimize the serialized representation for either + * - roundtrip integrity + * - equality + * + * It is a horrific hack, but it works, and it's the only way I can figure + * to make Spotless work with all of Gradle's cache systems. + */ +public class ConfigurationCacheHack { + static boolean SERIALIZE_FOR_ROUNDTRIP = false; + + public enum OptimizeFor { + ROUNDTRIP, EQUALITY, + } + + public static class StepList extends AbstractList { + private final boolean optimizeForEquality; + private transient ArrayList backingList = new ArrayList<>(); + + public StepList(OptimizeFor optimizeFor) { + this.optimizeForEquality = optimizeFor == OptimizeFor.EQUALITY; + } + + @Override + public void clear() { + backingList.clear(); + } + + @Override + public boolean addAll(Collection c) { + return backingList.addAll(c); + } + + private void writeObject(java.io.ObjectOutputStream out) throws IOException { + out.defaultWriteObject(); + SERIALIZE_FOR_ROUNDTRIP = !optimizeForEquality; + out.writeObject(backingList); + } + + private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + backingList = (ArrayList) in.readObject(); + } + + @Override + public FormatterStep get(int index) { + return backingList.get(index); + } + + @Override + public int size() { + return backingList.size(); + } + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index ce083e826a..3f28d42df7 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -35,6 +35,8 @@ import javax.annotation.Nullable; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** Formatter which performs the full formatting. */ public final class Formatter implements Serializable, AutoCloseable { private static final long serialVersionUID = 1L; @@ -57,11 +59,13 @@ private Formatter(String name, LineEnding.Policy lineEndingsPolicy, Charset enco } // override serialize output + @SuppressFBWarnings("ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD") private void writeObject(ObjectOutputStream out) throws IOException { out.writeObject(name); out.writeObject(lineEndingsPolicy); out.writeObject(encoding.name()); out.writeObject(rootDir.toString()); + ConfigurationCacheHack.SERIALIZE_FOR_ROUNDTRIP = true; out.writeObject(steps); out.writeObject(exceptionPolicy); } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java index 3af89083fc..263826c9ca 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java @@ -26,6 +26,7 @@ class FormatterStepSerializationRoundtrip initializer; private @Nullable RoundtripState roundtripStateInternal; + private @Nullable EqualityState equalityStateInternal; private final SerializedFunction equalityStateExtractor; private final SerializedFunction equalityStateToFormatter; @@ -43,10 +44,13 @@ public String getName() { @Override protected EqualityState stateSupplier() throws Exception { - if (roundtripStateInternal == null) { - roundtripStateInternal = initializer.get(); + if (equalityStateInternal == null) { + if (roundtripStateInternal == null) { + roundtripStateInternal = initializer.get(); + } + equalityStateInternal = equalityStateExtractor.apply(roundtripStateInternal); } - return equalityStateExtractor.apply(roundtripStateInternal); + return equalityStateInternal; } @Override @@ -56,8 +60,14 @@ protected FormatterFunc stateToFormatter(EqualityState equalityState) throws Exc // override serialize output private void writeObject(java.io.ObjectOutputStream out) throws IOException { - if (roundtripStateInternal == null) { - roundtripStateInternal = ThrowingEx.get(initializer::get); + if (ConfigurationCacheHack.SERIALIZE_FOR_ROUNDTRIP) { + if (roundtripStateInternal == null) { + roundtripStateInternal = ThrowingEx.get(initializer::get); + } + equalityStateInternal = null; + } else { + equalityStateInternal = ThrowingEx.get(this::stateSupplier); + roundtripStateInternal = null; } out.defaultWriteObject(); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index f930685baf..5e2fce2713 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -17,7 +17,6 @@ import java.io.File; import java.nio.charset.Charset; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -37,6 +36,7 @@ import org.gradle.api.tasks.PathSensitivity; import org.gradle.work.Incremental; +import com.diffplug.spotless.ConfigurationCacheHack; import com.diffplug.spotless.FormatExceptionPolicy; import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; @@ -150,17 +150,25 @@ public File getOutputDirectory() { return outputDirectory; } - protected final List steps = new ArrayList<>(); + private final List stepsInternalRoundtrip = new ConfigurationCacheHack.StepList(ConfigurationCacheHack.OptimizeFor.ROUNDTRIP); + private final List stepsInternalEquality = new ConfigurationCacheHack.StepList(ConfigurationCacheHack.OptimizeFor.EQUALITY); + + @Internal + public List getStepsInternalRoundtrip() { + return Collections.unmodifiableList(stepsInternalRoundtrip); + } @Input - public List getSteps() { - return Collections.unmodifiableList(steps); + public List getStepsInternalEquality() { + return Collections.unmodifiableList(stepsInternalEquality); } public void setSteps(List steps) { PluginGradlePreconditions.requireElementsNonNull(steps); - this.steps.clear(); - this.steps.addAll(steps); + this.stepsInternalRoundtrip.clear(); + this.stepsInternalEquality.clear(); + this.stepsInternalRoundtrip.addAll(steps); + this.stepsInternalEquality.addAll(steps); } /** Returns the name of this format. */ @@ -179,7 +187,7 @@ Formatter buildFormatter() { .lineEndingsPolicy(getLineEndingsPolicy().get()) .encoding(Charset.forName(encoding)) .rootDir(getProjectDir().get().getAsFile().toPath()) - .steps(steps) + .steps(stepsInternalRoundtrip) .exceptionPolicy(exceptionPolicy) .build(); } From 9881ea45039f078b04009d3e91a849bf3479e8ed Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 11 Oct 2024 16:19:42 -0700 Subject: [PATCH 2/8] Update changelogs. --- CHANGES.md | 1 + plugin-gradle/CHANGES.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 6f6ce2bf31..fb0498eea5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Added * Support for `rdf` ([#2261](https://github.com/diffplug/spotless/pull/2261)) * Support for `buf` on maven plugin ([#2291](https://github.com/diffplug/spotless/pull/2291)) +* `ConfigurationCacheHack` so we can support Gradle's configuration cache and remote build cache at the same time. ([TODO]()fixes [#2168](https://github.com/diffplug/spotless/issues/2168)) ### Changed * Support configuring the Equo P2 cache. ([#2238](https://github.com/diffplug/spotless/pull/2238)) * Add explicit support for JSONC / CSS via biome, via the file extensions `.css` and `.jsonc`. diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index fde6696b03..fdf29157b1 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -16,6 +16,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * Bump default `jackson` version to latest `2.17.2` -> `2.18.0`. ([#2279](https://github.com/diffplug/spotless/pull/2279)) ### Fixed * Java import order, ignore duplicate group entries. ([#2293](https://github.com/diffplug/spotless/pull/2293)) +* Remote build cache shouldn't have cache misses anymore. ([TODO]()fixes [#2168](https://github.com/diffplug/spotless/issues/2168)) ## [7.0.0.BETA2] - 2024-08-25 ### Changed From 716722a982542edc4373304965c69c42632f398a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 12 Oct 2024 22:23:36 -0700 Subject: [PATCH 3/8] Give `FormatterStepSerializationRoundtrip` a special serialization format instead. --- .../spotless/ConfigurationCacheHack.java | 74 ++++++++++------- .../java/com/diffplug/spotless/Formatter.java | 4 - .../FormatterStepSerializationRoundtrip.java | 80 +++++++++++++------ .../gradle/spotless/SpotlessTask.java | 15 ++-- 4 files changed, 109 insertions(+), 64 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java index 9962bd2a54..1544139a15 100644 --- a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java +++ b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java @@ -15,23 +15,29 @@ */ package com.diffplug.spotless; -import java.io.IOException; -import java.util.AbstractList; import java.util.ArrayList; import java.util.Collection; +import java.util.List; +import java.util.Objects; /** * Gradle requires three things: * - Gradle defines cache equality based on your serialized representation - * - Combined with remote build cache, you cannot have any absolute paths in your serialized representation - * - Combined with configuration cache, you must be able to roundtrip yourself through serialization + * - Combined with remote build cache, you cannot have any absolute paths in + * your serialized representation + * - Combined with configuration cache, you must be able to roundtrip yourself + * through serialization * * These requirements are at odds with each other, as described in these issues - * - Gradle issue to define custom equality https://github.com/gradle/gradle/issues/29816 - * - Spotless plea for developer cache instead of configuration cache https://github.com/diffplug/spotless/issues/987 - * - Spotless cache miss bug fixed by this class https://github.com/diffplug/spotless/issues/2168 + * - Gradle issue to define custom equality + * https://github.com/gradle/gradle/issues/29816 + * - Spotless plea for developer cache instead of configuration cache + * https://github.com/diffplug/spotless/issues/987 + * - Spotless cache miss bug fixed by this class + * https://github.com/diffplug/spotless/issues/2168 * - * The point of this class is to create containers which can optimize the serialized representation for either + * The point of this class is to create containers which can optimize the + * serialized representation for either * - roundtrip integrity * - equality * @@ -39,49 +45,59 @@ * to make Spotless work with all of Gradle's cache systems. */ public class ConfigurationCacheHack { - static boolean SERIALIZE_FOR_ROUNDTRIP = false; - public enum OptimizeFor { ROUNDTRIP, EQUALITY, } - public static class StepList extends AbstractList { + public static class StepList implements java.io.Serializable { private final boolean optimizeForEquality; - private transient ArrayList backingList = new ArrayList<>(); + private final ArrayList backingList = new ArrayList<>(); public StepList(OptimizeFor optimizeFor) { this.optimizeForEquality = optimizeFor == OptimizeFor.EQUALITY; } - @Override public void clear() { backingList.clear(); } - @Override - public boolean addAll(Collection c) { - return backingList.addAll(c); - } - - private void writeObject(java.io.ObjectOutputStream out) throws IOException { - out.defaultWriteObject(); - SERIALIZE_FOR_ROUNDTRIP = !optimizeForEquality; - out.writeObject(backingList); + public void addAll(Collection c) { + for (FormatterStep step : c) { + if (step instanceof FormatterStepSerializationRoundtrip) { + var clone = ((FormatterStepSerializationRoundtrip) step).hackClone(optimizeForEquality); + backingList.add(clone); + } else { + backingList.add(step); + } + } } - private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException { - in.defaultReadObject(); - backingList = (ArrayList) in.readObject(); + public List getSteps() { + var result = new ArrayList(backingList.size()); + for (Object obj : backingList) { + if (obj instanceof FormatterStepSerializationRoundtrip.HackClone) { + result.add(((FormatterStepSerializationRoundtrip.HackClone) obj).rehydrate()); + } else { + result.add((FormatterStep) obj); + } + } + return result; } @Override - public FormatterStep get(int index) { - return backingList.get(index); + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + StepList stepList = (StepList) o; + return optimizeForEquality == stepList.optimizeForEquality && + backingList.equals(stepList.backingList); } @Override - public int size() { - return backingList.size(); + public int hashCode() { + return Objects.hash(optimizeForEquality, backingList); } } } diff --git a/lib/src/main/java/com/diffplug/spotless/Formatter.java b/lib/src/main/java/com/diffplug/spotless/Formatter.java index 3f28d42df7..ce083e826a 100644 --- a/lib/src/main/java/com/diffplug/spotless/Formatter.java +++ b/lib/src/main/java/com/diffplug/spotless/Formatter.java @@ -35,8 +35,6 @@ import javax.annotation.Nullable; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - /** Formatter which performs the full formatting. */ public final class Formatter implements Serializable, AutoCloseable { private static final long serialVersionUID = 1L; @@ -59,13 +57,11 @@ private Formatter(String name, LineEnding.Policy lineEndingsPolicy, Charset enco } // override serialize output - @SuppressFBWarnings("ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD") private void writeObject(ObjectOutputStream out) throws IOException { out.writeObject(name); out.writeObject(lineEndingsPolicy); out.writeObject(encoding.name()); out.writeObject(rootDir.toString()); - ConfigurationCacheHack.SERIALIZE_FOR_ROUNDTRIP = true; out.writeObject(steps); out.writeObject(exceptionPolicy); } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java index 263826c9ca..6f123f2237 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java @@ -16,12 +16,12 @@ package com.diffplug.spotless; import java.io.IOException; -import java.io.ObjectStreamException; import java.io.Serializable; +import java.util.Objects; import edu.umd.cs.findbugs.annotations.Nullable; -class FormatterStepSerializationRoundtrip extends FormatterStepEqualityOnStateSerialization { +final class FormatterStepSerializationRoundtrip extends FormatterStepEqualityOnStateSerialization { private static final long serialVersionUID = 1L; private final String name; private final transient ThrowingEx.Supplier initializer; @@ -30,7 +30,7 @@ class FormatterStepSerializationRoundtrip equalityStateExtractor; private final SerializedFunction equalityStateToFormatter; - public FormatterStepSerializationRoundtrip(String name, ThrowingEx.Supplier initializer, SerializedFunction equalityStateExtractor, SerializedFunction equalityStateToFormatter) { + FormatterStepSerializationRoundtrip(String name, ThrowingEx.Supplier initializer, SerializedFunction equalityStateExtractor, SerializedFunction equalityStateToFormatter) { this.name = name; this.initializer = initializer; this.equalityStateExtractor = equalityStateExtractor; @@ -42,13 +42,17 @@ public String getName() { return name; } + private RoundtripState roundtripStateSupplier() throws Exception { + if (roundtripStateInternal == null) { + roundtripStateInternal = initializer.get(); + } + return roundtripStateInternal; + } + @Override protected EqualityState stateSupplier() throws Exception { if (equalityStateInternal == null) { - if (roundtripStateInternal == null) { - roundtripStateInternal = initializer.get(); - } - equalityStateInternal = equalityStateExtractor.apply(roundtripStateInternal); + equalityStateInternal = equalityStateExtractor.apply(roundtripStateSupplier()); } return equalityStateInternal; } @@ -58,25 +62,55 @@ protected FormatterFunc stateToFormatter(EqualityState equalityState) throws Exc return equalityStateToFormatter.apply(equalityState); } - // override serialize output - private void writeObject(java.io.ObjectOutputStream out) throws IOException { - if (ConfigurationCacheHack.SERIALIZE_FOR_ROUNDTRIP) { - if (roundtripStateInternal == null) { - roundtripStateInternal = ThrowingEx.get(initializer::get); + HackClone hackClone(boolean optimizeForEquality) { + return new HackClone<>(this, optimizeForEquality); + } + + static class HackClone implements Serializable { + transient FormatterStepSerializationRoundtrip original; + boolean optimizeForEquality; + @Nullable + FormatterStepSerializationRoundtrip cleaned; + + HackClone(@Nullable FormatterStepSerializationRoundtrip original, boolean optimizeForEquality) { + this.original = original; + this.optimizeForEquality = optimizeForEquality; + } + + private void writeObject(java.io.ObjectOutputStream out) throws IOException { + if (cleaned == null) { + cleaned = new FormatterStepSerializationRoundtrip(original.name, null, original.equalityStateExtractor, original.equalityStateToFormatter); + if (optimizeForEquality) { + cleaned.equalityStateInternal = ThrowingEx.get(original::stateSupplier); + } else { + cleaned.roundtripStateInternal = ThrowingEx.get(original::roundtripStateSupplier); + } } - equalityStateInternal = null; - } else { - equalityStateInternal = ThrowingEx.get(this::stateSupplier); - roundtripStateInternal = null; + out.defaultWriteObject(); } - out.defaultWriteObject(); - } - private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException { - in.defaultReadObject(); - } + public FormatterStep rehydrate() { + try { + throw new Exception("rehydrate optimizeForEquality=" + optimizeForEquality + " orig=" + original + " cleaned=" + cleaned); + } catch (Exception e) { + e.printStackTrace(); + } + return original != null ? original : Objects.requireNonNull(cleaned, "how is clean null if this has been serialized?"); + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + HackClone that = (HackClone) o; + return optimizeForEquality == that.optimizeForEquality && rehydrate().equals(that.rehydrate()); + } - private void readObjectNoData() throws ObjectStreamException { - throw new UnsupportedOperationException(); + @Override + public int hashCode() { + return rehydrate().hashCode() ^ Boolean.hashCode(optimizeForEquality); + } } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index 5e2fce2713..f54d4d20e7 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -17,7 +17,6 @@ import java.io.File; import java.nio.charset.Charset; -import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Objects; @@ -150,17 +149,17 @@ public File getOutputDirectory() { return outputDirectory; } - private final List stepsInternalRoundtrip = new ConfigurationCacheHack.StepList(ConfigurationCacheHack.OptimizeFor.ROUNDTRIP); - private final List stepsInternalEquality = new ConfigurationCacheHack.StepList(ConfigurationCacheHack.OptimizeFor.EQUALITY); + private final ConfigurationCacheHack.StepList stepsInternalRoundtrip = new ConfigurationCacheHack.StepList(ConfigurationCacheHack.OptimizeFor.ROUNDTRIP); + private final ConfigurationCacheHack.StepList stepsInternalEquality = new ConfigurationCacheHack.StepList(ConfigurationCacheHack.OptimizeFor.EQUALITY); @Internal - public List getStepsInternalRoundtrip() { - return Collections.unmodifiableList(stepsInternalRoundtrip); + public ConfigurationCacheHack.StepList getStepsInternalRoundtrip() { + return stepsInternalRoundtrip; } @Input - public List getStepsInternalEquality() { - return Collections.unmodifiableList(stepsInternalEquality); + public ConfigurationCacheHack.StepList getStepsInternalEquality() { + return stepsInternalEquality; } public void setSteps(List steps) { @@ -187,7 +186,7 @@ Formatter buildFormatter() { .lineEndingsPolicy(getLineEndingsPolicy().get()) .encoding(Charset.forName(encoding)) .rootDir(getProjectDir().get().getAsFile().toPath()) - .steps(stepsInternalRoundtrip) + .steps(stepsInternalRoundtrip.getSteps()) .exceptionPolicy(exceptionPolicy) .build(); } From 90ca5bf7b1bb1db3224137d24d365093a72d14af Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 12 Oct 2024 22:54:52 -0700 Subject: [PATCH 4/8] Cleanup. --- .../spotless/ConfigurationCacheHack.java | 103 ----------------- .../spotless/ConfigurationCacheHackList.java | 107 ++++++++++++++++++ .../FormatterStepSerializationRoundtrip.java | 12 +- .../gradle/spotless/SpotlessTask.java | 10 +- 4 files changed, 119 insertions(+), 113 deletions(-) delete mode 100644 lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java create mode 100644 lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java diff --git a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java deleted file mode 100644 index 1544139a15..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHack.java +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright 2024 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.spotless; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Objects; - -/** - * Gradle requires three things: - * - Gradle defines cache equality based on your serialized representation - * - Combined with remote build cache, you cannot have any absolute paths in - * your serialized representation - * - Combined with configuration cache, you must be able to roundtrip yourself - * through serialization - * - * These requirements are at odds with each other, as described in these issues - * - Gradle issue to define custom equality - * https://github.com/gradle/gradle/issues/29816 - * - Spotless plea for developer cache instead of configuration cache - * https://github.com/diffplug/spotless/issues/987 - * - Spotless cache miss bug fixed by this class - * https://github.com/diffplug/spotless/issues/2168 - * - * The point of this class is to create containers which can optimize the - * serialized representation for either - * - roundtrip integrity - * - equality - * - * It is a horrific hack, but it works, and it's the only way I can figure - * to make Spotless work with all of Gradle's cache systems. - */ -public class ConfigurationCacheHack { - public enum OptimizeFor { - ROUNDTRIP, EQUALITY, - } - - public static class StepList implements java.io.Serializable { - private final boolean optimizeForEquality; - private final ArrayList backingList = new ArrayList<>(); - - public StepList(OptimizeFor optimizeFor) { - this.optimizeForEquality = optimizeFor == OptimizeFor.EQUALITY; - } - - public void clear() { - backingList.clear(); - } - - public void addAll(Collection c) { - for (FormatterStep step : c) { - if (step instanceof FormatterStepSerializationRoundtrip) { - var clone = ((FormatterStepSerializationRoundtrip) step).hackClone(optimizeForEquality); - backingList.add(clone); - } else { - backingList.add(step); - } - } - } - - public List getSteps() { - var result = new ArrayList(backingList.size()); - for (Object obj : backingList) { - if (obj instanceof FormatterStepSerializationRoundtrip.HackClone) { - result.add(((FormatterStepSerializationRoundtrip.HackClone) obj).rehydrate()); - } else { - result.add((FormatterStep) obj); - } - } - return result; - } - - @Override - public boolean equals(Object o) { - if (this == o) - return true; - if (o == null || getClass() != o.getClass()) - return false; - StepList stepList = (StepList) o; - return optimizeForEquality == stepList.optimizeForEquality && - backingList.equals(stepList.backingList); - } - - @Override - public int hashCode() { - return Objects.hash(optimizeForEquality, backingList); - } - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java new file mode 100644 index 0000000000..ac2f344313 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java @@ -0,0 +1,107 @@ +/* + * Copyright 2024 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.spotless; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Objects; + +/** + * Gradle requires three things: + * - Gradle defines cache equality based on your serialized representation + * - Combined with remote build cache, you cannot have any absolute paths in + * your serialized representation + * - Combined with configuration cache, you must be able to roundtrip yourself + * through serialization + * + * These requirements are at odds with each other, as described in these issues + * - Gradle issue to define custom equality + * https://github.com/gradle/gradle/issues/29816 + * - Spotless plea for developer cache instead of configuration cache + * https://github.com/diffplug/spotless/issues/987 + * - Spotless cache miss bug fixed by this class + * https://github.com/diffplug/spotless/issues/2168 + * + * This class is a `List` which can optimize the + * serialized representation for either + * - roundtrip integrity + * - OR + * - equality + * + * Because it is not possible to provide both at the same time. + * It is a horrific hack, but it works, and it's the only way I can figure + * to make Spotless work with all of Gradle's cache systems at once. + */ +public class ConfigurationCacheHackList implements java.io.Serializable { + private final boolean optimizeForEquality; + private final ArrayList backingList = new ArrayList<>(); + + public static ConfigurationCacheHackList forEquality() { + return new ConfigurationCacheHackList(true); + } + + public static ConfigurationCacheHackList forRoundtrip() { + return new ConfigurationCacheHackList(false); + } + + private ConfigurationCacheHackList(boolean optimizeForEquality) { + this.optimizeForEquality = optimizeForEquality; + } + + public void clear() { + backingList.clear(); + } + + public void addAll(Collection c) { + for (FormatterStep step : c) { + if (step instanceof FormatterStepSerializationRoundtrip) { + var clone = ((FormatterStepSerializationRoundtrip) step).hackClone(optimizeForEquality); + backingList.add(clone); + } else { + backingList.add(step); + } + } + } + + public List getSteps() { + var result = new ArrayList(backingList.size()); + for (Object obj : backingList) { + if (obj instanceof FormatterStepSerializationRoundtrip.HackClone) { + result.add(((FormatterStepSerializationRoundtrip.HackClone) obj).rehydrate()); + } else { + result.add((FormatterStep) obj); + } + } + return result; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + ConfigurationCacheHackList stepList = (ConfigurationCacheHackList) o; + return optimizeForEquality == stepList.optimizeForEquality && + backingList.equals(stepList.backingList); + } + + @Override + public int hashCode() { + return Objects.hash(optimizeForEquality, backingList); + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java index 6f123f2237..edc8dd67c9 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java @@ -66,6 +66,13 @@ protected FormatterFunc stateToFormatter(EqualityState equalityState) throws Exc return new HackClone<>(this, optimizeForEquality); } + /** + * This class has one setting (optimizeForEquality) and two pieces of data + * - the original step, which is marked transient so it gets discarded during serialization + * - the cleaned step, which is lazily created during serialization, and the serialized form is optimized for either equality or roundtrip integrity + * + * It works in conjunction with ConfigurationCacheHackList to allow Spotless to work with all of Gradle's cache systems. + */ static class HackClone implements Serializable { transient FormatterStepSerializationRoundtrip original; boolean optimizeForEquality; @@ -90,11 +97,6 @@ private void writeObject(java.io.ObjectOutputStream out) throws IOException { } public FormatterStep rehydrate() { - try { - throw new Exception("rehydrate optimizeForEquality=" + optimizeForEquality + " orig=" + original + " cleaned=" + cleaned); - } catch (Exception e) { - e.printStackTrace(); - } return original != null ? original : Objects.requireNonNull(cleaned, "how is clean null if this has been serialized?"); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index f54d4d20e7..dc0ec744a8 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -35,7 +35,7 @@ import org.gradle.api.tasks.PathSensitivity; import org.gradle.work.Incremental; -import com.diffplug.spotless.ConfigurationCacheHack; +import com.diffplug.spotless.ConfigurationCacheHackList; import com.diffplug.spotless.FormatExceptionPolicy; import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.Formatter; @@ -149,16 +149,16 @@ public File getOutputDirectory() { return outputDirectory; } - private final ConfigurationCacheHack.StepList stepsInternalRoundtrip = new ConfigurationCacheHack.StepList(ConfigurationCacheHack.OptimizeFor.ROUNDTRIP); - private final ConfigurationCacheHack.StepList stepsInternalEquality = new ConfigurationCacheHack.StepList(ConfigurationCacheHack.OptimizeFor.EQUALITY); + private final ConfigurationCacheHackList stepsInternalRoundtrip = ConfigurationCacheHackList.forRoundtrip(); + private final ConfigurationCacheHackList stepsInternalEquality = ConfigurationCacheHackList.forEquality(); @Internal - public ConfigurationCacheHack.StepList getStepsInternalRoundtrip() { + public ConfigurationCacheHackList getStepsInternalRoundtrip() { return stepsInternalRoundtrip; } @Input - public ConfigurationCacheHack.StepList getStepsInternalEquality() { + public ConfigurationCacheHackList getStepsInternalEquality() { return stepsInternalEquality; } From 90cb7bbe525047f9710e6735e55e6e014569a149 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 13 Oct 2024 00:39:59 -0700 Subject: [PATCH 5/8] Fix spotbugs. --- .../com/diffplug/spotless/ConfigurationCacheHackList.java | 1 + .../spotless/FormatterStepSerializationRoundtrip.java | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java index ac2f344313..6bbc6ff2a6 100644 --- a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java +++ b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java @@ -47,6 +47,7 @@ * to make Spotless work with all of Gradle's cache systems at once. */ public class ConfigurationCacheHackList implements java.io.Serializable { + private static final long serialVersionUID = 1L; private final boolean optimizeForEquality; private final ArrayList backingList = new ArrayList<>(); diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java index edc8dd67c9..d2f0e4b5b5 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java @@ -20,10 +20,12 @@ import java.util.Objects; import edu.umd.cs.findbugs.annotations.Nullable; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; final class FormatterStepSerializationRoundtrip extends FormatterStepEqualityOnStateSerialization { private static final long serialVersionUID = 1L; private final String name; + @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "HackClone") private final transient ThrowingEx.Supplier initializer; private @Nullable RoundtripState roundtripStateInternal; private @Nullable EqualityState equalityStateInternal; @@ -74,6 +76,8 @@ protected FormatterFunc stateToFormatter(EqualityState equalityState) throws Exc * It works in conjunction with ConfigurationCacheHackList to allow Spotless to work with all of Gradle's cache systems. */ static class HackClone implements Serializable { + private static final long serialVersionUID = 1L; + @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "HackClone") transient FormatterStepSerializationRoundtrip original; boolean optimizeForEquality; @Nullable @@ -84,6 +88,7 @@ static class HackClone Date: Sun, 13 Oct 2024 01:21:29 -0700 Subject: [PATCH 6/8] We still need to trigger storing the roundtripState on writeObject. --- .../spotless/FormatterStepSerializationRoundtrip.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java index d2f0e4b5b5..81548bf759 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java @@ -64,6 +64,13 @@ protected FormatterFunc stateToFormatter(EqualityState equalityState) throws Exc return equalityStateToFormatter.apply(equalityState); } + private void writeObject(java.io.ObjectOutputStream out) throws IOException { + if (roundtripStateInternal == null) { + roundtripStateInternal = ThrowingEx.get(this::roundtripStateSupplier); + } + out.defaultWriteObject(); + } + HackClone hackClone(boolean optimizeForEquality) { return new HackClone<>(this, optimizeForEquality); } From c3252a28bf601c3bf8be69f69b85cce475f28165 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Oct 2024 09:33:23 -0700 Subject: [PATCH 7/8] Fixup the writeObject for the new HackClone situation. --- .../diffplug/spotless/FormatterStepSerializationRoundtrip.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java index 81548bf759..35167f04c6 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java @@ -65,7 +65,7 @@ protected FormatterFunc stateToFormatter(EqualityState equalityState) throws Exc } private void writeObject(java.io.ObjectOutputStream out) throws IOException { - if (roundtripStateInternal == null) { + if (initializer != null && roundtripStateInternal == null) { roundtripStateInternal = ThrowingEx.get(this::roundtripStateSupplier); } out.defaultWriteObject(); From 76c5bb36d932992309e25290a11298dfbaa532be Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Oct 2024 09:46:41 -0700 Subject: [PATCH 8/8] Better comments / documentation. --- .../FormatterStepSerializationRoundtrip.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java index 35167f04c6..f7f95076c2 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepSerializationRoundtrip.java @@ -65,8 +65,18 @@ protected FormatterFunc stateToFormatter(EqualityState equalityState) throws Exc } private void writeObject(java.io.ObjectOutputStream out) throws IOException { - if (initializer != null && roundtripStateInternal == null) { - roundtripStateInternal = ThrowingEx.get(this::roundtripStateSupplier); + if (initializer == null) { + // then this instance was created by Gradle's ConfigurationCacheHackList and the following will hold true + if (roundtripStateInternal == null && equalityStateInternal == null) { + throw new IllegalStateException("If the initializer was null, then one of roundtripStateInternal or equalityStateInternal should be non-null, and neither was"); + } + } else { + // this was a normal instance, which means we need to encode to roundtripStateInternal (since the initializer might not be serializable) + // and there's no reason to keep equalityStateInternal since we can always recompute it + if (roundtripStateInternal == null) { + roundtripStateInternal = ThrowingEx.get(this::roundtripStateSupplier); + } + equalityStateInternal = null; } out.defaultWriteObject(); }