Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2

[Ben Woosley <ben.woosley@gmail.com>]

Thanks! Some thoughts:

* One option on the Travis front would be to just test one combination
of the 1.1 build - e.g. linux + clang + 1.1, so you'll stay within the
5 parallel builds while also having some coverage on lfs 1.1.
* It might be risky to match on contentFile when looking for the
prefix - there could be expansions or other modifications applied by
git-lfs between input and output. I would think it a bit safer to just
match on the beginning of the line.
* Have you guys thought about splitting out git-p4? It seems like a
good candidate for an extension / add-on, and would remove the soft
git-lfs dependency.

Best,
Ben

On Wed, Apr 20, 2016 at 12:36 PM, Lars Schneider
<larsxschneider@gmail.com> wrote:
>
>> On Wed, Apr 20, 2016 at 12:00 PM, Luke Diamand <luke@diamand.org> wrote:
>>> On 20 April 2016 at 19:28, Ben Woosley <Ben.Woosley@gmail.com> wrote:
>>>> From: Ben Woosley <ben.woosley@gmail.com>
>>>>
>>>> The git lfs pointer output was changed in:
>>>> https://github.com/github/git-lfs/pull/1105
>>>>
>>>> This was causing Mac Travis runs to fail, as homebrew had updated to 1.2
>>>> while Linux was pinned at 1.1 via GIT_LFS_VERSION.
>>>>
>>>> The travis builds against 1.1 and 1.2 both on linux. Mac can't do the same as
>>>> it takes the latest homebrew version regardless.
>>>
>>> Is this related to the very similar thread going on here:
>>>
>>> http://thread.gmane.org/gmane.comp.version-control.git/291917/focus=291926
>>>
>>> Thanks
>>> Luke
>
>
> On 20 Apr 2016, at 21:13, Ben Woosley <ben.woosley@gmail.com> wrote:
>
>> Yep it's addressing the same problem - I developed this independently
>> after having only viewed the github repo:
>> https://github.com/git/git/pull/231
>>
>> Things I like about my patch:
>> 1) it maintains ongoing support for git-lfs 1.1 by retaining it in the travis CI
>> 2) it's a fairly minimal intervention into the existing behavior
>>
>> Lars how about adding my Travis changes to your patch?
>> Here's a look at the CI output: https://travis-ci.org/git/git/builds/124105972
>>
>> Best,
>> Ben
>
>
> Thanks a lot for your fix! It's great to see other people than
> me actually using this feature :)
>
> I already sent a v2 with LFS 1.x support, too:
> http://article.gmane.org/gmane.comp.version-control.git/291993
> Would you mind reviewing it, too?
> Sorry for the duplicated work :-(
>
> Your Travis changes are 100% correct and a very nice way to ensure we
> support Git LFS 1.1 and Git LFS 1.2. Unfortunately we run all other Git
> tests twice which increases the overall build time a lot (because we
> can only run 5 build jobs in parallel with the free Travis CI plan).
> I am not sure if testing an outdated LFS version justifies the increased
> build times...
>
> Best,
> Lars
>
> PS: Please see Junio's comment regarding top posting:
> http://article.gmane.org/gmane.comp.version-control.git/291932
>
>>>
>>>
>>>
>>>> ---
>>>> .travis.yml | 9 ++++++++-
>>>> git-p4.py   | 7 ++++++-
>>>> 2 files changed, 14 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/.travis.yml b/.travis.yml
>>>> index 78e433b..71510ee 100644
>>>> --- a/.travis.yml
>>>> +++ b/.travis.yml
>>>> @@ -23,7 +23,6 @@ env:
>>>>   global:
>>>>     - DEVELOPER=1
>>>>     - P4_VERSION="15.2"
>>>> -    - GIT_LFS_VERSION="1.1.0"
>>>>     - DEFAULT_TEST_TARGET=prove
>>>>     - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
>>>>     - GIT_TEST_OPTS="--verbose --tee"
>>>> @@ -31,6 +30,14 @@ env:
>>>>     # t9810 occasionally fails on Travis CI OS X
>>>>     # t9816 occasionally fails with "TAP out of sequence errors" on Travis CI OS X
>>>>     - GIT_SKIP_TESTS="t9810 t9816"
>>>> +  matrix:
>>>> +    - GIT_LFS_VERSION="1.2.0"
>>>> +    - GIT_LFS_VERSION="1.1.0"
>>>> +
>>>> +matrix:
>>>> +  exclude:
>>>> +    - os: osx
>>>> +      env: GIT_LFS_VERSION="1.1.0"
>>>>
>>>> before_install:
>>>>   - >
>>>> diff --git a/git-p4.py b/git-p4.py
>>>> index 527d44b..6c06d17 100755
>>>> --- a/git-p4.py
>>>> +++ b/git-p4.py
>>>> @@ -1064,7 +1064,12 @@ def generatePointer(self, contentFile):
>>>>         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]]
>>>> +        pointerLines = pointerFile.split('\n')
>>>> +        # In git-lfs < 1.2, the pointer output included some extraneous information
>>>> +        # this was removed in https://github.com/github/git-lfs/pull/1105
>>>> +        if pointerLines[0].startswith('Git LFS pointer for'):
>>>> +            pointerLines = pointerLines[2:]
>>>> +        pointerContents = [i+'\n' for i in pointerLines[:-1]]
>>>>         oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
>>>>         localLargeFile = os.path.join(
>>>>             os.getcwd(),
>>>>
>>>> --
>>>> https://github.com/git/git/pull/231
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe git" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>