* [PATCH] Update git-p4 to be compatible with git-lfs 1.2 @ 2016-04-20 18:28 Ben Woosley 2016-04-20 19:00 ` Luke Diamand 0 siblings, 1 reply; 10+ messages in thread From: Ben Woosley @ 2016-04-20 18:28 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2 2016-04-20 18:28 [PATCH] Update git-p4 to be compatible with git-lfs 1.2 Ben Woosley @ 2016-04-20 19:00 ` Luke Diamand 2016-04-20 19:13 ` Ben Woosley 0 siblings, 1 reply; 10+ messages in thread From: Luke Diamand @ 2016-04-20 19:00 UTC (permalink / raw) To: Ben Woosley; +Cc: Git Users 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2 2016-04-20 19:00 ` Luke Diamand @ 2016-04-20 19:13 ` Ben Woosley 2016-04-20 19:36 ` Lars Schneider 0 siblings, 1 reply; 10+ messages in thread From: Ben Woosley @ 2016-04-20 19:13 UTC (permalink / raw) To: Luke Diamand, larsxschneider; +Cc: Git Users 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2 2016-04-20 19:13 ` Ben Woosley @ 2016-04-20 19:36 ` Lars Schneider 2016-04-20 21:10 ` Ben Woosley 0 siblings, 1 reply; 10+ messages in thread From: Lars Schneider @ 2016-04-20 19:36 UTC (permalink / raw) To: Ben Woosley; +Cc: Luke Diamand, Git Users > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2 2016-04-20 19:36 ` Lars Schneider @ 2016-04-20 21:10 ` Ben Woosley 2016-04-22 8:10 ` Lars Schneider 0 siblings, 1 reply; 10+ messages in thread From: Ben Woosley @ 2016-04-20 21:10 UTC (permalink / raw) To: Lars Schneider; +Cc: Luke Diamand, Git Users 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2 2016-04-20 21:10 ` Ben Woosley @ 2016-04-22 8:10 ` Lars Schneider 2016-04-25 16:25 ` SZEDER Gábor 0 siblings, 1 reply; 10+ messages in thread From: Lars Schneider @ 2016-04-22 8:10 UTC (permalink / raw) To: Ben Woosley; +Cc: Luke Diamand, Git Users > > > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2 2016-04-22 8:10 ` Lars Schneider @ 2016-04-25 16:25 ` SZEDER Gábor 2016-04-25 17:24 ` Junio C Hamano 0 siblings, 1 reply; 10+ messages in thread From: SZEDER Gábor @ 2016-04-25 16:25 UTC (permalink / raw) To: Lars Schneider; +Cc: SZEDER Gábor, Ben Woosley, Luke Diamand, Git Users > > * 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2 2016-04-25 16:25 ` SZEDER Gábor @ 2016-04-25 17:24 ` Junio C Hamano 2016-04-25 23:10 ` SZEDER Gábor 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2016-04-25 17:24 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Lars Schneider, Ben Woosley, Luke Diamand, Git Users 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2 2016-04-25 17:24 ` Junio C Hamano @ 2016-04-25 23:10 ` SZEDER Gábor 2016-04-28 6:09 ` Lars Schneider 0 siblings, 1 reply; 10+ messages in thread From: SZEDER Gábor @ 2016-04-25 23:10 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, Ben Woosley, Luke Diamand, Git Users 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2 2016-04-25 23:10 ` SZEDER Gábor @ 2016-04-28 6:09 ` Lars Schneider 0 siblings, 0 replies; 10+ messages in thread From: Lars Schneider @ 2016-04-28 6:09 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Junio C Hamano, Ben Woosley, Luke Diamand, Git Users 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. > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-04-28 6:10 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-20 18:28 [PATCH] Update git-p4 to be compatible with git-lfs 1.2 Ben Woosley 2016-04-20 19:00 ` Luke Diamand 2016-04-20 19:13 ` Ben Woosley 2016-04-20 19:36 ` Lars Schneider 2016-04-20 21:10 ` Ben Woosley 2016-04-22 8:10 ` Lars Schneider 2016-04-25 16:25 ` SZEDER Gábor 2016-04-25 17:24 ` Junio C Hamano 2016-04-25 23:10 ` SZEDER Gábor 2016-04-28 6:09 ` Lars Schneider
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).