Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

lazer-dev
Copy link
Contributor

@lazer-dev lazer-dev commented Jul 22, 2025

Configuration Cache Support & Thread Safety for Git Pre-push Hook

This PR introduces two important improvements for the spotlessInstallGitPrePushHook task:

What’s Changed

  • Configuration Cache Compatibility
    • Refactored the Gradle task to avoid calling project during task execution.
    • Uses Property to inject the project root directory in a cache-safe way.
    • Task is now compatible with org.gradle.configuration-cache=true
  • Parallel Execution Safety
    • Added synchronization (synchronized lock) to prevent race conditions during parallel task execution.
    • Ensures that the hook is only installed once when tasks run in parallel (e.g., via --parallel or multi-project builds).

Why

  • Without these changes, running the task with configuration cache enabled caused build failures.
  • Running the task in parallel previously led to flaky behavior such as:
    • "Failed to create pre-push hook file"
    • Partial or corrupted hook installation.
  • These fixes make the feature stable and safe in all Gradle environments, including CI pipelines.

return;
}

synchronized (LOCK) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used the simplest strategy: double-checked locking with synchronized. Since this task is meant to be run manually by the user, it’s totally OK. It’s a simple and robust solution that doesn’t affect performance, so in my opinion this is a good approach.
If you disagree or have a better idea, I’m happy to hear it!

One more corner case related to parallel execution: the current solution prevents parallel file modification within a single JVM process.
However, if a user runs Gradle in multiple consoles at the same time, we could end up with parallel creation/editing of the same file with a few Gradle processes...
That could be handled using a file lock on the OS level with Java's FileChannel, but I’m not sure we need to worry about this case — it’s very unlikely to happen in practice.
What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not worth the complexity, managing within a single process is good enough

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nedtwigg
If there are no other concerns or comments, can we go ahead and merge this?

@lazer-dev lazer-dev force-pushed the git-hook-parallel-fix branch 5 times, most recently from 26423f1 to 74d1d85 Compare July 22, 2025 16:33
@lazer-dev lazer-dev force-pushed the git-hook-parallel-fix branch 2 times, most recently from 42a7d4e to 592c088 Compare July 24, 2025 02:56
@lazer-dev lazer-dev force-pushed the git-hook-parallel-fix branch from 592c088 to 58c2683 Compare July 24, 2025 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants