alirezanet / Husky.Net

Git hooks made easy with Husky.Net internal task runner! ๐Ÿถ It brings the dev-dependency concept to the .NET world!
https://alirezanet.github.io/Husky.Net/
MIT License
632 stars 29 forks source link

Using 'husky attach' inside multi-project solutions leads to compile errors due to 'husky install' running multiple times #54

Closed abelbraaksma closed 1 year ago

abelbraaksma commented 1 year ago

Version

v0.4.4

Details

After you run dotnet husky attach someproject.csproj, and doing so on multiple projects inside a solution to ensure people working on individual projects are forced to use Husky (in my case: formatting in pre-commit hook), leads to the following error:

The process cannot access the file 'D:\Projects\MyProject\.husky\_\.gitignore' because it is being used by another process.
error MSB3073: The command "dotnet husky install" exited with code 1.

It is clear that the error is caused by the parallel build feature of MSBuild, as each project in a solution is being build. I have not yet found a stable, different way of ensuring the dotnet husky install is run only once.

Hand-crafted ways that probably work is do a file-exists test of the .husky folder. But by itself, that is not enough, as Husky won't run unless core.hooksPath of Git is set to .husky, which this command so conveniently does.

Ideally, dotnet husky install should not lock files it reads, or use a mutex of sorts (available cross-platform in dotnet nowadays) to prevent it from running multiple times.

Alternative, without using exclusive locks, dotnet husky install could just check if Husky is already installed and of the correct version. But that still leaves the issue that, if it is not installed yet, that "Rebuild All" will have multiple parallel dotnet husky install running.

Steps to reproduce

You will now get the above mentioned error.

PS: I really like Husky.Net. The above is just a minor inconvenience. After trying many different ways of creating cross-platform auto-installed pre-commit hooks for Git, basically nothing worked out of the box, but Husky did. It is awesome!!!!

abelbraaksma commented 1 year ago

It took me a while, but here's a configuration that works with the current dotnet husky install by basically checking if it is installed or not. It is a bit tricky to get right, though, and I wouldn't expect people to add such complex tasks. Would be better if it could be fixed somehow in dotnet husky install instead.

The following code does:

It turned out to be a bit tricky to ensure the not running when cleaning bit. Maybe there's a better way, but in the end, the below works. I just dumped it in an EnableHusky.proj and use <Import Project="EnableHusky.proj" /> in each project file.

<!-- 
  Prevent running Husky-Install when Target=Clean. 
  Must run before 'BeforeClean' (just before 'Clean' is not correct and will execute Husky anyway.)
-->
<Target Name="NoHuskyCheckOnClean" BeforeTargets="BeforeClean">
  <CreateProperty Value="1">
    <Output TaskParameter="Value" PropertyName="SkipHuskyCheck" />
  </CreateProperty>
</Target>

<!--
  This checks whether Husky is already installed, then installs it, if necessary.
-->
<Target Name="CheckAndInstallHusky" BeforeTargets="Restore;CollectPackageReferences" Condition="'$(SkipHuskyCheck)' != '1'">
  <Exec Command="git config --local --default &quot;&quot; --get core.hookspath" ConsoleToMSBuild="true" StandardOutputImportance="Low">
    <Output TaskParameter="ConsoleOutput" PropertyName="HuskyHookLocation" />
  </Exec>
  <CreateProperty Value="true" Condition="'$(HuskyHookLocation)'=='.husky'">
    <Output TaskParameter="Value" PropertyName="HuskyAlreadyInstalled" />
  </CreateProperty>

  <!-- restore dotnet tools -->
  <Exec Command="dotnet tool restore" StandardOutputImportance="Low" StandardErrorImportance="High" />
  <Message Condition="'$(HuskyAlreadyInstalled)'=='true'" Text="Husky installation skipped: already installed" Importance="high" />

  <!-- call husky installer conditionally -->
  <CallTarget Targets="InstallHusky" Condition="'$(HuskyAlreadyInstalled)'!='true'" />
</Target>

<!-- The actual Husky Install target, dependently called from CheckAndInstallHusky -->
<Target Name="InstallHusky">
  <Message Text="Installing Husky" Importance="high" />
  <Exec Condition="'$(HuskyHookLocation)'!='.husky'" Command="dotnet husky install" StandardOutputImportance="Low" StandardErrorImportance="High" WorkingDirectory="..\.." />
</Target>
abelbraaksma commented 1 year ago

Lol, meanwhile you've been fixing this on your end! If you want me to test something, let me know (though it's past midnight here, may be tomorrow).

alirezanet commented 1 year ago

Hi @abelbraaksma, First of all, thank you for the feedback and your great ideas, I really enjoy these kinds of issues ๐Ÿ™.

On my end, I solved the problem with a global mutex (as you correctly suggested), and you can try it in v0.5.2.

but you also mentioned another problem which is multiple restore and install that could make builds slow. I usually don't attach husky to all of my projects and a common/shared project usually does the job, also according to my tests the performance overhead of these two commands is really low but if you think there is a better way to handle this or you had any other suggestion for improvement, you're really welcome, we can discuss it in another issue. btw your configuration is really interesting (I may use some of your checking ideas in the next versions) ๐Ÿ‘.

Good luck.

abelbraaksma commented 1 year ago

Thanks for the quick fix, it is much appreciated!

On the significance of small things, I did a little experiment:

Here's a timing on my machine of dotnet husky install. Commands were given without intervening pause (ignore the first timestamp and call):

