Re: [PATCH] git p4: chdir resolves symlinks only for relative paths
On Thu, Mar 07, 2013 at 09:36:06AM +0100, Miklós Fazekas wrote:
Show 34 quoted lines
> Sorry for the late turnaround here is an improved version. Now chdir
> has an optional argument client_path, if it's true then we don't do
> os.getcwd. I think that my first patch is also valid too - when the
> path is absolute no need for getcwd no matter what is the context,
> when it's relative we have to use os.getcwd() no matter of the
> context.
>
> ---
> If p4 client is configured to /p/foo which is a symlink:
> /p/foo -> /vol/barvol/projects/foo. Then resolving the
> symlink will confuse p4:
> "Path /vol/barvol/projects/foo/... is not under client root
> /p/foo". While AltRoots in p4 client specification can be
> used as a workaround on p4 side, git-p4 should not resolve
> symlinks in client paths.
> chdir(dir) uses os.getcwd() after os.chdir(dir) to resolve
> relative paths, but as a sideeffect it resolves symlinks
> too. Now for client paths we don't call os.getcwd().
>
> Signed-off-by: Miklós Fazekas <mfazekas@szemafor.com>
> ---
> git-p4.py | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 0682e61..2bd8cc2 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -68,12 +68,17 @@ def p4_build_cmd(cmd):
> real_cmd += cmd
> return real_cmd
>
> -def chdir(dir):
> +def chdir(dir,client_path=False):
Style (space after comma):
def chdir(dir, client_path=False):
Show 11 quoted lines
> # 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.getcwd() will resolve symlinks, so we should avoid it for
> + # client_paths.
> os.chdir(dir)
> - os.environ['PWD'] = os.getcwd()
> + if client_path:
> + os.environ['PWD'] = dir
> + else:
> + os.environ['PWD'] = os.getcwd()
Indentation seems to have gone a bit wrong here...
Show 9 quoted lines
>
> def die(msg):
> if verbose:
> @@ -1554,7 +1559,7 @@ class P4Submit(Command, P4UserMap):
> new_client_dir = True
> os.makedirs(self.clientPath)
>
> - chdir(self.clientPath)
> + chdir(self.clientPath,client_path=True)
Again, there should be a space after the comma here.
Show 5 quoted lines
> if self.dry_run:
> print "Would synchronize p4 checkout in %s" % self.clientPath
> else:
> --
> 1.7.10.2 (Apple Git-33)
John Keeping <john@keeping.me.uk>