Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin fails in git worktree even when native git is used #639

Closed
xkrogen opened this issue Jun 23, 2023 · 5 comments · Fixed by #642
Closed

Plugin fails in git worktree even when native git is used #639

xkrogen opened this issue Jun 23, 2023 · 5 comments · Fixed by #642
Labels
Milestone

Comments

@xkrogen
Copy link

xkrogen commented Jun 23, 2023

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
Copy link
Author

xkrogen commented Jun 23, 2023

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

@TheSnoozer
Copy link
Collaborator

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 TheSnoozer added this to the 7.0.0 milestone Jun 24, 2023
@TheSnoozer
Copy link
Collaborator

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
Copy link
Author

xkrogen commented Jun 26, 2023

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.

vttranlina added a commit to vttranlina/james-project that referenced this issue Aug 2, 2023
….9.10 -> io.github.git-commit-id:git-commit-id-maven-plugin 6.0.0"

This reverts commit ddac45e.
Has a bug when use james-project with git submodule: git-commit-id/git-commit-id-maven-plugin#639
The bug is not closed
Arsnael pushed a commit to apache/james-project that referenced this issue Aug 3, 2023
….9.10 -> io.github.git-commit-id:git-commit-id-maven-plugin 6.0.0"

This reverts commit ddac45e.
Has a bug when use james-project with git submodule: git-commit-id/git-commit-id-maven-plugin#639
The bug is not closed
@jkylling
Copy link
Contributor

jkylling commented Aug 12, 2023

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

File gitDirLinkPath = processGitDirFile(manuallyConfiguredDir);

Unfortunately, processGitDirFile is only aware of submodules
/**
* 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants