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

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

[Ben Woosley]

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

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

[Luke Diamand]

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



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

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

[Ben Woosley]

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

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

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

[Lars Schneider]

> 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

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

[Ben Woosley]

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
>

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

[Lars Schneider]

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

> On 20 Apr 2016, at 23:10, Ben Woosley <Ben.Woosley@gmail.com> wrote:
> 
> Thanks! Some thoughts:
Again, please see Junio's comment regarding top posting:
http://article.gmane.org/gmane.comp.version-control.git/291932

> 
> * 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.
TBH I still think testing an outdated Git LFS version does not justify
+10 extra minutes of computing. Plus if we want to be consistent we would
need to do the same for LFS 1.0, 1.2, and for pretty much every other
dependency...  


> * 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.
Agreed. I will incorporate this in my next version and CC you.


> * 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.
Yes, this was previously discussed here:
http://article.gmane.org/gmane.comp.version-control.git/276822/


Cheers,
Lars

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

[SZEDER Gábor]

> > * 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.
> TBH I still think testing an outdated Git LFS version does not justify
> +10 extra minutes of computing.

I agree that checking compatibility with an older Git LFS version
doesn't worth the extra 10 minutes.  However, since Git LFS is only
involved in two test scripts and for the rest of the test suite it
doesn't matter at all, doing a full build and running the whole test
suite for the sole sake of a different Git LFS version is definitely
unnecessary.

The Bash completion and prompt scripts are in a similar situation:
there are only two test scripts involved and the rest of the test
suite couldn't care less.  However, we definitely want to support
older Bash versions as well, all the way back to v3.0, and there are a
few commits fixing breakages reported by users of old Bash versions.

As I somehow grew fond of those Bash scripts over the years, I put
together a couple of patches allowing me to say 'cd t && make -j4
full-bash-test', which runs the completion and prompt tests with
multiple Bash versions.  For the seven major and minor releases
including and after v3.0 it usually takes less than 8 seconds.  As far
as runtime goes, I think that's well worth it.

You can have a look at these patches at

  https://github.com/szeder/git completion-test-multiple-bash-versions

and perhaps you could even adapt it to LFS and/or p4 somehow.

> Plus if we want to be consistent we would
> need to do the same for LFS 1.0, 1.2, and for pretty much every other
> dependency...  

I'm not sure we should be consistent in this case, at least not solely
for consistency's sake and not in git.git. Taking what I did for Bash
and doing it for different versions of LFS, p4, etc. could perhaps
keep the runtime under control, but t/Makefile would surely get out
of control rather quickly.  Putting these into a travis-ci matrix is
so much simpler, but the runtime makes it infeasible, of course.

I think the best we can do is to keep this out of git.git and let
(hope?) developers interested in a particular subsystem do this
"multiple version compatibility" tests as they see fit.

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

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> You can have a look at these patches at
>
>   https://github.com/szeder/git completion-test-multiple-bash-versions
>
> and perhaps you could even adapt it to LFS and/or p4 somehow.
>
>> Plus if we want to be consistent we would
>> need to do the same for LFS 1.0, 1.2, and for pretty much every other
>> dependency...  
>
> I'm not sure we should be consistent in this case, at least not solely
> for consistency's sake and not in git.git. Taking what I did for Bash
> and doing it for different versions of LFS, p4, etc. could perhaps
> keep the runtime under control, but t/Makefile would surely get out
> of control rather quickly.  Putting these into a travis-ci matrix is
> so much simpler, but the runtime makes it infeasible, of course.

I took a brief look of your branch, and I like its approach.  If I
understood your approach correctly, you:

 * Group selected tests in t/ as "these are bash related tests I
   care about" in t/Makefile;

 * Add Travis test target to build Git with specific versions of
   bash, and run the above target instead of the full test to
   exercise the version of bash you are testing.

And I agree that the same can be done for LFS versions and P4
versions.  Only a handful tests in t/ are about these niches.

> I think the best we can do is to keep this out of git.git and let
> (hope?) developers interested in a particular subsystem do this
> "multiple version compatibility" tests as they see fit.

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

[SZEDER Gábor]

Quoting Junio C Hamano <gitster@pobox.com>:

> SZEDER Gábor <szeder@ira.uka.de> writes:
>
>> You can have a look at these patches at
>>
>>   https://github.com/szeder/git completion-test-multiple-bash-versions
>>
>> and perhaps you could even adapt it to LFS and/or p4 somehow.
>>
>>> Plus if we want to be consistent we would
>>> need to do the same for LFS 1.0, 1.2, and for pretty much every other
>>> dependency...
>>
>> I'm not sure we should be consistent in this case, at least not solely
>> for consistency's sake and not in git.git. Taking what I did for Bash
>> and doing it for different versions of LFS, p4, etc. could perhaps
>> keep the runtime under control, but t/Makefile would surely get out
>> of control rather quickly.  Putting these into a travis-ci matrix is
>> so much simpler, but the runtime makes it infeasible, of course.
>
> I took a brief look of your branch, and I like its approach.  If I
> understood your approach correctly, you:
>
>  * Group selected tests in t/ as "these are bash related tests I
>    care about" in t/Makefile;

