git pre push hook gradle config cache fix + parallel execution handling

Author: lazer-dev

lib/src/main/java/com/diffplug/spotless/GitPrePushHookInstaller.java

@@ -24,6 +24,8 @@
 import java.nio.file.Files;
 import java.util.Locale;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 /**
  * Abstract class responsible for installing a Git pre-push hook in a repository.
  * This class ensures that specific checks and logic are run before a push operation in Git.
@@ -35,6 +37,10 @@ public abstract class GitPrePushHookInstaller {
 	private static final String HOOK_HEADER = "##### SPOTLESS HOOK START #####";
 	private static final String HOOK_FOOTER = "##### SPOTLESS HOOK END #####";
 
+	private static final Object LOCK = new Object();
+
+	private static volatile boolean installing = false;
+
 	/**
 	 * Logger for recording informational and error messages during the installation process.
 	 */
@@ -56,6 +62,41 @@ public GitPrePushHookInstaller(GitPreHookLogger logger, File root) {
 		this.root = requireNonNull(root, "root file can not be null");
 	}
 
+	/**
+	 * Installs the Git pre-push hook while ensuring thread safety and preventing parallel installations.
+	 *
+	 * The method:
+	 * 1. Uses a thread-safe mechanism to prevent concurrent installations
+	 * 2. If a parallel installation is detected, logs a warning and skips the installation
+	 * 3. Uses a synchronized block with a static lock object to ensure thread safety
+	 *
+	 * The installation process sets a flag during installation and resets it afterwards,
+	 * using the {@link #doInstall()} method to perform the actual installation.
+	 *
+	 * @throws Exception if any error occurs during installation
+	 */
+	@SuppressFBWarnings(value = "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD", justification = "This is a safe usage of a static lock object")
+	public void install() throws Exception {
+		if (installing) {
+			logger.warn("Parallel Spotless Git pre-push hook installation detected, skipping installation");
+			return;
+		}
+
+		synchronized (LOCK) {
+			if (installing) {
+				logger.warn("Parallel Spotless Git pre-push hook installation detected, skipping installation");
+				return;
+			}
+
+			try {
+				installing = true;
+				doInstall();
+			} finally {
+				installing = false;
+			}
+		}
+	}
+
 	/**
 	 * Installs the Git pre-push hook into the repository.
 	 *
@@ -70,7 +111,7 @@ public GitPrePushHookInstaller(GitPreHookLogger logger, File root) {
 	 *
 	 * @throws Exception if any error occurs during installation.
 	 */
