From: Lars Schneider <larsxschneider@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Sebastian Schuberth <sschuberth@gmail.com>,
Git Mailing List <git@vger.kernel.org>,
luke@diamand.org
Subject: Re: [PATCH v2 2/2] git-p4: fix Git LFS pointer parsing
Date: Wed, 20 Apr 2016 22:03:59 +0200 [thread overview]
Message-ID: <ADDF0944-7EBE-4DCC-B4D9-F4662AEFBEC7@gmail.com> (raw)
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.
>> 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],
next prev parent reply other threads:[~2016-04-20 20:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-20 8:10 [PATCH v2 0/2] git-p4: fix Git LFS pointer parsing larsxschneider
2016-04-20 8:10 ` [PATCH v2 1/2] travis-ci: update Git-LFS and P4 to the latest version larsxschneider
2016-04-20 8:10 ` [PATCH v2 2/2] git-p4: fix Git LFS pointer parsing larsxschneider
2016-04-20 8:59 ` Sebastian Schuberth
2016-04-20 18:30 ` Junio C Hamano
2016-04-20 20:03 ` Lars Schneider [this message]
2016-04-20 21:01 ` Junio C Hamano
2016-04-22 7:53 ` Lars Schneider
2016-04-22 7:55 ` Sebastian Schuberth
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ADDF0944-7EBE-4DCC-B4D9-F4662AEFBEC7@gmail.com \
--to=larsxschneider@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=luke@diamand.org \
--cc=sschuberth@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).