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

From: Sebastian Schuberth <sschuberth@gmail.com>
Date: Apr 24, 2016, 19:16 UTC
Message-ID: <CAHGBnuMs6h478FXGA_x2nP1D6BJCxGzhumR_xq5JFmNgzM6Sdw@mail.gmail.com>
In-Reply-To: <1461524292-20490-4-git-send-email-larsxschneider@gmail.com>

On Sun, Apr 24, 2016 at 8:58 PM,  <larsxschneider@gmail.com> wrote:

Show 15 quoted lines

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

Regards, Sebastian