rudikershaw / git-build-hook

A maven plugin for managing client side (local) git configuration for those working on your project. Including but not limited to setting git config, installing hooks, validating the local repository.
MIT License
138 stars 20 forks source link

Thread safety when setting hooksPath #43

Closed vatbub closed 1 year ago

vatbub commented 1 year ago

Hi there, I am using your plugin to set the directory for git hooks (c.f. my configuration below). Similar to #31 , I am trying to run a multi-threaded multi-module build with the -T 1C flag in maven. Even though the configure goal is marked as thread safe, it still fails sometimes with the following error:

Failed to execute goal com.rudikershaw.gitbuildhook:git-build-hook-maven-plugin:3.2.0:configure (install-git-hooks) on project image-converter.test-utils: Could not find or initialise a local git repository. A repository is required.: C:\Users\kammel\git\image-converter\.git\config (Der Prozess kann nicht auf die Datei zugreifen, da sie von einem anderen Prozess verwendet wird) -> [Help 1]

Essentially, this means that git-build-hook-maven-plugin is running in two different modules at the same time, trying to modify .git\config. Since one thread locks the file, the second thread fails to get write access, failing the build. The plugin would therefore need to synchronize write access to the config file to avoid that.

Here's my configuration:

pom.xml:

<plugin>
    <groupId>com.rudikershaw.gitbuildhook</groupId>
    <artifactId>git-build-hook-maven-plugin</artifactId>
    <version>3.2.0</version>
    <configuration>
        <gitConfig>
            <core.hooksPath>git-hooks/</core.hooksPath>
        </gitConfig>
    </configuration>
    <executions>
        <execution>
            <id>install-git-hooks</id>
            <goals>
                <goal>configure</goal>
            </goals>
            <phase>generate-sources</phase>
        </execution>
    </executions>
</plugin>

System configuration: Maven home: C:\apache-maven-3.8.6-bin\apache-maven-3.8.6 Java version: 17.0.4.1, vendor: Eclipse Adoptium, runtime: C:\Program Files\Eclipse Adoptium\jdk-17.0.4.101-hotspot Default locale: de_DE, platform encoding: Cp1252 OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"

I'm happy to help if you need any further information. Cheers and thanks for developing the plugin ;)

rudikershaw commented 1 year ago

Hi @vatbub,

Apologies for the delayed response. The error message says "Could not find or initialise a local git repository". All goals for this plugin require an initialised git repository. You can use the following goal

<goal>initialize</goal>

to have have the plugin initialise a repository if one does not already exist. If you include that where you define your plugin executions this problem should go away. There is one possible issue though, you are overriding the default phases for the plugin goals in your execution. This will make them run during the same phase, and so it cannot guarantee the goals will run in any given order.

If you really need to override the default phase for the configure goal, you may want to consider defining multiple executions instead. But, given that the configure goal already runs in the generate-sources phase, it may be better to simply remove your <phase> definition. I.e.

<execution>
    <id>install-git-hooks</id>
    <goals>
        <goal>configure</goal>
        <goal>initialize</goal>
    </goals>
</execution>

If this resolves your issue, please feel free to close it. Otherwise let me know if you have any follow up questions or clarifications.

vatbub commented 1 year ago

No worries, but this is not the cause of the error. A git repository exists and there is no need to reinitialize it. In fact, if you translate the rest of the error message, it says "C:\Users\kammel\git\image-converter.git\config (The process cannot access the file, because it is used by another process.)". So, .git\config exists, but it is locked by the plugin, which is trying to write to the config file in the execution of another module.

rudikershaw commented 1 year ago

Thanks for the clarification. I will look into this. I was under the impression that git itself was inherently thread safe. We're using jgit under the hood, which I believe is a re-implementation in Java, it looks like there are thread safety issues there.

If it comes to it we can implement a lock ourselves.

rudikershaw commented 1 year ago

I've been unable to reproduce the issue you described, but I have added some speculative thread safety fixes into a 3.4.1 release.

Would you be able to upgrade to 3.4.1 and let me know if you are still experiencing the error you described above? I will leave this issue open for the time being.

vatbub commented 1 year ago

Hi rudi, sorry for the late reply, I am testing the fix right now. So far, it seems to be working. I'll report back if I find any issues. Thanks for working on it ;)