Plugin fails in git worktree even when native git is used

[xkrogen]

Describe the bug (required)

This plugin never worked out of the box with git worktrees (#215) due to jGit limitations, but it was fairly easy to work around this issue by using native git as recommended in the comments on that issue. However recently this was broken by bbf8e1a. With this commit, even when native git is used, I get an error like:

[ERROR] Failed to execute goal io.github.git-commit-id:git-commit-id-maven-plugin:6.0.0:revision (default) on project trino-testing-services: Git command exited with invalid status [128]: directory: `/<git_root_dir>/.git/worktrees`, command: `git describe --always --dirty=-dirty --match=* --abbrev=7 --tags`, stdout: ``, stderr: `fatal: this operation must be run in a work tree` -> [Help 1]

The issue appears to be that it is incorrectly detecting the directory in which to run the native git commands using the new approach in the aforementioned commit.

Tell us about your plugin configuration (required)

This comes from the trino project which itself pulls the dependency on the commit-id plugin from airlift/airbase.
The root pom.xml in Trino has:

                <plugin>
                    <groupId>io.github.git-commit-id</groupId>
                    <artifactId>git-commit-id-maven-plugin</artifactId>
                    <configuration>
                        <runOnlyOnce>true</runOnlyOnce>
                        <injectAllReactorProjects>true</injectAllReactorProjects>
                        <!-- A workaround to make build work in a Git worktree, see https://github.com/git-commit-id/git-commit-id-maven-plugin/issues/215 -->
                        <useNativeGit>true</useNativeGit>
                    </configuration>
                </plugin>

The pom.xml pulled from airbase has:

                <plugin>
                    <groupId>io.github.git-commit-id</groupId>
                    <artifactId>git-commit-id-maven-plugin</artifactId>
                    <version>6.0.0</version>
                    <configuration>
                        <!-- Include only properties used above to speed up build (https://github.com/git-commit-id/git-commit-id-maven-plugin/issues/462) -->
                        <includeOnlyProperties>
                            <includeOnlyProperty>\Qgit.build.time</includeOnlyProperty>
                            <includeOnlyProperty>\Qgit.commit.id</includeOnlyProperty>
                            <includeOnlyProperty>\Qgit.commit.id.describe</includeOnlyProperty>
                        </includeOnlyProperties>
                        <dateFormat>yyyy-MM-dd'T'HH:mm:ssZZ</dateFormat>
                        <gitDescribe>
                            <tags>true</tags>
                        </gitDescribe>
                    </configuration>
                </plugin>
...
            <plugin>
                <groupId>io.github.git-commit-id</groupId>
                <artifactId>git-commit-id-maven-plugin</artifactId>
                <executions>
                    <execution>
                        <id>default</id>
                        <phase>initialize</phase>
                        <goals>
                            <goal>revision</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>

Tell us about the Plugin version used (required)

6.0.0

Tell us about the Maven version used (required)

± ./mvnw --version
Apache Maven 3.9.2 (c9616018c7a021c1c39be70fb2843d6f5f9b8a1c)
Maven home: /Users/ekrogen/.m2/wrapper/dists/apache-maven-3.9.2-bin/2ebamk1oldjuut6lacpbt82514/apache-maven-3.9.2
Java version: 17.0.5, vendor: Microsoft, runtime: /Library/Java/JavaVirtualMachines/jdk17.0.5-msft.jdk/Contents/Home
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "13.4.1", arch: "x86_64", family: "mac"

Steps to Reproduce (required)

1. Create a git worktree
2. Check out the master branch of the trinodb/trino project into this worktree
3. Try to build:

./mvnw package -DskipTests

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

From the message, it looks like the dotGitDirectory is incorrectly being detected as .git/worktrees/master, when it really should be master/.git (where master is the name of the worktree).

./mvnw package -DskipTests -Dmaven.javadoc.skip=true -Dmaven.source.skip=true -Dair.check.skip-all=true -Dskip.npm -Dskip.yarn
....
[INFO] --- git-commit-id:6.0.0:revision (default) @ trino-testing-services ---
[INFO] Current project: 'trino-testing-services', first project to execute based on dependency graph: 'trino-testing-services'
[INFO] dotGitDirectory '/<git_root_dir>/.git/worktrees/master'
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for trino-root 421-SNAPSHOT:
[INFO]
[INFO] trino-root ......................................... SUCCESS [  1.607 s]
[INFO] trino-testing-services ............................. FAILURE [  0.491 s]
[INFO] trino-spi .......................................... SKIPPED
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  4.050 s
[INFO] Finished at: 2023-06-23T09:31:45-07:00
[INFO] ------------------------------------------------------------------------
...
[ERROR] Failed to execute goal io.github.git-commit-id:git-commit-id-maven-plugin:6.0.0:revision (default) on project trino-testing-services: Git command exited with invalid status [128]: directory: `<git_root_dir>/.git/worktrees`, command: `git describe --always --dirty=-dirty --match=* --abbrev=7 --tags`, 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 :trino-testing-services

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

Trino

Your Environment (optional)

No response

Context (optional)

No response

[xkrogen]

cc @TheSnoozer since I believe it was your commit bbf8e1a which changed this behavior.

[TheSnoozer]

Thanks for the detailed report! Will take a look. In the linked trino ticket I see that you have found a temporary workaround. Certainly this is not ideal...but would need to understand how to fix it properly.

[TheSnoozer]

Mhh I'm looking at the change-set you linked:

Before the basedir where the git is operating was determined by

final File basedir = cb.getProjectBaseDir();

@Override
public File getProjectBaseDir() throws IOException {
  return project.getBasedir().getCanonicalFile();
}

After the change it is now simplified to:

final File basedir = cb.getDotGitDirectory().getParentFile();

with getDotGitDirectory defaulting to:

@Parameter(defaultValue = "${project.basedir}/.git")
File dotGitDirectory;

so the old implementation was asking
project.getBasedir().getCanonicalFile()
VS the new
project.getBasedir().getParentFile()
which should essentially just be project.getBasedir().

[xkrogen]

Thanks for taking a look @TheSnoozer !

Is there different/fallback logic to determine the dotGitDirectory if the default/configured one doesn't exist? I noticed while testing that if I configure dotGitDirectory to a "correct" .git location (i.e. it exists), then everything works. But if I configured it to point to a .git location that doesn't exist, regardless of which nonexistent location it was, the message was always the same. For example if I configure <dotGitDirectory>/foo/blah</dotGitDirectory> I still get:

[INFO] dotGitDirectory '/Users/ekrogen/dev/trino/.git/worktrees/master'
...
[ERROR] Failed to execute goal io.github.git-commit-id:git-commit-id-maven-plugin:6.0.0:revision (default) on project trino-testing-services: Git command exited with invalid status [128]: directory: `<git_root_dir>/.git/worktrees`, command: `git describe --always --dirty=-dirty --match=* --abbrev=7 --tags`, stdout: ``, stderr: `fatal: this operation must be run in a work tree` -> [Help 1]

So it seems like there is some other logic at play here?

As for why the change broke it, your argument for why it should have been the same is compelling :) FWIW there are no symlinks involved here. As you can see from the linked Trino issue, to get it to work I had to use ${project.basedir}/../../../.git, not just the default of ${project.basedir}/.git -- since this is a multi-module build with nesting. (Though I now realize I can much more cleanly just use ${maven.multiModuleProjectDirectory}.) So it seems that perhaps there was some difference in the face of multi-module builds?

It seems the simplest fix would actually be to change the default for dotGitDirectory to be ${maven.multiModuleProjectDirectory}/.git instead of ${project.basedir}/.git, better reflecting most users' intent, but from MNG-6589 it seems like this is supposed to be a private implementation detail. From MNG-7038 it appears there is actually public support for this coming, but only in Maven 4.0.0.

[jkylling]

The logic which sets dotGitDirectory to /Users/ekrogen/dev/trino/.git/worktrees/master seems to be at

git-commit-id-maven-plugin/src/main/java/pl/project13/maven/git/GitDirLocator.java

Line 63 in e7c6238

File gitDirLinkPath = processGitDirFile(manuallyConfiguredDir);

Unfortunately, processGitDirFile is only aware of submodules

git-commit-id-maven-plugin/src/main/java/pl/project13/maven/git/GitDirLocator.java

Lines 108 to 112 in e7c6238

	/**
	* Load a ".git" git submodule file and read the gitdir path from it.
	*
	* @return File object with path loaded or null
	*/

A naive fix would be to change <projectDir>/.git/worktrees/<tree> to <projectDir>/.git whenever we match the former pattern.

For anyone else stumbling on this issue, I found it helpful to run mvnDebug instead of mvn, attach a debugger, and set some method breakpoints. This made it quick to explore the stack and find the offending code.