diff --git a/CHANGES.md b/CHANGES.md index d5aeff6add..cfd1f363fd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,8 @@ This document is intended for Spotless developers. We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +### Fixed +- Concurrent P2 provisioning (parallel multi-project Gradle fingerprinting of `eclipse()` / `greclipse()` steps) no longer races Solstice's on-disk cache; also `ConfigurationCacheHackList.toString()` no longer evaluates step state (which could re-trigger provisioning while Gradle reports "cannot be serialized"). ([#3004](https://github.com/diffplug/spotless/issues/3004)) ### Changes - Bump default `adocfmt` version `0.2.0` -> `0.3.1`, which adds table formatting support (`formatTables`, `tableLayout`, `tableMaxLineWidth`, `tableBlankLines`). diff --git a/gradle.properties b/gradle.properties index ddb7a4ab97..5adafd62cc 100644 --- a/gradle.properties +++ b/gradle.properties @@ -3,7 +3,12 @@ org.gradle.jvmargs=-Xmx4g -XX:MaxMetaspaceSize=1024m -Dfile.encoding=UTF-8 org.gradle.parallel=true org.gradle.caching=true org.gradle.configuration-cache=true -org.gradle.configuration-cache.parallel=true +# Parallel fingerprinting of eclipse()/greclipse() steps races Solstice's on-disk +# P2 cache, failing as "Cannot fingerprint input property 'stepsInternalEquality'" +# / "Failed to provision P2 dependencies". We format ourselves with the *published* +# plugin (see settings.gradle), so the fix in this repo can't help until it ships. +# Re-enable once settings.gradle pins a Spotless containing the #3004 fix. +org.gradle.configuration-cache.parallel=false org.gradle.tooling.parallel=true name=spotless diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/P2Provisioner.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/P2Provisioner.java index 3ce9486476..21817ff275 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/P2Provisioner.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/P2Provisioner.java @@ -50,28 +50,42 @@ List provisionP2Dependencies( Provisioner mavenProvisioner, @Nullable File cacheDirectory) throws IOException; - /** Creates a non-caching P2Provisioner for simple use cases. */ + /** + * Creates a non-caching P2Provisioner for simple use cases. + *

