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

2016-12-13 Thread Duy Nguyen
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


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

2016-12-10 Thread Luke Diamand
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
>