Re: [PATCH v2 2/2] git-p4: fix Git LFS pointer parsing

From: Lars Schneider <larsxschneider@gmail.com>
Date: Apr 20, 2016, 20:03 UTC
Message-ID: <ADDF0944-7EBE-4DCC-B4D9-F4662AEFBEC7@gmail.com>
In-Reply-To: <xmqq8u08w2tb.fsf@gitster.mtv.corp.google.com>

On 20 Apr 2016, at 20:30, Junio C Hamano <gitster@pobox.com> wrote:

> Sebastian Schuberth <sschuberth@gmail.com> writes:
> 
>> Why do we need to remove the preamble at all, if present?

Because I need the content of a valid Git LFS pointer file a few lines below: https://github.com/larsxschneider/git/blob/5ee7601c49b6eaa9da5eb47db5cf12b5d8d672c9/git-p4.py#L1085

This pointer file content is then written to the Git repository instead of the actual file content.

Show 17 quoted lines

>> If all we
>> want is the oid, we should simply only look at the line that starts
>> with that keyword, which would skip any preamble. Which is what you
>> already do here. However, I'd probably use .splitlines() instead of
>> .split('\n') and .startswith('oid ') (note the trailing space) instead
>> of .startswith('oid') to ensure "oid" is a separate word.
>> 
>> But then again, I wonder why there's so much split() logic involved in
>> extracting the oid. Couldn't we replace all of that with a regexp like
>> 
>> oid = re.search(r"^oid \w+:(\w+)", pointerFile, re.MULTILINE).group(1)
> 
> Yup, all of that is a very useful suggestion.  If we know how the
> piece of information we want is identified in the output,
> specifically looking for it would future-proof the code better, as
> it will not be affected by future change that adds unexpected cruft
> to the output we are reading from.

I agree that this is a better solution.

@Junio: Can you squash the patch below or do you prefer a v3?

Thanks, Lars

diff --git a/git-p4.py b/git-p4.py
index ab52bc3..a0d529b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1073,8 +1073,7 @@ class GitLFS(LargeFileSystem):
         if pointerFile.startswith(preamble):
             pointerFile = pointerFile[len(preamble):]

-        oidEntry = [i for i in pointerFile.split('\n') if i.startswith('oid')]
-        oid = oidEntry[0].split(' ')[1].split(':')[1]
+        oid = re.search(r'^oid \w+:(\w+)', pointerFile, re.MULTILINE).group(1)
         localLargeFile = os.path.join(
             os.getcwd(),
             '.git', 'lfs', 'objects', oid[:2], oid[2:4],