+ * All queries are serialized on {@code P2Provisioner.class}. Gradle may fingerprint + * many Spotless tasks in parallel; each fingerprint serializes the equality + * {@code ConfigurationCacheHackList}, which eagerly resolves Eclipse/P2 jars. + * Concurrent Solstice queries race on the on-disk cache and fail with + * {@code Failed to provision P2 dependencies}, reported by Gradle as + * "ConfigurationCacheHackList cannot be serialized" + * (#3004, + * #2331). + */ static P2Provisioner createDefault() { return (modelWrapper, mavenProvisioner, cacheDirectory) -> { - try { - if (cacheDirectory != null) { - CacheLocations.override_p2data = cacheDirectory; - } - P2Model model = modelWrapper.unwrap(); - P2QueryResult query = model.query(P2ClientCache.PREFER_OFFLINE, P2QueryCache.ALLOW); - var classpath = new ArrayList(); - var mavenDeps = new ArrayList(); - mavenDeps.add("dev.equo.ide:solstice:1.8.1"); - mavenDeps.add("com.diffplug.durian:durian-swt.os:4.3.1"); - mavenDeps.addAll(query.getJarsOnMavenCentral()); - classpath.addAll(mavenProvisioner.provisionWithTransitives(false, mavenDeps)); - classpath.addAll(query.getJarsNotOnMavenCentral()); - for (var nested : NestedJars.inFiles(query.getJarsNotOnMavenCentral()).extractAllNestedJars()) { - classpath.add(nested.getValue()); + // Serialize all P2 queries in this JVM — Solstice's cache is not concurrent-safe. + synchronized (P2Provisioner.class) { + try { + if (cacheDirectory != null) { + CacheLocations.override_p2data = cacheDirectory; + } + P2Model model = modelWrapper.unwrap(); + P2QueryResult query = model.query(P2ClientCache.PREFER_OFFLINE, P2QueryCache.ALLOW); + var classpath = new ArrayList(); + var mavenDeps = new ArrayList(); + mavenDeps.add("dev.equo.ide:solstice:1.8.2"); + mavenDeps.add("com.diffplug.durian:durian-swt.os:4.3.1"); + mavenDeps.addAll(query.getJarsOnMavenCentral()); + classpath.addAll(mavenProvisioner.provisionWithTransitives(false, mavenDeps)); + classpath.addAll(query.getJarsNotOnMavenCentral()); + for (var nested : NestedJars.inFiles(query.getJarsNotOnMavenCentral()).extractAllNestedJars()) { + classpath.add(nested.getValue()); + } + return classpath; + } catch (Exception e) { + throw new IOException("Failed to provision P2 dependencies", e); } - return classpath; - } catch (Exception e) { - throw new IOException("Failed to provision P2 dependencies", e); } }; } diff --git a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java index 7ae51024f2..4e025462e3 100644 --- a/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java +++ b/lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java @@ -1,5 +1,5 @@ /* - * Copyright 2024-2025 DiffPlug + * Copyright 2024-2026 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,13 @@ import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.IdentityHashMap; import java.util.List; import java.util.Objects; +import java.util.Set; + +import javax.annotation.Nullable; import com.diffplug.spotless.yaml.SerializeToByteArrayHack; @@ -60,11 +65,47 @@ public final class ConfigurationCacheHackList implements Serializable { private boolean optimizeForEquality; private ArrayList backingList = new ArrayList<>(); + /** + * The failure from the most recent serialization attempt, if any. Not part of the + * serialized form - it exists only so {@link #toString()} can report why serialization + * failed without re-evaluating any step state. + */ + @Nullable private transient volatile String serializationFailure; + private boolean shouldWeSerializeToByteArrayFirst() { return backingList.stream().anyMatch(SerializeToByteArrayHack.class::isInstance); } private void writeObject(ObjectOutputStream out) throws IOException { + try { + writeSteps(out); + } catch (IOException | RuntimeException e) { + // Gradle reports a fingerprinting failure as "value '' cannot be + // serialized" and discards the cause, so stash it where toString() can report + // it. Otherwise the actionable message (e.g. "P2 dependencies not predeclared") + // is lost and the user only sees "cannot be serialized". See #3004. + serializationFailure = describeFailure(e); + throw e; + } + } + + /** Walks the cause chain so nested messages survive into {@link #toString()}. */ + private static String describeFailure(Throwable e) { + StringBuilder causes = new StringBuilder(); + Set seen = Collections.newSetFromMap(new IdentityHashMap<>()); + for (Throwable t = e; t != null && seen.add(t); t = t.getCause()) { + String message = t.getMessage(); + if (message != null && !message.isEmpty()) { + if (causes.length() > 0) { + causes.append(" > "); + } + causes.append(message); + } + } + return causes.length() == 0 ? e.getClass().getName() : causes.toString(); + } + + private void writeSteps(ObjectOutputStream out) throws IOException { boolean serializeToByteArrayFirst = shouldWeSerializeToByteArrayFirst(); out.writeBoolean(serializeToByteArrayFirst); out.writeBoolean(optimizeForEquality); @@ -150,4 +191,28 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(optimizeForEquality, backingList); } + + /** + * Must not call {@link #hashCode()} — that fingerprints every step and may provision + * P2/Maven deps. Gradle includes this value in "cannot be serialized" messages, so a + * side-effecting {@code toString} re-triggers provisioning while the build is already + * failing (see #3004). + *

+ * Gradle builds that message only after serialization has already thrown, so any + * failure is reported from {@link #serializationFailure} rather than by re-evaluating + * the steps. That keeps actionable errors such as "P2 dependencies not predeclared" + * visible to the user. + */ + @Override + public String toString() { + StringBuilder builder = new StringBuilder(getClass().getName()) + .append('@').append(Integer.toHexString(System.identityHashCode(this))) + .append("[optimizeForEquality=").append(optimizeForEquality) + .append(", size=").append(backingList.size()); + String failure = serializationFailure; + if (failure != null) { + builder.append(", failure=").append(failure); + } + return builder.append(']').toString(); + } } diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 7923949bb0..044101d79f 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`). ## [Unreleased] +### Fixed +- Parallel multi-project builds no longer intermittently fail with "Cannot fingerprint input property 'stepsInternalEquality': ConfigurationCacheHackList cannot be serialized" / "Failed to provision P2 dependencies" when using `eclipse()` (or other P2-backed steps). Subprojects now share one deduping P2 provisioner and P2 queries are serialized process-wide. ([#3004](https://github.com/diffplug/spotless/issues/3004)) ### Changes - Bump default `adocfmt` version `0.2.0` -> `0.3.1`, which adds table formatting support (`formatTables`, `tableLayout`, `tableMaxLineWidth`, `tableBlankLines`). diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index 90f282d873..c6fd993d20 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -59,10 +59,11 @@ public abstract class SpotlessTaskService implements BuildService apply = Collections.synchronizedMap(new HashMap<>()); private final Map source = Collections.synchronizedMap(new HashMap<>()); private final Map provisioner = Collections.synchronizedMap(new HashMap<>()); - private final Map p2Provisioner = Collections.synchronizedMap(new HashMap<>()); @Nullable GradleProvisioner.DedupingProvisioner predeclaredProvisioner; @Nullable GradleProvisioner.DedupingP2Provisioner predeclaredP2Provisioner; + /** Shared across subprojects so parallel fingerprinting reuses one P2 cache + lock. */ + @Nullable private volatile GradleProvisioner.DedupingP2Provisioner sharedP2Provisioner; @Nullable RegisterDependenciesTask registerDependenciesTask; Provisioner provisionerFor(SpotlessExtension spotless) { @@ -84,12 +85,31 @@ P2Provisioner p2ProvisionerFor(SpotlessExtension spotless) { if (predeclaredP2Provisioner != null) { return predeclaredP2Provisioner.cachedOnly; } else { - return p2Provisioner.computeIfAbsent(spotless.project.getPath(), - unused -> new GradleProvisioner.DedupingP2Provisioner(P2Provisioner.createDefault(), GradleProvisioner.defaultP2CacheDirectory(spotless.project))); + // One DedupingP2Provisioner for the whole build (not per-project). Parallel + // multi-project fingerprinting of eclipse()/greclipse() steps otherwise races + // on Solstice's on-disk P2 cache — Gradle then reports + // "ConfigurationCacheHackList cannot be serialized" (#3004). + return sharedP2Provisioner(spotless.project); } } } + private GradleProvisioner.DedupingP2Provisioner sharedP2Provisioner(Project project) { + GradleProvisioner.DedupingP2Provisioner local = sharedP2Provisioner; + if (local == null) { + synchronized (this) { + local = sharedP2Provisioner; + if (local == null) { + local = new GradleProvisioner.DedupingP2Provisioner( + P2Provisioner.createDefault(), + GradleProvisioner.defaultP2CacheDirectory(project)); + sharedP2Provisioner = local; + } + } + } + return local; + } + void registerSourceAlreadyRan(SpotlessTask task) { source.put(task.getPath(), task); } diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 7fe2f88f30..363020aa4c 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -3,10 +3,10 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +### Fixed +- Concurrent P2 provisioning no longer races Solstice's on-disk cache (affects Eclipse-based formatters under parallel builds). ([#3004](https://github.com/diffplug/spotless/issues/3004)) ### Changes - Bump default `adocfmt` version `0.2.0` -> `0.3.1`, which adds table formatting support (``, ``, ``, ``). - -### Changes - Add support to apply alternate license header within same format ([#872](https://github.com/diffplug/spotless/issues/872)) - Add support to skip license header application based on source file content pattern ([#650](https://github.com/diffplug/spotless/issues/650)). diff --git a/testlib/src/test/java/com/diffplug/spotless/ConfigurationCacheHackListTest.java b/testlib/src/test/java/com/diffplug/spotless/ConfigurationCacheHackListTest.java new file mode 100644 index 0000000000..4c8be1c136 --- /dev/null +++ b/testlib/src/test/java/com/diffplug/spotless/ConfigurationCacheHackListTest.java @@ -0,0 +1,111 @@ +/* + * Copyright 2024-2026 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 static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.Serializable; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.Test; + +class ConfigurationCacheHackListTest { + + /** Step whose equality/hashCode/serialization forces state evaluation. */ + private static FormatterStep lazyStep(String name, AtomicInteger stateEvals, Serializable state) { + return FormatterStep.createLazy(name, + () -> { + stateEvals.incrementAndGet(); + return state; + }, + SerializedFunction.identity(), + eq -> (FormatterFunc) (s -> s)); + } + + /** Step whose state evaluation always fails, like an unresolvable P2/Maven dependency. */ + private static FormatterStep explodingStep(String name, AtomicInteger stateEvals, String message) { + return FormatterStep.createLazy(name, + () -> { + stateEvals.incrementAndGet(); + throw new RuntimeException(message); + }, + SerializedFunction.identity(), + eq -> (FormatterFunc) (s -> s)); + } + + @Test + void toStringReportsSerializationFailureWithoutReEvaluating() throws Exception { + AtomicInteger evals = new AtomicInteger(); + ConfigurationCacheHackList list = ConfigurationCacheHackList.forEquality(); + list.addAll(List.of(explodingStep("unresolvable", evals, "P2 dependencies not predeclared"))); + + try (ObjectOutputStream out = new ObjectOutputStream(new ByteArrayOutputStream())) { + assertThatThrownBy(() -> out.writeObject(list)).isNotNull(); + } + int evalsAfterSerialize = evals.get(); + assertThat(evalsAfterSerialize).as("serialization evaluates state").isPositive(); + + // Gradle renders this value into "cannot be serialized" and drops the cause, so the + // actionable message has to survive here or the user never sees it (#3004). + assertThat(list.toString()).contains("P2 dependencies not predeclared"); + assertThat(evals.get()).as("toString must not re-evaluate step state").isEqualTo(evalsAfterSerialize); + } + + @Test + void toStringDoesNotEvaluateStepState() { + AtomicInteger evals = new AtomicInteger(); + ConfigurationCacheHackList list = ConfigurationCacheHackList.forEquality(); + list.addAll(List.of(lazyStep("expensive", evals, "state"))); + + // Gradle includes this value in "cannot be serialized" error messages. + // Default Object.toString() calls hashCode(), which fingerprints steps and + // may provision P2 deps — re-triggering the failure being reported (#3004). + String text = list.toString(); + assertThat(text).contains("ConfigurationCacheHackList"); + assertThat(text).contains("optimizeForEquality=true"); + assertThat(text).contains("size=1"); + assertThat(evals.get()).as("toString must not evaluate step state").isZero(); + } + + @Test + void equalityListRoundtripsThroughJavaSerialization() throws Exception { + AtomicInteger evals = new AtomicInteger(); + ConfigurationCacheHackList original = ConfigurationCacheHackList.forEquality(); + original.addAll(List.of(lazyStep("plain", evals, "eq-state"))); + + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ObjectOutputStream out = new ObjectOutputStream(bytes)) { + out.writeObject(original); + } + assertThat(evals.get()).as("serializing equality list evaluates state once").isEqualTo(1); + + ConfigurationCacheHackList restored; + try (ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(bytes.toByteArray()))) { + restored = (ConfigurationCacheHackList) in.readObject(); + } + assertThat(restored.getSteps()).hasSize(1); + assertThat(restored.getSteps().get(0).getName()).isEqualTo("plain"); + // toString after restore must still be side-effect free + assertThatCode(restored::toString).doesNotThrowAnyException(); + } +}