Re: [PATCHv3] git-p4: support git worktrees

2016-12-13 Thread Junio C Hamano
Luke Diamand  writes:

> git-p4 would attempt to find the git directory using
> its own specific code, which did not know about git
> worktrees.
>
> Rework it to use "git rev-parse --git-dir" instead.
>
> Add test cases for worktree usage and specifying
> git directory via --git-dir and $GIT_DIR.
>
> Signed-off-by: Luke Diamand 
> ---
>  git-p4.py | 17 +
>  t/t9800-git-p4-basic.sh   | 20 
>  t/t9806-git-p4-options.sh | 32 
>  3 files changed, 65 insertions(+), 4 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index fd5ca52..6a1f65f 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -85,6 +85,16 @@ def p4_build_cmd(cmd):
>  real_cmd += cmd
>  return real_cmd
>  
> +def git_dir(path):
> +""" Return TRUE if the given path is a git directory (/path/to/dir/.git).
> +This won't automatically add ".git" to a directory.
> +"""
> +d = read_pipe(["git", "--git-dir", path, "rev-parse", "--git-dir"], 
> True).strip()
> +if not d or len(d) == 0:
> +return None
> +else:
> +return d
> +
>  def chdir(path, is_client_path=False):
>  """Do chdir to the given path, and set the PWD environment
> variable for use by P4.  It does not look at getcwd() output.
> @@ -563,10 +573,7 @@ def currentGitBranch():
>  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
> +return git_dir(path) != None
>  
>  def parseRevision(ref):
>  return read_pipe("git rev-parse %s" % ref).strip()
> @@ -3682,6 +3689,7 @@ def main():
>  if cmd.gitdir == None:
>  cmd.gitdir = os.path.abspath(".git")
>  if not isValidGitDir(cmd.gitdir):
> +# "rev-parse --git-dir" without arguments will try $PWD/.git
>  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()
> @@ -3694,6 +3702,7 @@ def main():
>  else:
>  die("fatal: cannot locate git repository at %s" % cmd.gitdir)
>  
> +# so git commands invoked from the P4 workspace will succeed
>  os.environ["GIT_DIR"] = cmd.gitdir

The real fix has become surprisingly short and "feels right".

Will queue. Thanks.



[PATCHv3] git-p4: support git worktrees

2016-12-13 Thread Luke Diamand
git-p4 would attempt to find the git directory using
its own specific code, which did not know about git
worktrees.

Rework it to use "git rev-parse --git-dir" instead.

Add test cases for worktree usage and specifying
git directory via --git-dir and $GIT_DIR.

Signed-off-by: Luke Diamand 
---
 git-p4.py | 17 +
 t/t9800-git-p4-basic.sh   | 20 
 t/t9806-git-p4-options.sh | 32 
 3 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index fd5ca52..6a1f65f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -85,6 +85,16 @@ def p4_build_cmd(cmd):
 real_cmd += cmd
 return real_cmd
 
+def git_dir(path):
+""" Return TRUE if the given path is a git directory (/path/to/dir/.git).
+This won't automatically add ".git" to a directory.
+"""
+d = read_pipe(["git", "--git-dir", path, "rev-parse", "--git-dir"], 
True).strip()
+if not d or len(d) == 0:
+return None
+else:
+return d
+
 def chdir(path, is_client_path=False):
 """Do chdir to the given path, and set the PWD environment
variable for use by P4.  It does not look at getcwd() output.
@@ -563,10 +573,7 @@ def currentGitBranch():
 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
+return git_dir(path) != None
 
 def parseRevision(ref):
 return read_pipe("git rev-parse %s" % ref).strip()
@@ -3682,6 +3689,7 @@ def main():
 if cmd.gitdir == None:
 cmd.gitdir = os.path.abspath(".git")
 if not isValidGitDir(cmd.gitdir):
+# "rev-parse --git-dir" without arguments will try $PWD/.git
 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()
@@ -3694,6 +3702,7 @@ def main():
 else:
 die("fatal: cannot locate git repository at %s" % cmd.gitdir)
 
+# so git commands invoked from the P4 workspace will succeed
 os.environ["GIT_DIR"] = cmd.gitdir
 
 if not cmd.run(args):
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
 '
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 254d428..1ab76c4 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -269,6 +269,38 @@ test_expect_success 'submit works with two branches' '
)
 '
 
+test_expect_success 'use --git-dir option and GIT_DIR' '
+   test_when_finished cleanup_git &&
+   git p4 clone //depot --destination="$git" &&
+   (
+   cd "$git" &&
+   git config git-p4.skipSubmitEdit true &&
+   test_commit first-change &&
+   git p4 submit --git-dir "$git"
+   ) &&
+   (
+   cd "$cli" &&
+   p4 sync &&
+   test_path_is_file first-change.t &&
+   echo "cli_file" >cli_file.t &&
+   p4 add cli_file.t &&
+   p4 submit -d "cli change"
+   ) &&
+   (git --git-dir "$git" p4 sync) &&
+   (cd "$git" && git checkout -q p4/master) &&
+   test_path_is_file "$git"/cli_file.t &&
+   (
+   cd "$cli" &&
+   echo "cli_file2" >cli_file2.t &&
+   p4 add cli_file2.t  &&
+   p4 submit -d "cli change2"
+   ) &&
+   (GIT_DIR="$git" git p4 sync) &&
+   (cd "$git" && git checkout -q p4/master) &&
+   test_path_is_file "$git"/cli_file2.t
+'
+
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
2.8.2.703.g78b384c.dirty