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.

Sign up for GitHub

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

Jump to bottom

Automatic worktree detection #243

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Automatic worktree detection #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85dc579
Add worktree scripted test
steinybot Jun 12, 2024
80f9f3d
Automatically detect worktree support
steinybot Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/main/scala/com/github/sbt/git/GitPlugin.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ object SbtGit {
// Build settings.
import GitKeys._
def buildSettings = Seq(
useConsoleForROGit := false,
gitReader := new DefaultReadableGit(baseDirectory.value, if (useConsoleForROGit.value) Some(new ConsoleGitReadableOnly(ConsoleGitRunner, file("."), sLog.value)) else None),
useConsoleForROGit := isGitLinkedRepo(baseDirectory.value),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we could check if jgit v7 is used and if so make useConsoleForROGit false, no matter if we are in a linked repo or not (because jgit 7 supports worktrees, read). Like:

    useConsoleForROGit := jGitVersionLtOrEq6 && isGitLinkedRepo(baseDirectory.value),

I looked into jgit but they do not publish the version in a variable nor method... So I don't see a way of doing this easily.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Maven Central we can find the latest jgit v6 and first v7 version published. The diff is quite big, most of the things we don't need to look at.
So I provide my own html rendered diff here from this command:

git diff v6.10.0.202406032230-r..v7.0.0.202409031743-r org.eclipse.jgit

You can see in v7 the class org.eclipse.jgit.lib.Constants added the GITDIR_FILE constant which is directly related to git worktree.

Would it be able to make use of reflection and check if the Contants class contains the field GITDIR_FILE and if so, we can treat the jgit version on the classpath as one that supports git worktree?
Like:

    useConsoleForROGit := !jGitWithWorktreeSupportDetected() && isGitLinkedRepo(baseDirectory.value),

and in the jGitWithWorktreeSupportDetected method put the reflection code?

What do you think?

Copy link
Member

@eed3si9n eed3si9n Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stepping back a bit, instead of making the automatic detection more complicated, would it make sense to make this more of a machine-wide setting that someone could always opt out of JGit (or opt into JGit)? Also in general, I don't really get the motivation behind JGit. Is the assumption that someone could be working on an opensource project but somehow do not have git installed on their PATH?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also sbt-pgp. In sbt/sbt-pgp#146, I defaulted to using command line gpg and I think the plugin became significantly more reliable because it relied less on the never-ending emulation of gpg via Bouncy Castle.

gitReader := new DefaultReadableGit(baseDirectory.value, if (useConsoleForROGit.value) Some(new ConsoleGitReadableOnly(ConsoleGitRunner, baseDirectory.value, sLog.value)) else None),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this intentionally "." or was that an oversight? Without this it doesn't work when the meta build is the one with the worktree.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely not an oversight, but apparently this was made in an attempt to work with worktree as well:
1fa3ca5

Copy link
Contributor Author

@steinybot steinybot Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. works when the main build has the worktree which is likely what the first implementation was solving for. During my testing I encountered the case where the meta build had the worktree and so . does not work. So I think baseDirectory.value is the correct approach as it should work for both cases. It is probably not a real problem anyway since I doubt anyone would ever set it up like this. Do you agree?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably not a real problem anyway since I doubt anyone would ever set it up like this. Do you agree?

I don't even know what a worktree is, so I'm like 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah fair enough. They are pretty neat. Essentially they allow you to checkout a branch to a different directory without having to have a completely separate clone. So it is almost always going to be at the local root project base directory (or higher), just like a normal .git directory would be.

gitRunner := ConsoleGitRunner,
gitHeadCommit := gitReader.value.withGit(_.headCommitSha),
gitHeadMessage := gitReader.value.withGit(_.headCommitMessage),
Expand All @@ -126,6 +126,21 @@ object SbtGit {
ThisBuild / gitUncommittedChanges := gitReader.value.withGit(_.hasUncommittedChanges),
scmInfo := parseScmInfo(gitReader.value.withGit(_.remoteOrigin))
)

private def isGitLinkedRepo(dir: File): Boolean =
isGitWorktreeDir(Option(System.getenv("GIT_DIR")).fold(dir)(file))

@scala.annotation.tailrec
private def isGitWorktreeDir(dir: File): Boolean = {
val maybeGit = dir / ".git"
// In a linked worktree, .git is a file that contains the path to the main worktree.
if (maybeGit.exists()) maybeGit.isFile
else Option(dir.getParentFile) match {
case Some(parent) => isGitWorktreeDir(parent)
case None => false
}
}

private[sbt] def parseScmInfo(remoteOrigin: String): Option[ScmInfo] = {
val user = """(?:[^@\/]+@)?"""
val domain = """([^\/]+)"""
Expand Down
1 change: 1 addition & 0 deletions src/sbt-test/git-versioning/worktree/.project/project/plugins.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
addSbtPlugin("com.github.sbt" % "sbt-git" % sys.props("project.version"))
13 changes: 13 additions & 0 deletions src/sbt-test/git-versioning/worktree/init.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/usr/bin/env bash

set -eux

mkdir -p main

cd main
git init
git commit --allow-empty -m "Initial commit"
git worktree add ../project
cd -

cp -r .project/project project
3 changes: 3 additions & 0 deletions src/sbt-test/git-versioning/worktree/test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
$ exec ./init.sh
> reload plugins
Copy link
Contributor Author

@steinybot steinybot Jun 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out how to run scripted sbt in a subdirectory so I use the meta build as a workaround. Is it possible to change the directory?

> show gitHeadMessage