Merge branch 'pw/p4-symlinked-root'

"git p4" did not behave well when the path to the root of the P4
client was not its real path.

* pw/p4-symlinked-root:
git p4: avoid expanding client paths in chdir
git p4 test: should honor symlink in p4 client root
git p4 test: make sure P4CONFIG relative path works

diff --git a/git-p4.py b/git-p4.py
index 647f11020257598a8245eb1e84f64b80a2c55aca..911bbce6c5d4e19243e50d5916005872ac672961 100755 (executable)
--- a/git-p4.py
+++ b/git-p4.py
@@ -79,12 +79,27 @@ def p4_build_cmd(cmd):
         real_cmd += cmd
     return real_cmd
 
-def chdir(dir):
-    # P4 uses the PWD environment variable rather than getcwd(). Since we're
-    # not using the shell, we have to set it ourselves.  This path could
-    # be relative, so go there first, then figure out where we ended up.
-    os.chdir(dir)
-    os.environ['PWD'] = os.getcwd()
+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.
+       Since we're not using the shell, it is necessary to set the
+       PWD environment variable explicitly.
+
+       Normally, expand the path to force it to be absolute.  This
+       addresses the use of relative path names inside P4 settings,
+       e.g. P4CONFIG=.p4config.  P4 does not simply open the filename
+       as given; it looks for .p4config using PWD.
+
+       If is_client_path, the path was handed to us directly by p4,
+       and may be a symbolic link.  Do not call os.getcwd() in this
+       case, because it will cause p4 to think that PWD is not inside
+       the client path.
+       """
+
+    os.chdir(path)
+    if not is_client_path:
+        path = os.getcwd()
+    os.environ['PWD'] = path
 
 def die(msg):
     if verbose:
@@ -1624,7 +1639,7 @@ def run(self, args):
             new_client_dir = True
             os.makedirs(self.clientPath)
 
-        chdir(self.clientPath)
+        chdir(self.clientPath, is_client_path=True)
         if self.dry_run:
             print "Would synchronize p4 checkout in %s" % self.clientPath
         else:

diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
index dc92e60cd6f09fa5e2610d491cf1c07d43c45dfd..11d2b5102cde384ad11540610a52f62c4732aa8c 100755 (executable)
--- a/t/t9808-git-p4-chdir.sh
+++ b/t/t9808-git-p4-chdir.sh
@@ -42,6 +42,47 @@ test_expect_success 'P4CONFIG and relative dir clone' '
        )
 '
 
+# Common setup using .p4config to set P4CLIENT and P4PORT breaks
+# if clone destination is relative.  Make sure that chdir() expands
+# the relative path in --dest to absolute.
+test_expect_success 'p4 client root would be relative due to clone --dest' '
+       test_when_finished cleanup_git &&
+       (
+               echo P4PORT=$P4PORT >git/.p4config &&
+               P4CONFIG=.p4config &&
+               export P4CONFIG &&
+               unset P4PORT &&
+               git p4 clone --dest="git" //depot
+       )
+'
+
+# When the p4 client Root is a symlink, make sure chdir() does not use
+# getcwd() to convert it to a physical path.
+test_expect_success SYMLINKS 'p4 client root symlink should stay symbolic' '
+       physical="$TRASH_DIRECTORY/physical" &&
+       symbolic="$TRASH_DIRECTORY/symbolic" &&
+       test_when_finished "rm -rf \"$physical\"" &&
+       test_when_finished "rm \"$symbolic\"" &&
+       mkdir -p "$physical" &&
+       ln -s "$physical" "$symbolic" &&
+       test_when_finished cleanup_git &&
+       (
+               P4CLIENT=client-sym &&
+               p4 client -i <<-EOF &&
+               Client: $P4CLIENT
+               Description: $P4CLIENT
+               Root: $symbolic
+               LineEnd: unix
+               View: //depot/... //$P4CLIENT/...
+               EOF
+               git p4 clone --dest="$git" //depot &&
+               cd "$git" &&
+               test_commit file2 &&
+               git config git-p4.skipSubmitEdit true &&
+               git p4 submit
+       )
+'
+
 test_expect_success 'kill p4d' '
        kill_p4d
 '