Re: [PATCH] git-p4: Fix occasional truncation of symlink contents.

2013-08-12 Thread Alexandru Juncu
On 11 August 2013 14:57, Pete Wyckoff p...@padd.com wrote:
 al...@rosedu.org wrote on Thu, 08 Aug 2013 16:17 +0300:
 Symlink contents in p4 print sometimes have a trailing
 new line character, but sometimes it doesn't. git-p4
 should only remove the last character if that character
 is '\n'.

 Your patch looks fine, and harmless if symlinks continue
 to have \n on the end.  I'd like to understand a bit why
 this behavior is different for you, though.  Could you do
 this test on a symlink in your depot?

 Here //depot/symlink points to symlink-target.  You can
 see the \n in position 0o332 below.  What happens on a
 symlink in your repo?

 arf-git-test$ p4 fstat //depot/symlink
 ... depotFile //depot/symlink
 ... clientFile /dev/shm/trash directory.t9802-git-p4-filetype/cli/symlink
 ... isMapped
 ... headAction add
 ... headType symlink
 ... headTime 1376221978
 ... headRev 1
 ... headChange 6
 ... headModTime 1376221978
 ... haveRev 1

 arf-git-test$ p4 -G print //depot/symlink | od -c
 000   {   s 004  \0  \0  \0   c   o   d   e   s 004  \0  \0  \0   s
 020   t   a   t   s  \t  \0  \0  \0   d   e   p   o   t   F   i   l
 040   e   s 017  \0  \0  \0   /   /   d   e   p   o   t   /   s   y
 060   m   l   i   n   k   s 003  \0  \0  \0   r   e   v   s 001  \0
 100  \0  \0   1   s 006  \0  \0  \0   c   h   a   n   g   e   s 001
 120  \0  \0  \0   6   s 006  \0  \0  \0   a   c   t   i   o   n   s
 140 003  \0  \0  \0   a   d   d   s 004  \0  \0  \0   t   y   p   e
 160   s  \a  \0  \0  \0   s   y   m   l   i   n   k   s 004  \0  \0
 200  \0   t   i   m   e   s  \n  \0  \0  \0   1   3   7   6   2   2
 220   1   9   7   8   s  \b  \0  \0  \0   f   i   l   e   S   i   z
 240   e   s 002  \0  \0  \0   1   5   0   {   s 004  \0  \0  \0   c
 260   o   d   e   s 006  \0  \0  \0   b   i   n   a   r   y   s 004
 300  \0  \0  \0   d   a   t   a   s 017  \0  \0  \0   s   y   m   l
 320   i   n   k   -   t   a   r   g   e   t  \n   0   {   s 004  \0
 340  \0  \0   c   o   d   e   s 006  \0  \0  \0   b   i   n   a   r
 360   y   s 004  \0  \0  \0   d   a   t   a   s  \0  \0  \0  \0   0
 400

 Also, what version is your server, from p4 info:

 Server version: P4D/LINUX26X86_64/2013.1/610569 (2013/03/19)

 -- Pete


Hello!

Let me give you an example. Here are the links as resulted in the git
p4 clone (one was correct, one wasn't):

./lib/update.sh - ../update.sh
./apps/update.sh - ../update.s


alexj@ixro-alexj ~/perforce $ p4 print path/lib/update.sh
//path/update.sh#6 - edit change 119890 (symlink)
../update.sh
alexj@ixro-alexj ~/perforce $ p4 print path/apps/update.sh
//path/apps/update.sh#144 - edit change 116063 (symlink)
../update.shalexj@ixro-alexj ~/perforce/unstable $

Notice the output and the prompt.

(I removed the exact path to the file from the output)

The fstat output is kind of irrelevant, but the hexdump shows the missing \n:

p4 -G print lib/update.sh|od -c

360   t   e   .   s   h  \n   0

p4 -G print apps/update.sh|od -c
360   p   d   a   t   e   .   s   h   0


Server version: P4D/LINUX26X86_64/2013.1/610569

 Signed-off-by: Alex Juncu aju...@ixiacom.com
 Signed-off-by: Alex Badea aba...@ixiacom.com
 ---
  git-p4.py | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

 diff --git a/git-p4.py b/git-p4.py
 index 31e71ff..a53a6dc 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -2180,9 +2180,13 @@ class P4Sync(Command, P4UserMap):
  git_mode = 100755
  if type_base == symlink:
  git_mode = 12
 -# p4 print on a symlink contains target\n; remove the newline
 +# p4 print on a symlink sometimes contains target\n;
 +# if it does, remove the newline
  data = ''.join(contents)
 -contents = [data[:-1]]
 +if data[-1] == '\n':
 +contents = [data[:-1]]
 +else:
 +contents = [data]

  if type_base == utf16:
  # p4 delivers different text in the python output to -G
 --
 1.8.4.rc0.1.g8f6a3e5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-p4: Fix occasional truncation of symlink contents.

2013-08-12 Thread Alexandru Juncu
On 12 August 2013 15:38, Pete Wyckoff p...@padd.com wrote:
 al...@rosedu.org wrote on Mon, 12 Aug 2013 10:46 +0300:
 On 11 August 2013 14:57, Pete Wyckoff p...@padd.com wrote:
  al...@rosedu.org wrote on Thu, 08 Aug 2013 16:17 +0300:
  Symlink contents in p4 print sometimes have a trailing
  new line character, but sometimes it doesn't. git-p4
  should only remove the last character if that character
  is '\n'.
 
  Your patch looks fine, and harmless if symlinks continue
  to have \n on the end.  I'd like to understand a bit why
  this behavior is different for you, though.  Could you do
  this test on a symlink in your depot?
 
  Here //depot/symlink points to symlink-target.  You can
  see the \n in position 0o332 below.  What happens on a
  symlink in your repo?
 
  arf-git-test$ p4 fstat //depot/symlink
  ... depotFile //depot/symlink
  ... clientFile /dev/shm/trash 
  directory.t9802-git-p4-filetype/cli/symlink
  ... isMapped
  ... headAction add
  ... headType symlink
  ... headTime 1376221978
  ... headRev 1
  ... headChange 6
  ... headModTime 1376221978
  ... haveRev 1
 
  arf-git-test$ p4 -G print //depot/symlink | od -c
  000   {   s 004  \0  \0  \0   c   o   d   e   s 004  \0  \0  \0   s
  020   t   a   t   s  \t  \0  \0  \0   d   e   p   o   t   F   i   l
  040   e   s 017  \0  \0  \0   /   /   d   e   p   o   t   /   s   y
  060   m   l   i   n   k   s 003  \0  \0  \0   r   e   v   s 001  \0
  100  \0  \0   1   s 006  \0  \0  \0   c   h   a   n   g   e   s 001
  120  \0  \0  \0   6   s 006  \0  \0  \0   a   c   t   i   o   n   s
  140 003  \0  \0  \0   a   d   d   s 004  \0  \0  \0   t   y   p   e
  160   s  \a  \0  \0  \0   s   y   m   l   i   n   k   s 004  \0  \0
  200  \0   t   i   m   e   s  \n  \0  \0  \0   1   3   7   6   2   2
  220   1   9   7   8   s  \b  \0  \0  \0   f   i   l   e   S   i   z
  240   e   s 002  \0  \0  \0   1   5   0   {   s 004  \0  \0  \0   c
  260   o   d   e   s 006  \0  \0  \0   b   i   n   a   r   y   s 004
  300  \0  \0  \0   d   a   t   a   s 017  \0  \0  \0   s   y   m   l
  320   i   n   k   -   t   a   r   g   e   t  \n   0   {   s 004  \0
  340  \0  \0   c   o   d   e   s 006  \0  \0  \0   b   i   n   a   r
  360   y   s 004  \0  \0  \0   d   a   t   a   s  \0  \0  \0  \0   0
  400
 
  Also, what version is your server, from p4 info:
 
  Server version: P4D/LINUX26X86_64/2013.1/610569 (2013/03/19)
 
  -- Pete
 

 Hello!

 Let me give you an example. Here are the links as resulted in the git
 p4 clone (one was correct, one wasn't):

 ./lib/update.sh - ../update.sh
 ./apps/update.sh - ../update.s


 alexj@ixro-alexj ~/perforce $ p4 print path/lib/update.sh
 //path/update.sh#6 - edit change 119890 (symlink)
 ../update.sh
 alexj@ixro-alexj ~/perforce $ p4 print path/apps/update.sh
 //path/apps/update.sh#144 - edit change 116063 (symlink)
 ../update.shalexj@ixro-alexj ~/perforce/unstable $

 Notice the output and the prompt.

 (I removed the exact path to the file from the output)

 The fstat output is kind of irrelevant, but the hexdump shows the missing \n:

 p4 -G print lib/update.sh|od -c

 360   t   e   .   s   h  \n   0

 p4 -G print apps/update.sh|od -c
 360   p   d   a   t   e   .   s   h   0

 I had forgotten, in fact, another thread on this very topic:

 http://thread.gmane.org/gmane.comp.version-control.git/221500

 Now with your evidence, I think we can decide that no matter how
 the symlink managed to sneak into p4d, git p4 should be able to
 handle it.

 The only problem is that due to the \n ambiguity, in your setup
 where p4d loses the \n, git p4 will not handle symlinks with a
 target that ends with a newline, e.g.:

 symlink(foo\n, bar);

 But the small chance of someone actually doing that, coupled with
 the fact that for you git p4 is unusable with these symlinks,
 says we should go ahead and make the change.

 You should send the patch to junio for inclusion in pu/ for the
 next release, with:

 Acked-by: Pete Wyckoff p...@padd.com

 Thanks for fixing this!

 -- Pete

Sorry, I didn't get where I am supposed to submit the patch (I thought
I was supposed to send it here, lkml style).


 Server version: P4D/LINUX26X86_64/2013.1/610569

  Signed-off-by: Alex Juncu aju...@ixiacom.com
  Signed-off-by: Alex Badea aba...@ixiacom.com
  ---
   git-p4.py | 8 ++--
   1 file changed, 6 insertions(+), 2 deletions(-)
 
  diff --git a/git-p4.py b/git-p4.py
  index 31e71ff..a53a6dc 100755
  --- a/git-p4.py
  +++ b/git-p4.py
  @@ -2180,9 +2180,13 @@ class P4Sync(Command, P4UserMap):
   git_mode = 100755
   if type_base == symlink:
   git_mode = 12
  -# p4 print on a symlink contains target\n; remove the 
  newline
  +# p4 print on a symlink sometimes contains 

[PATCH] git-p4: Fix occasional truncation of symlink contents.

2013-08-08 Thread Alexandru Juncu
Symlink contents in p4 print sometimes have a trailing
new line character, but sometimes it doesn't. git-p4
should only remove the last character if that character
is '\n'.

Signed-off-by: Alex Juncu aju...@ixiacom.com
Signed-off-by: Alex Badea aba...@ixiacom.com
---
 git-p4.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 31e71ff..a53a6dc 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2180,9 +2180,13 @@ class P4Sync(Command, P4UserMap):
 git_mode = 100755
 if type_base == symlink:
 git_mode = 12
-# p4 print on a symlink contains target\n; remove the newline
+# p4 print on a symlink sometimes contains target\n;
+# if it does, remove the newline
 data = ''.join(contents)
-contents = [data[:-1]]
+if data[-1] == '\n':
+contents = [data[:-1]]
+else:
+contents = [data]
 
 if type_base == utf16:
 # p4 delivers different text in the python output to -G
-- 
1.8.4.rc0.1.g8f6a3e5

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html