Re: [PATCH v3 3/3] git-p4: fix Git LFS pointer parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24 Apr 2016, at 21:16, Sebastian Schuberth <sschuberth@xxxxxxxxx> wrote:

> On Sun, Apr 24, 2016 at 8:58 PM,  <larsxschneider@xxxxxxxxx> wrote:
> 
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -1064,8 +1064,15 @@ class GitLFS(LargeFileSystem):
>>         if pointerProcess.wait():
>>             os.remove(contentFile)
>>             die('git-lfs pointer command failed. Did you install the extension?')
>> -        pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
>> -        oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
>> +
>> +        # Git LFS removed the preamble in the output of the 'pointer' command
>> +        # starting from version 1.2.0. Check for the preamble here to support
>> +        # earlier versions.
>> +        # c.f. https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43
>> +        if pointerFile.startswith('Git LFS pointer for'):
>> +            re.sub(r'Git LFS pointer for.*\n\n', '', pointerFile)
> 
> I liked the code from v2 better. I know Ben said "there could be
> expansions or other modifications applied by git-lfs between input and
> output", but I believe it's better to be too strict than too lenient
> if you're omitting lines from the output. Also, the regex matches
> against the whole multi-line string. That is, if the file for some
> reason was ending in '\n\n' instead of just '\n', the '.*' would match
> almost all content of the pointer file, not just the remains of the
> preamble. One way to fix this would be to use
> 
> re.sub(r'Git LFS pointer for [^\n]+\n\n', '', pointerFile)
> 
> instead.

In general you are right as "*" is greedy. However, in Python "." matches any 
character except a newline [1]. Therefore I think the regex is correct.

Nevertheless... thanks for making me read the line again. I forgot to
assign the pointerFile variable in the version I sent around :-(

This is how it should be:

pointerFile = re.sub(r'Git LFS pointer for.*\n\n', '', pointerFile)

Thanks,
Lars


[1] https://docs.python.org/2/library/re.html

--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]