Plugin fails with git submodule involved

[Arsnael]

Describe the bug (required)

The community in Apache James project (https://github.com/apache/james-project) upgrade recently this plugin from version 4.9.10 to 7.0.0 : apache/james-project@88e1ae8

For the TMail backend project using Apache james as a submodule to implement extra features on top, it creates the following issue when trying to upgrade our project to latest version of the submodule:

[ERROR] Failed to execute goal io.github.git-commit-id:git-commit-id-maven-plugin:7.0.0:revision (get-the-git-infos) on project testing-base: Git command exited with invalid status [128]: directory: `/home/jenkins/build/workspace/Tmail_build_PR-932/.git/modules`, command: `git describe --always --dirty=-dirty --match=* --abbrev=7`, stdout: ``, stderr: `fatal: this operation must be run in a work tree` -> [Help 1]

PR: linagora/tmail-backend#932

It seems similar to this issue: #639 that has been solved and merged into 7.0.0 it seems (cf #642).

Except that I actually never really used worktrees in the first place? Is there some conflict perhaps?

Tell us about your plugin configuration (required)

It comes from the Apache James project that is used as a submodule into TMail project

<plugin>
    <groupId>io.github.git-commit-id</groupId>
    <artifactId>git-commit-id-maven-plugin</artifactId>
    <version>7.0.0</version>
</plugin>

...

<plugin>
    <groupId>io.github.git-commit-id</groupId>
    <artifactId>git-commit-id-maven-plugin</artifactId>
    <configuration>
        <dotGitDirectory>${project.basedir}/.git</dotGitDirectory>
        <injectAllReactorProjects>true</injectAllReactorProjects>
        <generateGitPropertiesFile>true</generateGitPropertiesFile>
        <generateGitPropertiesFilename>${project.build.outputDirectory}/git.properties</generateGitPropertiesFilename>
        <failOnNoGitDirectory>false</failOnNoGitDirectory>
        <format>json</format>
        <gitDescribe>
            <skip>false</skip>
            <always>true</always>
            <dirty>-dirty</dirty>
        </gitDescribe>
        <includeOnlyProperties>
            <includeOnlyProperty>git.commit.id</includeOnlyProperty>
            <includeOnlyProperty>git.commit.id.describe</includeOnlyProperty>
            <includeOnlyProperty>git.commit.message.short</includeOnlyProperty>
            <includeOnlyProperty>git.dirty</includeOnlyProperty>
        </includeOnlyProperties>
        <prefix>git</prefix>
        <skip>false</skip>
        <useNativeGit>true</useNativeGit>
        <verbose>false</verbose>
    </configuration>
    <executions>
        <execution>
            <id>get-the-git-infos</id>
            <goals>
                <goal>revision</goal>
            </goals>
        </execution>
    </executions>
</plugin>

Tell us about the Plugin version used (required)

7.0.0

Tell us about the Maven version used (required)

mvn --version
Apache Maven 3.9.4 (dfbb324ad4a7c8fb0bf182e6d91b0ae20e3d2dd9)
Maven home: /home/rene/.sdkman/candidates/maven/current
Java version: 17.0.8.1, vendor: Eclipse Adoptium, runtime: /home/rene/.sdkman/candidates/java/17.0.8.1-tem
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "6.6.16-275.current", arch: "amd64", family: "unix"

Steps to Reproduce (required)

1. Clone tmail backend forked project: https://github.com/Arsnael/tmail-backend
2. Checkout branch james-sub-upgrade-2
3. Initialize submodule:

git submodule init
git submodule update

4. Try to build:

mvn clean install -DskipTests

Are there any stacktraces or any error messages? (required)

mvn clean install -Dmaven.javadoc.skip=true -DskipTests -T1C
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  6.193 s (Wall Clock)
[INFO] Finished at: 2024-02-22T17:14:27+07:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal io.github.git-commit-id:git-commit-id-maven-plugin:7.0.0:revision (get-the-git-infos) on project testing-base: Git command exited with invalid status [128]: directory: `/home/rene/Dev/github/tmail-backend/.git/modules`, command: `git describe --always --dirty=-dirty --match=* --abbrev=7`, stdout: ``, stderr: `fatal: this operation must be run in a work tree` -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <args> -rf :testing-base

Is there a (public) project where this issue can be reproduced? (optional)

https://github.com/Arsnael/tmail-backend

Your Environment (optional)

git version 2.43.1

Context (optional)

No response

[TheSnoozer]

Thanks for the ticket and the reproducer. Will try to troubleshoot on the weekend. Unclear how difficult it would be to fix it, so i can't provide any ETA at this time.

[Arsnael]

@TheSnoozer Thank you for your quick answer and for the time you will take to look at it. I understand it seems like a tricky one, just don't hesitate to ping me back if you need more details regarding our project and issue !

[TheSnoozer]

Again thanks for the detailed explanation and the ability to reproduce!
And sorry for coming back to this so late...

I did a "quick" troubleshooting session and it indeed seems that the lookupGitDirectory function in the GitCommitIdMojo is somewhat to blame here.

running your project with mvn .... -X reveals the following configuration is applied to the plugin:

 <dotGitDirectory default-value="${project.basedir}/.git">/tmp/tmail-backend/james-project/testing/base/.git</dotGitDirectory>

When surrounding this lookupGitDirectory with print statements it would tell us:

####### before lookupGitDirectory=/tmp/tmail-backend/james-project/testing/base/.git
####### after lookupGitDirectory=/tmp/tmail-backend/james-project/../.git/modules/james-project

The issue you linked in fact only added a fix for worktrees, the resolveWorktree only has an effect on worktrees:

####### before resolveWorktree=../.git/modules/james-project
####### after resolveWorktree=../.git/modules/james-project

I guess all that means we would need a resolveModules that would convert a ../.git/modules/james-project into .git.

[Arsnael]

Very detailed explanation, thanks for taking a look at this :)

