git p4: avoid expanding client paths in chdir

The generic chdir() helper sets the PWD environment variable, as that is what is used by p4 to know its current working directory. Normally the shell would do this, but in git-p4, we must do it by hand. However, when the path contains a symbolic link, os.getcwd() will return the physical location. If the p4 client specification includes symlinks, setting PWD to the physical location causes p4 to think it is not inside the client workspace. It complains, e.g. Path /vol/bar/projects/foo/... is not under client root /p/foo One workaround is to use AltRoots in the p4 client specification, but it is cleaner to handle it directly in git-p4. Other uses of chdir still require setting PWD to an absolute path so p4 features like P4CONFIG work. See bf1d68f (git-p4: use absolute directory for PWD env var, 2011-12-09). [ pw: tweak patch and commit message ] Thanks-to: John Keeping <john@keeping.me.uk> Signed-off-by: Pete Wyckoff <pw@padd.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Miklós Fazekas committed Mar 11, 2013 at 17:45 UTC bbd848633eb0a26ccd7306cb76964ab41cff6b83
2 files changed +23 -8
git-p4.py
+22 -7
@@ -79,12 +79,27 @@ def p4_build_cmd(cmd):
79 real_cmd += cmd
80 return real_cmd
81
82 -def chdir(dir):
83 - # P4 uses the PWD environment variable rather than getcwd(). Since we're
84 - # not using the shell, we have to set it ourselves. This path could
85 - # be relative, so go there first, then figure out where we ended up.
86 - os.chdir(dir)
87 - os.environ['PWD'] = os.getcwd()
82 +def chdir(path, is_client_path=False):
83 + """Do chdir to the given path, and set the PWD environment
84 + variable for use by P4. It does not look at getcwd() output.
85 + Since we're not using the shell, it is necessary to set the
86 + PWD environment variable explicitly.
87 +
88 + Normally, expand the path to force it to be absolute. This
89 + addresses the use of relative path names inside P4 settings,
90 + e.g. P4CONFIG=.p4config. P4 does not simply open the filename
91 + as given; it looks for .p4config using PWD.
92 +
93 + If is_client_path, the path was handed to us directly by p4,
94 + and may be a symbolic link. Do not call os.getcwd() in this
95 + case, because it will cause p4 to think that PWD is not inside
96 + the client path.
97 + """
98 +
99 + os.chdir(path)
100 + if not is_client_path:
101 + path = os.getcwd()
102 + os.environ['PWD'] = path
103
104 def die(msg):
105 if verbose:
@@ -1624,7 +1639,7 @@ class P4Submit(Command, P4UserMap):
1639 new_client_dir = True
1640 os.makedirs(self.clientPath)
1641
1627 - chdir(self.clientPath)
1642 + chdir(self.clientPath, is_client_path=True)
1643 if self.dry_run:
1644 print "Would synchronize p4 checkout in %s" % self.clientPath
1645 else:
t/t9808-git-p4-chdir.sh
+1 -1
@@ -58,7 +58,7 @@ test_expect_success 'p4 client root would be relative due to clone --dest' '
58
59 # When the p4 client Root is a symlink, make sure chdir() does not use
60 # getcwd() to convert it to a physical path.
61 -test_expect_failure SYMLINKS 'p4 client root symlink should stay symbolic' '
61 +test_expect_success SYMLINKS 'p4 client root symlink should stay symbolic' '
62 physical="$TRASH_DIRECTORY/physical" &&
63 symbolic="$TRASH_DIRECTORY/symbolic" &&
64 test_when_finished "rm -rf \"$physical\"" &&