Re: [PATCH] git-p4: Fix support for symlinks.

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 7, 2007, 08:40 UTC
Message-ID: <7vtzrb68kq.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <200708071025.47965.simon@lst.de>

Simon Hausmann <simon@lst.de> writes:

Show 29 quoted lines

> Detect symlinks as file type, set the git file mode accordingly and strip off the trailing newline in the p4 print output.
>
> Signed-off-by: Simon Hausmann <simon@lst.de>
> ---
>  contrib/fast-import/git-p4 |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 41e86e7..9c6f911 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -839,11 +839,15 @@ class P4Sync(Command):
>              if file["action"] == "delete":
>                  self.gitStream.write("D %s\n" % relPath)
>              else:
> +                data = file['data']
> +
>                  mode = 644
>                  if file["type"].startswith("x"):
>                      mode = 755
> -
> -                data = file['data']
> +                elif file["type"] == "symlink":
> +                    mode = 120000
> +                    # p4 print on a symlink contains "target\n", so strip it off
> +                    data = data[:-1]
>  
>                  if self.isWindows and file["type"].endswith("text"):
>                      data = data.replace("\r\n", "\n")

Thanks for a quick fix.

Brian, does this resolve the issue for you? I do not have an access to p4 myself so I won't make a good judge in this area myself. An Ack is appreciated.

Simon, just a style nit.

Every time I see decimal integers 644 and/or 755, it interrupts my flow of thought and forces me to read the change and its surrounding text needlessly carefully.

If you read the code, you can see that this "mode" variable is formatted with ("%d" % mode) for writing out, and is not used as permission bit pattern in any bitwise operations, so you can tell that these constants _are_ safe. But it takes extra efforts to convince yourself that they indeed are.

I would prefer this kind of thing to be written as either:

	mode = 0644
        "%o" % mode

or

	mode = "644"
        "%s" % mode

to make it clear that the author knew what he was doing when he wrote the code.