On 12 August 2013 15:38, Pete Wyckoff <pw@xxxxxxxx> wrote:
> alexj@xxxxxxxxxx wrote on Mon, 12 Aug 2013 10:46 +0300:
>> On 11 August 2013 14:57, Pete Wyckoff <pw@xxxxxxxx> wrote:
>> > alexj@xxxxxxxxxx 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 <pw@xxxxxxxx>
>
> 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 <ajuncu@xxxxxxxxxxx>
>> >> Signed-off-by: Alex Badea <abadea@xxxxxxxxxxx>
>> >> ---
>> >> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html