6d4bb3833c3d2114d (fetch: verify we have everything we need before updating our ref) breaks fetch

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] <commit-id>... [ -- paths... ]
  limiting output:
    --max-count=<n>
    --max-age=<epoch>
    --min-age=<epoch>
    --sparse
    --no-merges
    --min-parents=<n>
    --no-min-parents
    --max-parents=<n>
    --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=<n> | --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

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] <commit-id>... [ -- 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

Carlos Martín Nieto [off-list ref] 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

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:

quoted

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] <commit-id>... [ -- 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

On Wed, 2011-09-28 at 12:12 -0700, Jakub Narebski wrote:

Carlos Martín Nieto [off-list ref] writes:

quoted

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

On Sat, Oct 01, 2011 at 01:54:08AM +0200, Carlos Martín Nieto wrote:

quoted

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

From: "Jeff King" <redacted>

On Sat, Oct 01, 2011 at 01:54:08AM +0200, Carlos Martín Nieto wrote:

quoted

quoted

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.

quoted

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 

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