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 >> > 0000000 { s 004 \0 \0 \0 c o d e s 004 \0 \0 \0 s >> > 0000020 t a t s \t \0 \0 \0 d e p o t F i l >> > 0000040 e s 017 \0 \0 \0 / / d e p o t / s y >> > 0000060 m l i n k s 003 \0 \0 \0 r e v s 001 \0 >> > 0000100 \0 \0 1 s 006 \0 \0 \0 c h a n g e s 001 >> > 0000120 \0 \0 \0 6 s 006 \0 \0 \0 a c t i o n s >> > 0000140 003 \0 \0 \0 a d d s 004 \0 \0 \0 t y p e >> > 0000160 s \a \0 \0 \0 s y m l i n k s 004 \0 \0 >> > 0000200 \0 t i m e s \n \0 \0 \0 1 3 7 6 2 2 >> > 0000220 1 9 7 8 s \b \0 \0 \0 f i l e S i z >> > 0000240 e s 002 \0 \0 \0 1 5 0 { s 004 \0 \0 \0 c >> > 0000260 o d e s 006 \0 \0 \0 b i n a r y s 004 >> > 0000300 \0 \0 \0 d a t a s 017 \0 \0 \0 s y m l >> > 0000320 i n k - t a r g e t \n 0 { s 004 \0 >> > 0000340 \0 \0 c o d e s 006 \0 \0 \0 b i n a r >> > 0000360 y s 004 \0 \0 \0 d a t a s \0 \0 \0 \0 0 >> > 0000400 >> > >> > 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 >> >> 0000360 t e . s h \n 0 >> >> p4 -G print apps/update.sh|od -c >> 0000360 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 = "120000" >> >> - # 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