d:\temp 2:09:03.99>dotnet husky install
Git hooks installed
d:\temp 2:09:10.41>dotnet husky install
Git hooks installed
d:\temp 2:09:11.15>dotnet husky install
Git hooks installed
d:\temp 2:09:11.90>dotnet husky install
Git hooks installed
d:\temp 2:09:12.64>dotnet husky install
Git hooks installed
d:\temp 2:09:13.39>

On my machine that is 4 commands is 13.39 - 10.41 = 2.98s, or about 0.75s per command. This may be dotnet overhead, I don't know. But take the Clean example. I have 12 projects in a solution (it depends, we have many solutions, some smaller, some larger, up to 30-something projects). Cleaning takes about 2-3s (without Husky). With each project having this command as well, it adds 9s, which is significant.

I also timed dotnet tool restore, which likely depends on the amount of tools. It took 0.5s per command. Altogether, in this example the two together become ~1.25s. Could be worse, but it adds up (and if this is network time, a faster PC won't solve it enough).

I usually don't attach husky to all of my projects and a common/shared project usually does the job

Yes, I thought of that too. But as soon as it is shared, it'll run on each compile. Oh wait, Visual Studio probably won't recompile the dependency, as long as it is a project with output. But that requires something to be compiled, but maybe there's a simpler way with the EnableHusky.proj approach where you import it everywhere, but it runs only once (like VS does for already compiled projects).

Edit: maybe something like this could be (ab)used for the "run once during solution build" issue: https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-batching?view=vs-2022

Btw, your name reminds me of this brilliant chess player, Alireza Firouzja. I assume you're a different Alireza? โ™Ÿ๏ธ

abelbraaksma commented 1 year ago

To answer my own question, it turns out that Target Batching works. With your mutex (which prevents problems when run in parallel) and the Outputs vs Inputs behavior (where MSbuild diligently tests whether source has been updated), this indeed prevents the execution of the tasks.

Note that the current code still prevents unnecessary running of dotnet husky install if it is already installed. But tbh, the overhead of this is much less now that MSBuild takes care of dirty-flagging the installation (it only runs when the version is updated).

There's now probably some code that's redundant and hopefully a smaller version is possible. I'll soon test with your mutexed version. For posterity, this is the current version (paths may need to be adjusted if people try to use this, i.e. like the path to dotnet-tools.json):

<Project>

  <PropertyGroup>
    <CurrentDate>$([System.DateTime]::Now.ToString(o))</CurrentDate>
  </PropertyGroup>

  <!-- 
    Prevent running Husky-Install when Target=Clean. 
    Must run before 'BeforeClean' (just before 'Clean' is not correct and will execute Husky anyway.)
  -->
  <Target Name="NoHuskyCheckOnClean" BeforeTargets="BeforeClean">
    <CreateProperty Value="1">
      <Output TaskParameter="Value" PropertyName="SkipHuskyCheck" />
    </CreateProperty>
  </Target>

  <!--
    This checks whether Husky is already installed, then installs it, if necessary.
  -->
  <Target Name="CheckAndInstallHusky" 
          BeforeTargets="Restore;CollectPackageReferences" 
          Condition="'$(SkipHuskyCheck)' != '1'" 
          Inputs="$(MSBuildProjectDirectory)\..\..\.config\dotnet-tools.json"
          Outputs="$(MSBuildProjectDirectory)\..\..\.config\husky-installed.lock">

    <Exec Command="git config --local --default &quot;&quot; --get core.hookspath" ConsoleToMSBuild="true" StandardOutputImportance="Low">
      <Output TaskParameter="ConsoleOutput" PropertyName="HuskyHookLocation" />
    </Exec>

    <CreateProperty Value="true" Condition="'$(HuskyHookLocation)'=='.husky'">
      <Output TaskParameter="Value" PropertyName="HuskyAlreadyInstalled" />
    </CreateProperty>

    <!-- restore dotnet tools -->
    <Exec Command="dotnet tool restore" StandardOutputImportance="Low" StandardErrorImportance="High" />
    <Message Condition="'$(HuskyAlreadyInstalled)'=='true'" Text="Husky installation skipped: already installed" Importance="high" />

    <!-- call husky installer conditionally -->
    <CallTarget Targets="InstallHusky" Condition="'$(HuskyAlreadyInstalled)'!='true'" />

    <!-- after successful task exec, ensure on next build it is not run again (MSBuild uses Inputs/Outputs compare for that) -->
    <WriteLinesToFile File="$(MSBuildProjectDirectory)\..\..\.config\husky-installed.lock"
                      WriteOnlyWhenDifferent="true"
                      Overwrite="true"
                      Lines="$(CurrentDate)"/>

  </Target>

  <!-- The actual Husky Install target, dependently called from CheckAndInstallHusky -->
  <Target Name="InstallHusky">
    <Message Text="Installing Husky" Importance="high" />
    <Exec Condition="'$(HuskyHookLocation)'!='.husky'" Command="dotnet husky install" StandardOutputImportance="Low" StandardErrorImportance="High" WorkingDirectory="..\.." />
  </Target>

</Project>
alirezanet commented 1 year ago

Thanks for sharing your configuration, something like this could be an additional advanced feature to the husky attach command, I'll be happy if you test this and share your final thoughts on another issue that you think is necessary for others facing a similar problem.

Btw, your name reminds me of this brilliant chess player, I assume you're a different Alireza? โ™Ÿ๏ธ

Yeah ๐Ÿ˜‰, there are a lot of Alireza in Iran, but firoozja is originally from the city I'm living now. :) we have at least one thing in common ๐Ÿ˜…, plus I also like chess but my rating is half of his.

Have a nice day