Yes.

>  * Add Travis test target to build Git with specific versions of
>    bash, and run the above target instead of the full test to
>    exercise the version of bash you are testing.

Not quite.

   * Add t/Makefile targets to run a Bash-related test script with a
     specific Bash version, one target for each script-version pair,
     and a target to run all Bash-related tests with all listed
     Bash-versions.

   * Extend the travis-ci config so that, after building Git as usual
     and running the full test suite as usual, it additionaly runs all
     Bash-related tests will all listed Bash versions on Linux builds.

> And I agree that the same can be done for LFS versions and P4
> versions.  Only a handful tests in t/ are about these niches.

Luckily for me, running a test script with a specific Bash version is
as trivial as '/path/to/bash-vX.Y t9902-completion.sh'.  No
modifications to the test scripts or to lib-bash.sh were necessary.

OTOH, Git LFS and p4 tests, AFAICS, rely on git-lfs and p4 binaries
being available in $PATH, and the p4 tests need two binaries.  So
there is more work that has to be done, as we would need a way to
override those binaries found in $PATH, either through an environment
variable or a command line option.  Bonus points for a solution that
would work equally well with LFS, p4 and Bash: then perhaps we could
have a single unified block of Makefile metaprogramming, which could
generate  all those test targets from a list of dependency-specific
tests and a list of paths to different versions of that dependency.
Then it might even be suitable for inclusion in git.git.


>> I think the best we can do is to keep this out of git.git and let
>> (hope?) developers interested in a particular subsystem do this
>> "multiple version compatibility" tests as they see fit.

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

[Lars Schneider]

On 26 Apr 2016, at 01:10, SZEDER Gábor <szeder@ira.uka.de> wrote:

> 
> Quoting Junio C Hamano <gitster@pobox.com>:
> 
>> SZEDER Gábor <szeder@ira.uka.de> writes:
>> 
>>> You can have a look at these patches at
>>> 
>>>  https://github.com/szeder/git completion-test-multiple-bash-versions
>>> 
>>> and perhaps you could even adapt it to LFS and/or p4 somehow.
>>> 
>>>> Plus if we want to be consistent we would
>>>> need to do the same for LFS 1.0, 1.2, and for pretty much every other
>>>> dependency...
>>> 
>>> I'm not sure we should be consistent in this case, at least not solely
>>> for consistency's sake and not in git.git. Taking what I did for Bash
>>> and doing it for different versions of LFS, p4, etc. could perhaps
>>> keep the runtime under control, but t/Makefile would surely get out
>>> of control rather quickly.  Putting these into a travis-ci matrix is
>>> so much simpler, but the runtime makes it infeasible, of course.
>> 
>> I took a brief look of your branch, and I like its approach.  If I
>> understood your approach correctly, you:
>> 
>> * Group selected tests in t/ as "these are bash related tests I
>>   care about" in t/Makefile;
> 
> Yes.
> 
>> * Add Travis test target to build Git with specific versions of
>>   bash, and run the above target instead of the full test to
>>   exercise the version of bash you are testing.
> 
> Not quite.
> 
>  * Add t/Makefile targets to run a Bash-related test script with a
>    specific Bash version, one target for each script-version pair,
>    and a target to run all Bash-related tests with all listed
>    Bash-versions.
> 
>  * Extend the travis-ci config so that, after building Git as usual
>    and running the full test suite as usual, it additionaly runs all
>    Bash-related tests will all listed Bash versions on Linux builds.
> 
>> And I agree that the same can be done for LFS versions and P4
>> versions.  Only a handful tests in t/ are about these niches.
> 
> Luckily for me, running a test script with a specific Bash version is
> as trivial as '/path/to/bash-vX.Y t9902-completion.sh'.  No
> modifications to the test scripts or to lib-bash.sh were necessary.
> 
> OTOH, Git LFS and p4 tests, AFAICS, rely on git-lfs and p4 binaries
> being available in $PATH, and the p4 tests need two binaries.  So
> there is more work that has to be done, as we would need a way to
> override those binaries found in $PATH, either through an environment
> variable or a command line option.  Bonus points for a solution that
> would work equally well with LFS, p4 and Bash: then perhaps we could
> have a single unified block of Makefile metaprogramming, which could
> generate  all those test targets from a list of dependency-specific
> tests and a list of paths to different versions of that dependency.
> Then it might even be suitable for inclusion in git.git.

Thanks for sharing this! It's awesome! I will try to find a generic
solution based on your work that handles bash versions, LFS versions,
and P4 versions! Stay tuned :-)

Cheers,
Lars

> 
> 
>>> I think the best we can do is to keep this out of git.git and let
>>> (hope?) developers interested in a particular subsystem do this
>>> "multiple version compatibility" tests as they see fit.
> 
>