Fix parallel install mutex bug and support incremental build#159
Conversation
PR Summary
|
Move all git operations (rev-parse, IsSubmodule, GetGitDirectory) inside the mutex in InstallCommand so reads don't conflict with writes to .git/config. On first run with parallel MSBuild nodes, concurrent 'git rev-parse' calls would fail with 'Permission denied' because another process held .git/config locked for 'git config core.hooksPath'. The mutex now wraps validation + resource creation as a single critical section via the extracted DoInstallAsync method. Add unit test that detects git call interleaving using separate mocks per install instance and deterministic SemaphoreSlim coordination.
Add MSBuild incremental build support to the Husky target so it skips re-running when nothing has changed. Uses a stamp file pattern: - Inputs: .config/dotnet-tools.json (triggers on tool version changes) - Outputs: .husky/_/install.stamp (marker created after success) - Touch task creates the stamp after dotnet tool restore + install - FileWrites ensures dotnet clean removes the stamp Changes: - AttachCommand.cs: Add Inputs/Outputs/Touch/FileWrites to GetTarget() - Husky.csproj: Same incremental pattern on the embedded target - AttachCommandTests.cs: Verify new target structure and path handling - automate.md: Updated snippets, added incrementality and Directory.Build.targets documentation Fixes: alirezanet#155
fa12f12 to
13d7d81
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses monorepo/multi-project build issues by (1) preventing parallel Husky installs from interleaving git operations and (2) making the Husky MSBuild integration incremental via an input/output stamp file.
Changes:
- Wrap git validation + install work under the install mutex to avoid
.git/configlock contention in parallel builds. - Add MSBuild
Inputs/Outputswith a sentinel.husky/_/install.stampto avoid running Husky install on every build. - Update attach generation, tests, and docs to reflect the incremental target pattern.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/Husky/Cli/InstallCommand.cs |
Refactors install flow so git operations are performed under the mutex when parallelism is enabled. |
src/Husky/Husky.csproj |
Makes the embedded Husky target incremental using Inputs/Outputs + a stamp file. |
src/Husky/Cli/AttachCommand.cs |
Updates generated MSBuild target to include Inputs/Outputs, Touch, and FileWrites for the stamp. |
docs/guide/automate.md |
Updates the manual snippet and guidance to the new incremental build pattern. |
tests/HuskyTest/Cli/InstallCommandTests.cs |
Adds a concurrency regression test for git call interleaving. |
tests/HuskyTest/Cli/AttachCommandTests.cs |
Extends assertions to validate incremental target attributes/elements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @MattKotsenas, thanks for the PR |
|
Hi @alirezanet! Thanks for the quick response! In this particular case, Copilot is incorrect. It's assuming that Here's a blog post to read more if you're interested: https://learn.microsoft.com/en-us/archive/msdn-magazine/2009/february/msbuild-best-practices-for-creating-reliable-builds-part-1 Let me know if you have any questions. Thanks! |
|
Thanks @alirezanet, I apologize because I know it can be annoying, but do you know when the next release would be with this fix? Depending on the timing I'll either wait for this release to fix our build, or try to find a short term workaround. Any guidance is greatly appreciated. Thanks! |
I'm working on a couple of other small issues, if I manage to fix them, I'll release the next version in 1-2 days max, maybe even tonight! 🤞 honestly releasing this library is a bit risky because it might block a lot of people if I miss any small potential issues. if you could help me later to test it properly I would do it faster 😊. (I mean after release) |
|
Sounds good! Feel free to ping me when you have a release or a pre-release and I'm happy to test it |
Just release the |
Updated [husky](https://github.com/alirezanet/husky.net) from 0.8.0 to 0.9.0. <details> <summary>Release notes</summary> _Sourced from [husky's releases](https://github.com/alirezanet/husky.net/releases)._ ## 0.9.0 ## What's Changed * remove net6.0/net7.0 since out of support by @WeihanLi in alirezanet/Husky.Net#135 * fix: update regex pattern to allow breaking commits by @joaoopereira in alirezanet/Husky.Net#153 * Fix parallel install mutex bug and support incremental build by @MattKotsenas in alirezanet/Husky.Net#159 * fix: handle file paths with spaces in git commands in alirezanet/Husky.Net#156 * feat: add `staged` property to custom variables for re-staging support in alirezanet/Husky.Net#163 * feat: support variables in include/exclude glob patterns in alirezanet/Husky.Net#161 * fix: use AfterTargets="Restore" to support NuGet credential helpers for private feeds in alirezanet/Husky.Net#162 ## New Contributors * @joaoopereira made their first contribution in alirezanet/Husky.Net#153 * @MattKotsenas made their first contribution in alirezanet/Husky.Net#159 **Full Changelog**: alirezanet/Husky.Net@v0.8.0...v0.9.0 Commits viewable in [compare view](alirezanet/Husky.Net@v0.8.0...v0.9.0). </details> Updated [Microsoft.Extensions.Logging.Abstractions](https://github.com/dotnet/dotnet) from 10.0.4 to 10.0.5. <details> <summary>Release notes</summary> _Sourced from [Microsoft.Extensions.Logging.Abstractions's releases](https://github.com/dotnet/dotnet/releases)._ No release notes found for this version range. Commits viewable in [compare view](https://github.com/dotnet/dotnet/commits). </details> Updated [Microsoft.SourceLink.GitHub](https://github.com/dotnet/dotnet) from 10.0.200 to 10.0.201. <details> <summary>Release notes</summary> _Sourced from [Microsoft.SourceLink.GitHub's releases](https://github.com/dotnet/dotnet/releases)._ ## 10.0.201 You can build .NET 10.0 from the repository by cloning the release tag `v10.0.201` and following the build instructions in the [main README.md](https://github.com/dotnet/dotnet/blob/v10.0.201/README.md#building). Alternatively, you can build from the sources attached to this release directly. More information on this process can be found in the [dotnet/dotnet repository](https://github.com/dotnet/dotnet/blob/v10.0.201/README.md#building-from-released-sources). Attached are PGP signatures for the GitHub generated tarball and zipball. You can find the public key at https://dot.net/release-key-2023 Commits viewable in [compare view](dotnet/dotnet@v10.0.200...v10.0.201). </details> Updated [System.CommandLine](https://github.com/dotnet/dotnet) from 2.0.4 to 2.0.5. <details> <summary>Release notes</summary> _Sourced from [System.CommandLine's releases](https://github.com/dotnet/dotnet/releases)._ No release notes found for this version range. Commits viewable in [compare view](https://github.com/dotnet/dotnet/commits). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Description
Fixes #155
This PR fixes two issues that occur when the target is present in more than one project (common in monorepo scenarios).
1. Fix concurrent
git configlock contentionWhen multiple MSBuild nodes run
dotnet husky installin parallel, concurrentgit rev-parsecalls would intermittently fail with:The existing mutex serialized
CreateResources(which writes.git/configviagit config core.hooksPath), but the validation calls (git rev-parse,IsSubmodule,GetGitDirectory) ran outside the mutex. When one process held.git/configlocked for writing, concurrent processes trying to read it gotPermission denied.The fix is to move all git operations inside the mutex by extracting a
DoInstallAsyncmethod that contains both validation and resource creation. The mutex now wraps the entire install as a single critical section.Reproduced at ~10% failure rate with a 100-project monorepo test harness.
2. Make the Husky MSBuild target incremental
The
Huskytarget randotnet tool restore+dotnet husky installon every build, even when nothing changed. This added seconds to every restore/build cycle, particularly painful when the target is inDirectory.Build.targets(runs once per project in the solution).The fix is to add MSBuild
Inputs/Outputsto the target..config/dotnet-tools.jsonis an input so that the target re-runs when tool version changes..husky/_/install.stampis a sentinel file created after successful install.Touchtask creates the stamp;FileWritesensuresdotnet cleanremoves it.Both the
dotnet husky attachcommand and the embedded target inHusky.csprojnow generate the incremental pattern. Documentation updated with the new snippet and a note about using$(MSBuildThisFileDirectory)inDirectory.Build.targets.3. Tradeoff: self-healing vs. performance
Previously, husky re-installed on every build, which meant a corrupted or manually altered install (e.g., someone running
git config --unset core.hooksPath) would silently self-heal on the next build. With the stamp file, this self-healing is lost as the stamp says "done" and husky won't re-run until the inputs change or the stamp is removed.I believe this is the right tradeoff:
git configor file write failure, or manual config editing)dotnet clean && dotnet restore, which is a well-understood MSBuild patternType of change
Checklist