-	public void install() throws Exception {
+	private void doInstall() throws Exception {
 		logger.info("Installing git pre-push hook");
 
 		if (!isGitInstalled()) {

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtensionImpl.java

@@ -41,6 +41,7 @@ public SpotlessExtensionImpl(Project project) {
 		rootInstallPreHook = project.getTasks().register(EXTENSION + INSTALL_GIT_PRE_PUSH_HOOK, SpotlessInstallPrePushHookTask.class, task -> {
 			task.setGroup(BUILD_SETUP_TASK_GROUP);
 			task.setDescription(INSTALL_GIT_PRE_PUSH_HOOK_DESCRIPTION);
+			task.getRootDir().set(project.getRootDir());
 		});
 
 		project.afterEvaluate(unused -> {

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessInstallPrePushHookTask.java

@@ -15,7 +15,11 @@
  */
 package com.diffplug.gradle.spotless;
 
+import java.io.File;
+
 import org.gradle.api.DefaultTask;
+import org.gradle.api.provider.Property;
+import org.gradle.api.tasks.Internal;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.work.DisableCachingByDefault;
 
@@ -30,7 +34,10 @@
  * <p>The task leverages {@link GitPrePushHookInstallerGradle} to implement the installation process.
  */
 @DisableCachingByDefault(because = "not worth caching")
-public class SpotlessInstallPrePushHookTask extends DefaultTask {
+public abstract class SpotlessInstallPrePushHookTask extends DefaultTask {
+
+	@Internal
+	abstract Property<File> getRootDir();
 
 	/**
 	 * Executes the task to install the Git pre-push hook.
@@ -60,7 +67,7 @@ public void error(String format, Object... arguments) {
 			}
 		};
 
-		final var installer = new GitPrePushHookInstallerGradle(logger, getProject().getRootDir());
+		final var installer = new GitPrePushHookInstallerGradle(logger, getRootDir().get());
 		installer.install();
 	}
 }

plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpotlessInstallPrePushHookTaskTest.java

@@ -35,7 +35,7 @@ public void should_create_pre_hook_file_when_hook_file_does_not_exists() throws
 
 		// when
 		var output = gradleRunner()
-				.withArguments("spotlessInstallGitPrePushHook")
+				.withArguments("spotlessInstallGitPrePushHook", "--system-prop=org.gradle.configuration-cache=true")
 				.build()
 				.getOutput();
 
@@ -65,7 +65,7 @@ public void should_append_to_existing_pre_hook_file_when_hook_file_exists() thro
 
 		// when
 		final var output = gradleRunner()
-				.withArguments("spotlessInstallGitPrePushHook")
+				.withArguments("spotlessInstallGitPrePushHook", "--system-prop=org.gradle.configuration-cache=true")
 				.build()
 				.getOutput();
 

testlib/src/test/java/com/diffplug/spotless/GitPrePushHookInstallerTest.java

@@ -17,10 +17,12 @@
 
 import static com.diffplug.spotless.GitPrePushHookInstaller.Executor.GRADLE;
 import static com.diffplug.spotless.GitPrePushHookInstaller.Executor.MAVEN;
+import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
 
-import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.stream.IntStream;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -31,7 +33,7 @@
 class GitPrePushHookInstallerTest extends ResourceHarness {
 	private final static String OS = System.getProperty("os.name");
 
-	private final List<String> logs = new ArrayList<>();
+	private final List<String> logs = new CopyOnWriteArrayList<>();
 	private final GitPreHookLogger logger = new GitPreHookLogger() {
 		@Override
 		public void info(String format, Object... arguments) {
@@ -68,9 +70,9 @@ public void should_not_create_pre_hook_file_when_git_is_not_installed() throws E
 		gradle.install();
 
 		// then
-		assertThat(logs).hasSize(2);
-		assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
-		assertThat(logs).element(1).isEqualTo("Git not found in root directory");
+		assertThat(logs).containsExactly(
+				"Installing git pre-push hook",
+				"Git not found in root directory");
 		assertThat(newFile(".git/hooks/pre-push")).doesNotExist();
 	}
 
@@ -84,11 +86,11 @@ public void should_use_global_gradle_when_gradlew_is_not_installed() throws Exce
 		gradle.install();
 
 		// then
-		assertThat(logs).hasSize(4);
-		assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
-		assertThat(logs).element(1).isEqualTo("Git pre-push hook not found, creating it");
-		assertThat(logs).element(2).isEqualTo("Local gradle wrapper (gradlew) not found, falling back to global command 'gradle'");
-		assertThat(logs).element(3).isEqualTo("Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
+		assertThat(logs).containsExactly(
+				"Installing git pre-push hook",
+				"Git pre-push hook not found, creating it",
+				"Local gradle wrapper (gradlew) not found, falling back to global command 'gradle'",
+				"Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
 
 		final var content = gradleHookContent("git_pre_hook/pre-push.created-tpl", ExecutorType.GLOBAL);
 		assertFile(".git/hooks/pre-push").hasContent(content);
@@ -108,10 +110,10 @@ public void should_reinstall_pre_hook_file_when_hook_already_installed() throws
 		gradle.install();
 
 		// then
-		assertThat(logs).hasSize(3);
-		assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
-		assertThat(logs).element(1).isEqualTo("Git pre-push hook already installed, reinstalling it");
-		assertThat(logs).element(2).isEqualTo("Git pre-push hook installed successfully to the file " + hookFile.getAbsolutePath());
+		assertThat(logs).containsExactly(
+				"Installing git pre-push hook",
+				"Git pre-push hook already installed, reinstalling it",
+				"Git pre-push hook installed successfully to the file " + hookFile.getAbsolutePath());
 
 		final var content = gradleHookContent("git_pre_hook/pre-push.existing-installed-end-tpl", ExecutorType.WRAPPER);
 		assertFile(".git/hooks/pre-push").hasContent(content);
@@ -131,10 +133,10 @@ public void should_reinstall_pre_hook_file_when_hook_already_installed_in_the_mi
 		gradle.install();
 
 		// then
-		assertThat(logs).hasSize(3);
-		assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
-		assertThat(logs).element(1).isEqualTo("Git pre-push hook already installed, reinstalling it");
-		assertThat(logs).element(2).isEqualTo("Git pre-push hook installed successfully to the file " + hookFile.getAbsolutePath());
+		assertThat(logs).containsExactly(
+				"Installing git pre-push hook",
+				"Git pre-push hook already installed, reinstalling it",
+				"Git pre-push hook installed successfully to the file " + hookFile.getAbsolutePath());
 
 		final var content = gradleHookContent("git_pre_hook/pre-push.existing-reinstalled-middle-tpl", ExecutorType.WRAPPER);
 		assertFile(".git/hooks/pre-push").hasContent(content);
@@ -171,10 +173,10 @@ public void should_create_pre_hook_file_when_hook_file_does_not_exists() throws
 		gradle.install();
 
 		// then
-		assertThat(logs).hasSize(3);
-		assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
-		assertThat(logs).element(1).isEqualTo("Git pre-push hook not found, creating it");
-		assertThat(logs).element(2).isEqualTo("Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
+		assertThat(logs).containsExactly(
+				"Installing git pre-push hook",
+				"Git pre-push hook not found, creating it",
+				"Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
 
 		final var content = gradleHookContent("git_pre_hook/pre-push.created-tpl", ExecutorType.WRAPPER);
 		assertFile(".git/hooks/pre-push").hasContent(content);
@@ -192,9 +194,9 @@ public void should_append_to_existing_pre_hook_file_when_hook_file_exists() thro
 		gradle.install();
 
 		// then
-		assertThat(logs).hasSize(2);
-		assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
-		assertThat(logs).element(1).isEqualTo("Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
+		assertThat(logs).containsExactly(
+				"Installing git pre-push hook",
+				"Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
 
 		final var content = gradleHookContent("git_pre_hook/pre-push.existing-installed-end-tpl", ExecutorType.WRAPPER);
 		assertFile(".git/hooks/pre-push").hasContent(content);
@@ -211,10 +213,10 @@ public void should_create_pre_hook_file_for_maven_when_hook_file_does_not_exists
 		gradle.install();
 
 		// then
-		assertThat(logs).hasSize(3);
-		assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
-		assertThat(logs).element(1).isEqualTo("Git pre-push hook not found, creating it");
-		assertThat(logs).element(2).isEqualTo("Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
+		assertThat(logs).containsExactly(
+				"Installing git pre-push hook",
+				"Git pre-push hook not found, creating it",
+				"Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
 
 		final var content = mavenHookContent("git_pre_hook/pre-push.created-tpl", ExecutorType.WRAPPER);
 		assertFile(".git/hooks/pre-push").hasContent(content);
@@ -230,11 +232,11 @@ public void should_use_global_maven_when_maven_wrapper_is_not_installed() throws
 		gradle.install();
 
 		// then
-		assertThat(logs).hasSize(4);
-		assertThat(logs).element(0).isEqualTo("Installing git pre-push hook");
-		assertThat(logs).element(1).isEqualTo("Git pre-push hook not found, creating it");
-		assertThat(logs).element(2).isEqualTo("Local maven wrapper (mvnw) not found, falling back to global command 'mvn'");
-		assertThat(logs).element(3).isEqualTo("Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
+		assertThat(logs).containsExactly(
+				"Installing git pre-push hook",
+				"Git pre-push hook not found, creating it",
+				"Local maven wrapper (mvnw) not found, falling back to global command 'mvn'",
+				"Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
 
 		final var content = mavenHookContent("git_pre_hook/pre-push.created-tpl", ExecutorType.GLOBAL);
 		assertFile(".git/hooks/pre-push").hasContent(content);
@@ -334,6 +336,30 @@ public void should_use_gradle_global_when_bat_and_cmd_files_not_exists_for_windo
 		assertThat(hook).contains("SPOTLESS_EXECUTOR=gradle");
 	}
 
+	@Test
+	public void should_handle_parallel_installation() {
+		// given
+		setFile(".git/config").toContent("");
+
+		// when
+		parallelRun(() -> {
+			final var gradle = new GitPrePushHookInstallerGradle(logger, rootFolder());
+			gradle.install();
+		});
+
+		// then
+		assertThat(logs).containsExactlyInAnyOrder(
+				"Installing git pre-push hook",
+				"Git pre-push hook not found, creating it",
+				"Parallel Spotless Git pre-push hook installation detected, skipping installation",
+				"Parallel Spotless Git pre-push hook installation detected, skipping installation",
+				"Local gradle wrapper (gradlew) not found, falling back to global command 'gradle'",
+				"Git pre-push hook installed successfully to the file " + newFile(".git/hooks/pre-push").getAbsolutePath());
+
+		final var content = gradleHookContent("git_pre_hook/pre-push.created-tpl", ExecutorType.GLOBAL);
+		assertFile(".git/hooks/pre-push").hasContent(content);
+	}
+
 	private String gradleHookContent(String resourcePath, ExecutorType executorType) {
 		return getTestResource(resourcePath)
 				.replace("${executor}", executorType == ExecutorType.WRAPPER ? "./" + newFile("gradlew").getName() : "gradle")
@@ -348,7 +374,31 @@ private String mavenHookContent(String resourcePath, ExecutorType executorType)
 				.replace("${applyCommand}", "spotless:apply");
 	}
 
+	private void parallelRun(ThrowableRun runnable) {
+		IntStream.range(0, 3)
+				.mapToObj(i -> new Thread(() -> {
+					try {
+						runnable.run();
+					} catch (Exception e) {
+						throw new RuntimeException(e);
+					}
+				}))
+				.peek(Thread::start)
+				.collect(toList())
+				.forEach(t -> {
+					try {
+						t.join();
+					} catch (InterruptedException e) {
+						throw new RuntimeException(e);
+					}
+				});
+	}
+
 	private enum ExecutorType {
 		WRAPPER, GLOBAL
 	}
+
+	private interface ThrowableRun {
+		void run() throws Exception;
+	}
 }