[TheSnoozer]

I took another look and created various tests to verify my hypothosis (mainly wanting to ensure that the bug is actually fixed).

After the second round of "quick" troubleshooting where I now also run the commands manually I think the gitDirectory of /tmp/tmail-backend/james-project/../.git/modules/james-project is actually the correct one!

/tmp/tmail-backend/.git/modules/james-project$ git rev-parse HEAD
530f2626a0c1f3b27c759f84ffadec94f3a809a4
/tmp/tmail-backend/.git/modules/james-project$ git describe --always --dirty=-dirty --match=* --abbrev=7
james-project-3.8.0-613-g530f262-dirty

/tmp/tmail-backend$ git submodule foreach git rev-parse HEAD
Entering 'james-project'
530f2626a0c1f3b27c759f84ffadec94f3a809a4
/tmp/tmail-backend$ git rev-parse HEAD
65bc3287a3ef1251c3d037615e17f707f25e2485

I think what mainly threw me off is that the tests i have performed indicated that this issue only happens when running with <useNativeGit>true</useNativeGit>.

Setting a <useNativeGit>false</useNativeGit> makes everything work...so where is the issue hiding?

it took me way to long to realize that this may be a very very very very old oversight in running the NativeGit:
https://github.com/git-commit-id/git-commit-id-plugin-core/blob/0dcee417f2d4c530663c7a8bbeb0be2adf2fa519/src/main/java/pl/project13/core/GitCommitIdPlugin.java#L353

Here we run unconditionally cb.getDotGitDirectory().getParentFile().
With the gotGitDirectory set to /tmp/tmail-backend/james-project/../.git/modules/james-project I now see why and how the git executable is executed in the modules folder.

I guess when running native git commands assuming that we need to go to the parent-dir makes a lot of sense:

/tmp/tmail-backend/.git$ git status
fatal: this operation must be run in a work tree

However I guess what this now means is that we need to getParentFile() conditionally.

[Arsnael]

Once again very detailed explanation !

Thanks for taking the time to fix this, very appreciated :)

Let us know when you do a new release including this fix @TheSnoozer !

Cheers

[TheSnoozer]

Thanks again, released now: https://repo1.maven.org/maven2/io/github/git-commit-id/git-commit-id-maven-plugin/8.0.0/

Please check the release notes why this is now a breaking change (https://github.com/git-commit-id/git-commit-id-maven-plugin/releases/tag/v8.0.0)

[Arsnael]

Alright I will test that out, thanks @TheSnoozer :)