Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Automatic worktree detection#243

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
steinybot wants to merge2 commits intosbt:main
base:main
Choose a base branch
Loading
fromsteinybot:worktree-detection

Conversation

steinybot
Copy link
Contributor

@steinybotsteinybot commentedJun 12, 2024
edited
Loading

It's a pain submitting PRs to all the repositories that use sbt-git when I want to use a git worktree. This should eliminate the need for that.

Related issues:

sideeffffect, mkurz, and raboof reacted with thumbs up emoji
@@ -0,0 +1,3 @@
$ exec ./init.sh
> reload plugins
Copy link
ContributorAuthor

@steinybotsteinybotJun 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?

useConsoleForROGit :=false,
gitReader := new DefaultReadableGit(baseDirectory.value, if (useConsoleForROGit.value) Some(new ConsoleGitReadableOnly(ConsoleGitRunner,file("."), sLog.value)) else None),
useConsoleForROGit :=isGitLinkedRepo(baseDirectory.value),
gitReader := new DefaultReadableGit(baseDirectory.value, if (useConsoleForROGit.value) Some(new ConsoleGitReadableOnly(ConsoleGitRunner,baseDirectory.value, sLog.value)) else None),
Copy link
ContributorAuthor

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
ContributorAuthor

@steinybotsteinybotSep 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 thinkbaseDirectory.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
ContributorAuthor

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.

@steinybot
Copy link
ContributorAuthor

@SethTisue appologies for mentioning you directly but I noticed in#248 you were planning on doing a release soon. Any chance of getting this in too? It's a bit of a pain having to workaround it in each project that uses sbt-git.

sideeffffect reacted with thumbs up emoji

@SethTisue
Copy link
Member

@eed3si9n@adpi2 would either of you like to take a look at this? I don't have any insight into Jason's questions above about the code tails.

if we don't hear back, I'd be happy to optimistically merge this anyway

@SethTisue
Copy link
Member

re: publishing, I've opened#253

steinybot and mikegpl reacted with thumbs up emoji

@mkurz
Copy link
Member

If you haven't heard yet:

sideeffffect and steinybot reacted with thumbs up emoji

@SethTisue
Copy link
Member

@steinybot mind rebasing? also, does@mkurz's info affect this, or is just evidence for urgency?

@mkurz
Copy link
Member

fyi, the current Playmain branch drops support for Java 11, so we just upgrade jgit ourselves to avoid all those workarounds in future (I think it will take years until sbt-git will drop Java 11...):

Tested locally and can confirm it works.

mzuehlke reacted with rocket emoji

@@ -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 makeuseConsoleForROGit 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 Centralwe 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 providemy 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 classorg.eclipse.jgit.lib.Constants added theGITDIR_FILE constant which is directly related to git worktree.

Would it be able to make use of reflection and check if theContants class contains the fieldGITDIR_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 thejGitWithWorktreeSupportDetected method put the reflection code?

What do you think?

Copy link
Member

@eed3si9need3si9nOct 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 (oropt 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 havegit 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. Insbt/sbt-pgp#146, I defaulted to using command linegpg and I think the plugin became significantly more reliable because it relied less on the never-ending emulation of gpg via Bouncy Castle.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@eed3si9need3si9need3si9n left review comments

@mkurzmkurzmkurz left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@steinybot@SethTisue@mkurz@eed3si9n

[8]ページ先頭

©2009-2025 Movatter.jp