# [PATCHv2] git-p4 worktree support 4 messages from 2016-12-10 to 2016-12-13. Participants: Luke Diamand, Duy Nguyen. Thread: https://gitlist.dev/t/44672 ## Luke Diamand, 2016-12-10 21:57 Subject: [PATCHv2] git-p4 worktree support Message-ID: <20161210215734.7468-1-luke@diamand.org> URL: https://gitlist.dev/e/20161210215734.7468-1-luke%40diamand.org ``` Second attempt at teaching git-p4 about worktrees. Earlier discussion here: http://marc.info/?l=git&m=148097985622294 Git-p4 exports GIT_DIR so that when it chdirs into the P4 client area to apply the change, git commands called from there will work correctly. Luke Diamand (1): git-p4: support git worktrees git-p4.py | 47 ++++++++++++++++++++++++++--------------------- t/t9800-git-p4-basic.sh | 20 ++++++++++++++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) -- 2.8.2.703.g78b384c.dirty ``` ## Luke Diamand, 2016-12-10 21:57 Subject: [PATCHv2] git-p4: support git worktrees Message-ID: <20161210215734.7468-2-luke@diamand.org> URL: https://gitlist.dev/e/20161210215734.7468-2-luke%40diamand.org In-Reply-To: <20161210215734.7468-1-luke@diamand.org> ``` git-p4 would attempt to find the git directory using its own specific code, which did not know about git worktrees. This caused git operations to fail needlessly. Rework it to use "git rev-parse --git-dir" instead, which knows about worktrees. Signed-off-by: Luke Diamand --- git-p4.py | 47 ++++++++++++++++++++++++++--------------------- t/t9800-git-p4-basic.sh | 20 ++++++++++++++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/git-p4.py b/git-p4.py index fd5ca52..6aa8957 100755 --- a/git-p4.py +++ b/git-p4.py @@ -49,6 +49,13 @@ defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$' # Grab changes in blocks of this many revisions, unless otherwise requested defaultBlockSize = 512 +def gitdir(): + d = read_pipe("git rev-parse --git-dir").strip() + if not d or len(d) == 0: + return None + else: + return d + def p4_build_cmd(cmd): """Build a suitable p4 command line. @@ -562,12 +569,6 @@ def currentGitBranch(): else: return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip() -def isValidGitDir(path): - if (os.path.exists(path + "/HEAD") - and os.path.exists(path + "/refs") and os.path.exists(path + "/objects")): - return True; - return False - def parseRevision(ref): return read_pipe("git rev-parse %s" % ref).strip() @@ -3462,7 +3463,7 @@ class P4Sync(Command, P4UserMap): if self.tempBranches != []: for branch in self.tempBranches: read_pipe("git update-ref -d %s" % branch) - os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation)) + os.rmdir(os.path.join(gitdir(), self.tempBranchLocation)) # Create a symbolic ref p4/HEAD pointing to p4/ to allow # a convenient shortcut refname "p4". @@ -3678,23 +3679,27 @@ def main(): (cmd, args) = parser.parse_args(sys.argv[2:], cmd); global verbose verbose = cmd.verbose + if cmd.needsGit: - if cmd.gitdir == None: - cmd.gitdir = os.path.abspath(".git") - if not isValidGitDir(cmd.gitdir): - cmd.gitdir = read_pipe("git rev-parse --git-dir").strip() - if os.path.exists(cmd.gitdir): - cdup = read_pipe("git rev-parse --show-cdup").strip() - if len(cdup) > 0: - chdir(cdup); - - if not isValidGitDir(cmd.gitdir): - if isValidGitDir(cmd.gitdir + "/.git"): - cmd.gitdir += "/.git" - else: + if cmd.gitdir: + os.environ["GIT_DIR"] = cmd.gitdir + + # did we get a valid git dir on the command line or via $GIT_DIR? + if not gitdir(): die("fatal: cannot locate git repository at %s" % cmd.gitdir) - os.environ["GIT_DIR"] = cmd.gitdir + else: + # already in a git directory? + if not gitdir(): + die("fatal: not in a valid git repository") + + cdup = read_pipe("git rev-parse --show-cdup").strip() + if len(cdup) > 0: + chdir(cdup); + + # ensure subshells spawned in the p4 repo directory + # get the correct GIT_DIR + os.environ["GIT_DIR"] = os.path.abspath(gitdir()) if not cmd.run(args): parser.print_help() diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh index 0730f18..093e9bd 100755 --- a/t/t9800-git-p4-basic.sh +++ b/t/t9800-git-p4-basic.sh @@ -257,6 +257,26 @@ test_expect_success 'submit from detached head' ' ) ' +test_expect_success 'submit from worktree' ' + test_when_finished cleanup_git && + git p4 clone --dest="$git" //depot && + ( + cd "$git" && + git worktree add ../worktree-test + ) && + ( + cd "$git/../worktree-test" && + test_commit "worktree-commit" && + git config git-p4.skipSubmitEdit true && + git p4 submit + ) && + ( + cd "$cli" && + p4 sync && + test_path_is_file worktree-commit.t + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' -- 2.8.2.703.g78b384c.dirty ``` ## Luke Diamand, 2016-12-11 07:19 Subject: Re: [PATCHv2] git-p4: support git worktrees Message-ID: URL: https://gitlist.dev/e/CAE5ih7_6Ap_dY3mRb3Hk2yzDRMkZ3HnnQOaikF%3Dybx_XNdVWhQ%40mail.gmail.com In-Reply-To: <20161210215734.7468-2-luke@diamand.org> ``` On 10 December 2016 at 21:57, Luke Diamand wrote: > git-p4 would attempt to find the git directory using > its own specific code, which did not know about git > worktrees. This caused git operations to fail needlessly. > > Rework it to use "git rev-parse --git-dir" instead, which > knows about worktrees. Actually this doesn't work as well as the original version. "git rev-parse --git-dir" won't go and find the ".git" subdirectory. The previous version would go looking for it, so this would introduce a regression. Luke > > Signed-off-by: Luke Diamand > --- > git-p4.py | 47 ++++++++++++++++++++++++++--------------------- > t/t9800-git-p4-basic.sh | 20 ++++++++++++++++++++ > 2 files changed, 46 insertions(+), 21 deletions(-) > > diff --git a/git-p4.py b/git-p4.py > index fd5ca52..6aa8957 100755 > --- a/git-p4.py > +++ b/git-p4.py > @@ -49,6 +49,13 @@ defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$' > # Grab changes in blocks of this many revisions, unless otherwise requested > defaultBlockSize = 512 > > +def gitdir(): > + d = read_pipe("git rev-parse --git-dir").strip() > + if not d or len(d) == 0: > + return None > + else: > + return d > + > def p4_build_cmd(cmd): > """Build a suitable p4 command line. > > @@ -562,12 +569,6 @@ def currentGitBranch(): > else: > return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip() > > -def isValidGitDir(path): > - if (os.path.exists(path + "/HEAD") > - and os.path.exists(path + "/refs") and os.path.exists(path + "/objects")): > - return True; > - return False > - > def parseRevision(ref): > return read_pipe("git rev-parse %s" % ref).strip() > > @@ -3462,7 +3463,7 @@ class P4Sync(Command, P4UserMap): > if self.tempBranches != []: > for branch in self.tempBranches: > read_pipe("git update-ref -d %s" % branch) > - os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation)) > + os.rmdir(os.path.join(gitdir(), self.tempBranchLocation)) > > # Create a symbolic ref p4/HEAD pointing to p4/ to allow > # a convenient shortcut refname "p4". > @@ -3678,23 +3679,27 @@ def main(): > (cmd, args) = parser.parse_args(sys.argv[2:], cmd); > global verbose > verbose = cmd.verbose > + > if cmd.needsGit: > - if cmd.gitdir == None: > - cmd.gitdir = os.path.abspath(".git") > - if not isValidGitDir(cmd.gitdir): > - cmd.gitdir = read_pipe("git rev-parse --git-dir").strip() > - if os.path.exists(cmd.gitdir): > - cdup = read_pipe("git rev-parse --show-cdup").strip() > - if len(cdup) > 0: > - chdir(cdup); > - > - if not isValidGitDir(cmd.gitdir): > - if isValidGitDir(cmd.gitdir + "/.git"): > - cmd.gitdir += "/.git" > - else: > + if cmd.gitdir: > + os.environ["GIT_DIR"] = cmd.gitdir > + > + # did we get a valid git dir on the command line or via $GIT_DIR? > + if not gitdir(): > die("fatal: cannot locate git repository at %s" % cmd.gitdir) > > - os.environ["GIT_DIR"] = cmd.gitdir > + else: > + # already in a git directory? > + if not gitdir(): > + die("fatal: not in a valid git repository") > + > + cdup = read_pipe("git rev-parse --show-cdup").strip() > + if len(cdup) > 0: > + chdir(cdup); > + > + # ensure subshells spawned in the p4 repo directory > + # get the correct GIT_DIR > + os.environ["GIT_DIR"] = os.path.abspath(gitdir()) > > if not cmd.run(args): > parser.print_help() > diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh > index 0730f18..093e9bd 100755 > --- a/t/t9800-git-p4-basic.sh > +++ b/t/t9800-git-p4-basic.sh > @@ -257,6 +257,26 @@ test_expect_success 'submit from detached head' ' > ) > ' > > +test_expect_success 'submit from worktree' ' > + test_when_finished cleanup_git && > + git p4 clone --dest="$git" //depot && > + ( > + cd "$git" && > + git worktree add ../worktree-test > + ) && > + ( > + cd "$git/../worktree-test" && > + test_commit "worktree-commit" && > + git config git-p4.skipSubmitEdit true && > + git p4 submit > + ) && > + ( > + cd "$cli" && > + p4 sync && > + test_path_is_file worktree-commit.t > + ) > +' > + > test_expect_success 'kill p4d' ' > kill_p4d > ' > -- > 2.8.2.703.g78b384c.dirty > ``` ## Duy Nguyen, 2016-12-13 11:17 Subject: Re: [PATCHv2] git-p4: support git worktrees Message-ID: URL: https://gitlist.dev/e/CACsJy8A%2BcEJoKyfEnJvqwT4pvUGgcrmJdRmXKyRoAff4EYgcuw%40mail.gmail.com In-Reply-To: ``` On Sun, Dec 11, 2016 at 2:19 PM, Luke Diamand wrote: > On 10 December 2016 at 21:57, Luke Diamand wrote: >> git-p4 would attempt to find the git directory using >> its own specific code, which did not know about git >> worktrees. This caused git operations to fail needlessly. >> >> Rework it to use "git rev-parse --git-dir" instead, which >> knows about worktrees. > > Actually this doesn't work as well as the original version. "git > rev-parse --git-dir" won't go and find the ".git" subdirectory. The > previous version would go looking for it, so this would introduce a > regression. Maybe git rev-parse --git-path HEAD, then strip out the /HEAD part? -- Duy ```