# 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch 9 messages from 2011-09-28 to 2011-10-01. Participants: Carlos Martín Nieto, Junio C Hamano, Jeff King, Jakub Narebski, Philip Oakley. Thread: https://gitlist.dev/t/28522 ## Carlos Martín Nieto, 2011-09-28 16:04 Subject: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch Message-ID: <1317225869.30267.18.camel@bee.lab.cmartin.tk> URL: https://gitlist.dev/e/1317225869.30267.18.camel%40bee.lab.cmartin.tk ``` Hello, Whilst trying to do some work related to fetch, I came across a regression in the 'next' branch. Bisecting gave me this commit as breaking point (and I tried with the parent and there it worked). When doing 'git fetch', rev-list will complain about usage, and fetch will say that we didn't receive enough, even though earlier versions of git have no problems. This fails both on github and on git.or.cz and for git and http transports: $ ./git-fetch git://repo.or.cz/git usage: git rev-list [OPTION] ... [ -- paths... ] limiting output: --max-count= --max-age= --min-age= --sparse --no-merges --min-parents= --no-min-parents --max-parents= --no-max-parents --remove-empty --all --branches --tags --remotes --stdin --quiet ordering output: --topo-order --date-order --reverse formatting output: --parents --children --objects | --objects-edge --unpacked --header | --pretty --abbrev= | --no-abbrev --abbrev-commit --left-right special purpose: --bisect --bisect-vars --bisect-all error: git://repo.or.cz/git did not send all necessary objects ``` ## Junio C Hamano, 2011-09-28 18:50 Subject: Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch Message-ID: <7vsjngzelc.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsjngzelc.fsf%40alter.siamese.dyndns.org In-Reply-To: <1317225869.30267.18.camel@bee.lab.cmartin.tk> ``` Carlos Martín Nieto writes: > Whilst trying to do some work related to fetch, I came across a > regression in the 'next' branch.... > > $ ./git-fetch git://repo.or.cz/git That invocation of ./git-fetch looks suspicious. Are you sure that it internally invokes ./git-rev-list from the same version that knows --verify-objects option (you just built in your current directory), or is it invoking an old git-rev-list that is installed and is reachable from your usual $PATH, which does not know that option yet? When I try a new version that was just built in my current directory, here is an incantation I use: GIT_EXEC_PATH=`pwd` PATH=`pwd`:/usr/bin:/bin GITPERLLIB=`pwd`/perl/blib/lib export GIT_EXEC_PATH PATH GITPERLLIB ``` ## Jeff King, 2011-09-28 18:53 Subject: Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch Message-ID: <20110928185327.GB1482@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110928185327.GB1482%40sigill.intra.peff.net In-Reply-To: <1317225869.30267.18.camel@bee.lab.cmartin.tk> ``` On Wed, Sep 28, 2011 at 06:04:27PM +0200, Carlos Martín Nieto wrote: > Whilst trying to do some work related to fetch, I came across a > regression in the 'next' branch. Bisecting gave me this commit as > breaking point (and I tried with the parent and there it worked). When > doing 'git fetch', rev-list will complain about usage, and fetch will > say that we didn't receive enough, even though earlier versions of git > have no problems. This fails both on github and on git.or.cz and for git > and http transports: > > $ ./git-fetch git://repo.or.cz/git > usage: git rev-list [OPTION] ... [ -- paths... ] Hmm. I notice you're running a not-installed version of fetch. Might this be a problem with a new git fetch running an older, installed version of rev-list? The commit you mention calls rev-list with --verify-objects, but that feature is only added in the parent commit. So I can reproduce your issue with: $ git checkout 6d4bb38~2 ;# or anything before --verify-objects $ make install $ git checkout 6d4bb38 $ make $ ./git-fetch git://repo.or.cz/git but this works (because it sets the exec path properly): $ ./bin-wrappers/git fetch git://repo.or.cz/git as does this: $ make install $ ./git-fetch git://repo.or.cz/git So I don't think there's a bug. It's just that running compiled programs straight out of the build directory isn't supported. It works _most_ of the time, but as you can see, you may end up calling older, installed versions of git. The bin-wrappers scripts set up the exec path properly to let you test. -Peff ``` ## Jakub Narebski, 2011-09-28 19:12 Subject: Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch Message-ID: URL: https://gitlist.dev/e/m3y5x8o527.fsf%40localhost.localdomain In-Reply-To: <1317225869.30267.18.camel@bee.lab.cmartin.tk> ``` Carlos Martín Nieto writes: > Hello, > > Whilst trying to do some work related to fetch, I came across a > regression in the 'next' branch. Bisecting gave me this commit as > breaking point (and I tried with the parent and there it worked). When > doing 'git fetch', rev-list will complain about usage, and fetch will > say that we didn't receive enough, even though earlier versions of git > have no problems. This fails both on github and on git.or.cz and for git > and http transports: > > $ ./git-fetch git://repo.or.cz/git Have you tried $ ./git fetch git://repo.or.cz/git ? -- Jakub Narębski ``` ## Carlos Martín Nieto, 2011-09-30 23:48 Subject: Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch Message-ID: <1317426491.4331.23.camel@centaur.lab.cmartin.tk> URL: https://gitlist.dev/e/1317426491.4331.23.camel%40centaur.lab.cmartin.tk In-Reply-To: <20110928185327.GB1482@sigill.intra.peff.net> ``` On Wed, 2011-09-28 at 14:53 -0400, Jeff King wrote: > On Wed, Sep 28, 2011 at 06:04:27PM +0200, Carlos Martín Nieto wrote: > > > Whilst trying to do some work related to fetch, I came across a > > regression in the 'next' branch. Bisecting gave me this commit as > > breaking point (and I tried with the parent and there it worked). When > > doing 'git fetch', rev-list will complain about usage, and fetch will > > say that we didn't receive enough, even though earlier versions of git > > have no problems. This fails both on github and on git.or.cz and for git > > and http transports: > > > > $ ./git-fetch git://repo.or.cz/git > > usage: git rev-list [OPTION] ... [ -- paths... ] > > Hmm. I notice you're running a not-installed version of fetch. Might > this be a problem with a new git fetch running an older, installed > version of rev-list? Yes, this seems indeed to be the case. > > The commit you mention calls rev-list with --verify-objects, but that > feature is only added in the parent commit. So I can reproduce your > issue with: > > $ git checkout 6d4bb38~2 ;# or anything before --verify-objects > $ make install > $ git checkout 6d4bb38 > $ make > $ ./git-fetch git://repo.or.cz/git > > but this works (because it sets the exec path properly): > > $ ./bin-wrappers/git fetch git://repo.or.cz/git > > as does this: > > $ make install > $ ./git-fetch git://repo.or.cz/git > > So I don't think there's a bug. It's just that running compiled programs > straight out of the build directory isn't supported. It works _most_ of > the time, but as you can see, you may end up calling older, installed > versions of git. The bin-wrappers scripts set up the exec path properly > to let you test. Indeed, as both you and Junio pointed out (within three minutes of each other :) I was running git from the build directory and expected it to work, as I was testing a few changed I had made to the fetch code. Mea culpa, I tend to forget that git tends to behave like a bunch of shell scripts that happen to be written in C. Thanks to both. cmn ``` ## Carlos Martín Nieto, 2011-09-30 23:54 Subject: Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch Message-ID: <1317426849.4331.29.camel@centaur.lab.cmartin.tk> URL: https://gitlist.dev/e/1317426849.4331.29.camel%40centaur.lab.cmartin.tk In-Reply-To: ``` On Wed, 2011-09-28 at 12:12 -0700, Jakub Narebski wrote: > Carlos Martín Nieto writes: > > > Hello, > > > > Whilst trying to do some work related to fetch, I came across a > > regression in the 'next' branch. Bisecting gave me this commit as > > breaking point (and I tried with the parent and there it worked). When > > doing 'git fetch', rev-list will complain about usage, and fetch will > > say that we didn't receive enough, even though earlier versions of git > > have no problems. This fails both on github and on git.or.cz and for git > > and http transports: > > > > $ ./git-fetch git://repo.or.cz/git > > Have you tried > > $ ./git fetch git://repo.or.cz/git But this would execute /usr/local/libexec/git-fetch, wouldn't it? That is precisely what I don't want to execute, because I changed some code in builtin/fetch.c that I want to test. But yes, the problem was that the local git-fetch was trying to pass an option to rev-list that my older installed binary didn't understand. In this particular case I don't want to run the older git-fetch, but otherwise, that would work. I guess I'll have to either properly install git from 'next' or base my changed on 'maint' cmn ``` ## Jeff King, 2011-10-01 06:03 Subject: Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch Message-ID: <20111001060353.GA25228@sigill.intra.peff.net> URL: https://gitlist.dev/e/20111001060353.GA25228%40sigill.intra.peff.net In-Reply-To: <1317426849.4331.29.camel@centaur.lab.cmartin.tk> ``` On Sat, Oct 01, 2011 at 01:54:08AM +0200, Carlos Martín Nieto wrote: > > Have you tried > > > > $ ./git fetch git://repo.or.cz/git > > But this would execute /usr/local/libexec/git-fetch, wouldn't it? That > is precisely what I don't want to execute, because I changed some code > in builtin/fetch.c that I want to test. No, but only because fetch is a builtin. However, it still doesn't set up exec_path correctly, so your rev-list problem would not go away. > I guess I'll have to either properly install git from 'next' or base my > changed on 'maint' Just use bin-wrappers/git. That's exactly what it's there for (and it's what the test scripts use to make sure we are testing what is compiled). Your change isn't the problem; only your testing method. -Peff ``` ## Philip Oakley, 2011-10-01 10:38 Subject: Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch Message-ID: URL: https://gitlist.dev/e/BED2B875AEDF4E52843F1C9211A8D2A1%40PhilipOakley In-Reply-To: <20111001060353.GA25228@sigill.intra.peff.net> ``` From: "Jeff King" > On Sat, Oct 01, 2011 at 01:54:08AM +0200, Carlos Martín Nieto wrote: > >> > Have you tried >> > >> > $ ./git fetch git://repo.or.cz/git >> >> But this would execute /usr/local/libexec/git-fetch, wouldn't it? That >> is precisely what I don't want to execute, because I changed some code >> in builtin/fetch.c that I want to test. > > No, but only because fetch is a builtin. However, it still doesn't set > up exec_path correctly, so your rev-list problem would not go away. > >> I guess I'll have to either properly install git from 'next' or base my >> changed on 'maint' > > Just use bin-wrappers/git. That's exactly what it's there for (and it's > what the test scripts use to make sure we are testing what is compiled). > > Your change isn't the problem; only your testing method. > > -Peff Peff, Is there a write up of the the git testing method and how to use bin-wrappers etc. I didn't see anything in the Documentation, but I may not have looked carefully enough Philip ``` ## Jeff King, 2011-10-01 10:54 Subject: Re: 6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch Message-ID: <20111001105405.GA4396@sigill.intra.peff.net> URL: https://gitlist.dev/e/20111001105405.GA4396%40sigill.intra.peff.net In-Reply-To: ``` On Sat, Oct 01, 2011 at 11:38:08AM +0100, Philip Oakley wrote: > Is there a write up of the the git testing method and how to use > bin-wrappers etc. I didn't see anything in the Documentation, but I > may not have looked carefully enough Bin-wrappers (and the alternative, which is setting up the exec-path yourself) are mentioned briefly in INSTALL. Running tests is described in t/README. Other than that, you're left on your own to read the code and the commit messages. :) -Peff ```