{"thread":{"id":"44672","subject":"[PATCHv2] git-p4 worktree support","startedAt":"2016-12-10T21:57:58Z","lastAt":"2016-12-13T11:18:14Z","messageCount":4,"participants":["Luke Diamand","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"307453","messageId":"20161210215734.7468-1-luke@diamand.org","threadId":"44672","inReplyTo":null,"subject":"[PATCHv2] git-p4 worktree support","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-12-10T21:57:33Z","receivedAt":"2016-12-10T21:57:58Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Second attempt at teaching git-p4 about worktrees.\n\nEarlier discussion here:\n\nhttp://marc.info/?l=git&m=148097985622294\n\nGit-p4 exports GIT_DIR so that when it chdirs into the\nP4 client area to apply the change, git commands called\nfrom there will work correctly.\n\nLuke Diamand (1):\n git-p4: support git worktrees\n\n git-p4.py | 47 ++++++++++++++++++++++++++---------------------\n t/t9800-git-p4-basic.sh | 20 ++++++++++++++++++++\n 2 files changed, 46 insertions(+), 21 deletions(-)\n\n-- \n2.8.2.703.g78b384c.dirty\n\n"},{"id":"307454","messageId":"20161210215734.7468-2-luke@diamand.org","threadId":"44672","inReplyTo":"20161210215734.7468-1-luke@diamand.org","subject":"[PATCHv2] git-p4: support git worktrees","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-12-10T21:57:34Z","receivedAt":"2016-12-10T21:58:01Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"git-p4 would attempt to find the git directory using\nits own specific code, which did not know about git\nworktrees. This caused git operations to fail needlessly.\n\nRework it to use \"git rev-parse --git-dir\" instead, which\nknows about worktrees.\n\nSigned-off-by: Luke Diamand \n---\n git-p4.py | 47 ++++++++++++++++++++++++++---------------------\n t/t9800-git-p4-basic.sh | 20 ++++++++++++++++++++\n 2 files changed, 46 insertions(+), 21 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex fd5ca52..6aa8957 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -49,6 +49,13 @@ defaultLabelRegexp = r'[a-zA-Z0-9_\\-.]+$'\n # Grab changes in blocks of this many revisions, unless otherwise requested\n defaultBlockSize = 512\n \n+def gitdir():\n+ d = read_pipe(\"git rev-parse --git-dir\").strip()\n+ if not d or len(d) == 0:\n+ return None\n+ else:\n+ return d\n+\n def p4_build_cmd(cmd):\n \"\"\"Build a suitable p4 command line.\n \n@@ -562,12 +569,6 @@ def currentGitBranch():\n else:\n return read_pipe([\"git\", \"name-rev\", \"HEAD\"]).split(\" \")[1].strip()\n \n-def isValidGitDir(path):\n- if (os.path.exists(path + \"/HEAD\")\n- and os.path.exists(path + \"/refs\") and os.path.exists(path + \"/objects\")):\n- return True;\n- return False\n-\n def parseRevision(ref):\n return read_pipe(\"git rev-parse %s\" % ref).strip()\n \n@@ -3462,7 +3463,7 @@ class P4Sync(Command, P4UserMap):\n if self.tempBranches != []:\n for branch in self.tempBranches:\n read_pipe(\"git update-ref -d %s\" % branch)\n- os.rmdir(os.path.join(os.environ.get(\"GIT_DIR\", \".git\"), self.tempBranchLocation))\n+ os.rmdir(os.path.join(gitdir(), self.tempBranchLocation))\n \n # Create a symbolic ref p4/HEAD pointing to p4/ to allow\n # a convenient shortcut refname \"p4\".\n@@ -3678,23 +3679,27 @@ def main():\n (cmd, args) = parser.parse_args(sys.argv[2:], cmd);\n global verbose\n verbose = cmd.verbose\n+\n if cmd.needsGit:\n- if cmd.gitdir == None:\n- cmd.gitdir = os.path.abspath(\".git\")\n- if not isValidGitDir(cmd.gitdir):\n- cmd.gitdir = read_pipe(\"git rev-parse --git-dir\").strip()\n- if os.path.exists(cmd.gitdir):\n- cdup = read_pipe(\"git rev-parse --show-cdup\").strip()\n- if len(cdup) > 0:\n- chdir(cdup);\n-\n- if not isValidGitDir(cmd.gitdir):\n- if isValidGitDir(cmd.gitdir + \"/.git\"):\n- cmd.gitdir += \"/.git\"\n- else:\n+ if cmd.gitdir:\n+ os.environ[\"GIT_DIR\"] = cmd.gitdir\n+\n+ # did we get a valid git dir on the command line or via $GIT_DIR?\n+ if not gitdir():\n die(\"fatal: cannot locate git repository at %s\" % cmd.gitdir)\n \n- os.environ[\"GIT_DIR\"] = cmd.gitdir\n+ else:\n+ # already in a git directory?\n+ if not gitdir():\n+ die(\"fatal: not in a valid git repository\")\n+\n+ cdup = read_pipe(\"git rev-parse --show-cdup\").strip()\n+ if len(cdup) > 0:\n+ chdir(cdup);\n+\n+ # ensure subshells spawned in the p4 repo directory\n+ # get the correct GIT_DIR\n+ os.environ[\"GIT_DIR\"] = os.path.abspath(gitdir())\n \n if not cmd.run(args):\n parser.print_help()\ndiff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\nindex 0730f18..093e9bd 100755\n--- a/t/t9800-git-p4-basic.sh\n+++ b/t/t9800-git-p4-basic.sh\n@@ -257,6 +257,26 @@ test_expect_success 'submit from detached head' '\n \t)\n '\n \n+test_expect_success 'submit from worktree' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit worktree add ../worktree-test\n+\t) &&\n+\t(\n+\t\tcd \"$git/../worktree-test\" &&\n+\t\ttest_commit \"worktree-commit\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 sync &&\n+\t\ttest_path_is_file worktree-commit.t\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n2.8.2.703.g78b384c.dirty\n\n"},{"id":"307459","messageId":"CAE5ih7_6Ap_dY3mRb3Hk2yzDRMkZ3HnnQOaikF=ybx_XNdVWhQ@mail.gmail.com","threadId":"44672","inReplyTo":"20161210215734.7468-2-luke@diamand.org","subject":"Re: [PATCHv2] git-p4: support git worktrees","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-12-11T07:19:30Z","receivedAt":"2016-12-11T07:19:39Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 10 December 2016 at 21:57, Luke Diamand wrote:\n> git-p4 would attempt to find the git directory using\n> its own specific code, which did not know about git\n> worktrees. This caused git operations to fail needlessly.\n>\n> Rework it to use \"git rev-parse --git-dir\" instead, which\n> knows about worktrees.\n\nActually this doesn't work as well as the original version. \"git\nrev-parse --git-dir\" won't go and find the \".git\" subdirectory. The\nprevious version would go looking for it, so this would introduce a\nregression.\n\nLuke\n\n\n>\n> Signed-off-by: Luke Diamand \n> ---\n> git-p4.py | 47 ++++++++++++++++++++++++++---------------------\n> t/t9800-git-p4-basic.sh | 20 ++++++++++++++++++++\n> 2 files changed, 46 insertions(+), 21 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index fd5ca52..6aa8957 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -49,6 +49,13 @@ defaultLabelRegexp = r'[a-zA-Z0-9_\\-.]+$'\n> # Grab changes in blocks of this many revisions, unless otherwise requested\n> defaultBlockSize = 512\n>\n> +def gitdir():\n> + d = read_pipe(\"git rev-parse --git-dir\").strip()\n> + if not d or len(d) == 0:\n> + return None\n> + else:\n> + return d\n> +\n> def p4_build_cmd(cmd):\n> \"\"\"Build a suitable p4 command line.\n>\n> @@ -562,12 +569,6 @@ def currentGitBranch():\n> else:\n> return read_pipe([\"git\", \"name-rev\", \"HEAD\"]).split(\" \")[1].strip()\n>\n> -def isValidGitDir(path):\n> - if (os.path.exists(path + \"/HEAD\")\n> - and os.path.exists(path + \"/refs\") and os.path.exists(path + \"/objects\")):\n> - return True;\n> - return False\n> -\n> def parseRevision(ref):\n> return read_pipe(\"git rev-parse %s\" % ref).strip()\n>\n> @@ -3462,7 +3463,7 @@ class P4Sync(Command, P4UserMap):\n> if self.tempBranches != []:\n> for branch in self.tempBranches:\n> read_pipe(\"git update-ref -d %s\" % branch)\n> - os.rmdir(os.path.join(os.environ.get(\"GIT_DIR\", \".git\"), self.tempBranchLocation))\n> + os.rmdir(os.path.join(gitdir(), self.tempBranchLocation))\n>\n> # Create a symbolic ref p4/HEAD pointing to p4/ to allow\n> # a convenient shortcut refname \"p4\".\n> @@ -3678,23 +3679,27 @@ def main():\n> (cmd, args) = parser.parse_args(sys.argv[2:], cmd);\n> global verbose\n> verbose = cmd.verbose\n> +\n> if cmd.needsGit:\n> - if cmd.gitdir == None:\n> - cmd.gitdir = os.path.abspath(\".git\")\n> - if not isValidGitDir(cmd.gitdir):\n> - cmd.gitdir = read_pipe(\"git rev-parse --git-dir\").strip()\n> - if os.path.exists(cmd.gitdir):\n> - cdup = read_pipe(\"git rev-parse --show-cdup\").strip()\n> - if len(cdup) > 0:\n> - chdir(cdup);\n> -\n> - if not isValidGitDir(cmd.gitdir):\n> - if isValidGitDir(cmd.gitdir + \"/.git\"):\n> - cmd.gitdir += \"/.git\"\n> - else:\n> + if cmd.gitdir:\n> + os.environ[\"GIT_DIR\"] = cmd.gitdir\n> +\n> + # did we get a valid git dir on the command line or via $GIT_DIR?\n> + if not gitdir():\n> die(\"fatal: cannot locate git repository at %s\" % cmd.gitdir)\n>\n> - os.environ[\"GIT_DIR\"] = cmd.gitdir\n> + else:\n> + # already in a git directory?\n> + if not gitdir():\n> + die(\"fatal: not in a valid git repository\")\n> +\n> + cdup = read_pipe(\"git rev-parse --show-cdup\").strip()\n> + if len(cdup) > 0:\n> + chdir(cdup);\n> +\n> + # ensure subshells spawned in the p4 repo directory\n> + # get the correct GIT_DIR\n> + os.environ[\"GIT_DIR\"] = os.path.abspath(gitdir())\n>\n> if not cmd.run(args):\n> parser.print_help()\n> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\n> index 0730f18..093e9bd 100755\n> --- a/t/t9800-git-p4-basic.sh\n> +++ b/t/t9800-git-p4-basic.sh\n> @@ -257,6 +257,26 @@ test_expect_success 'submit from detached head' '\n> )\n> '\n>\n> +test_expect_success 'submit from worktree' '\n> + test_when_finished cleanup_git &&\n> + git p4 clone --dest=\"$git\" //depot &&\n> + (\n> + cd \"$git\" &&\n> + git worktree add ../worktree-test\n> + ) &&\n> + (\n> + cd \"$git/../worktree-test\" &&\n> + test_commit \"worktree-commit\" &&\n> + git config git-p4.skipSubmitEdit true &&\n> + git p4 submit\n> + ) &&\n> + (\n> + cd \"$cli\" &&\n> + p4 sync &&\n> + test_path_is_file worktree-commit.t\n> + )\n> +'\n> +\n> test_expect_success 'kill p4d' '\n> kill_p4d\n> '\n> --\n> 2.8.2.703.g78b384c.dirty\n>\n"},{"id":"307586","messageId":"CACsJy8A+cEJoKyfEnJvqwT4pvUGgcrmJdRmXKyRoAff4EYgcuw@mail.gmail.com","threadId":"44672","inReplyTo":"CAE5ih7_6Ap_dY3mRb3Hk2yzDRMkZ3HnnQOaikF=ybx_XNdVWhQ@mail.gmail.com","subject":"Re: [PATCHv2] git-p4: support git worktrees","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-12-13T11:17:37Z","receivedAt":"2016-12-13T11:18:14Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Dec 11, 2016 at 2:19 PM, Luke Diamand wrote:\n> On 10 December 2016 at 21:57, Luke Diamand wrote:\n>> git-p4 would attempt to find the git directory using\n>> its own specific code, which did not know about git\n>> worktrees. This caused git operations to fail needlessly.\n>>\n>> Rework it to use \"git rev-parse --git-dir\" instead, which\n>> knows about worktrees.\n>\n> Actually this doesn't work as well as the original version. \"git\n> rev-parse --git-dir\" won't go and find the \".git\" subdirectory. The\n> previous version would go looking for it, so this would introduce a\n> regression.\n\nMaybe git rev-parse --git-path HEAD, then strip out the /HEAD part?\n-- \nDuy\n"}]}