Re: [PUB]corrupt repos does not return error with `git fsck`

[Jeff King <peff@peff.net>]

On Wed, May 20, 2015 at 01:39:36PM -0700, Junio C Hamano wrote:

> Yeah, "bad object" sounds as if we tried to parse something that
> exists and it was corrupt.  So classifying "a file or a pack index
> entry exists where a valid object with that name should reside in"
> as "bad object" and "there is no such file or a pack index entry
> that would house the named object" as "missing object" _might_ make
> things better.
> 
> But let's think about it a bit more.  Would it have prevented the
> original confusion if we said "missing object"?  I have a feeling
> that it wouldn't have.  Faheem was so convinced that the object
> named with the 40-hex *must* exist in the cloned repository, and if
> we told "missing object" to such a person, it will just enforce the
> (mis)conception that the repository is somehow corrupt, when it is
> not.
> 
> So...

I dunno. If it were phrased not as "missing object" but as "there is no
such object in the repository", then it seems more clear to me that the
error is in the request, not in the repository (and hopefully the user
would examine their assumption that it should be).

But "bad object" is just a horrible error message. It actively implies
corruption. And I think if we do have corruption, then parse_object()
already reports it. For example:

  # helpers
  objfile() {
    printf '.git/objects/%s' $(echo $1 | sed 's,..,&/,')
  }
  blob=$(echo content | git hash-object -w --stdin)

  # object with a sha1 mismatch
  mismatch=1234567890123456789012345678901234567890
  mkdir .git/objects/12
  cp $(objfile $blob) $(objfile $mismatch)

  # plain old missing object
  missing=1234abcdef1234abcdef1234abcdef1234abcdef

  # object with data corruption
  corrupt=$blob
  chmod +w $(objfile $corrupt)
  dd if=/dev/zero of=$(objfile $corrupt) bs=1 count=1 conv=notrunc seek=10

  # now show each
  for bad in corrupt mismatch missing; do
    echo "==> $bad"
    git --no-pager show $(eval "echo \$$bad")
  done

produces:

  ==> corrupt
  error: inflate: data stream error (invalid distance too far back)
  error: unable to unpack d95f3ad14dee633a758d2e331151e950dd13e4ed header
  error: inflate: data stream error (invalid distance too far back)
  fatal: loose object d95f3ad14dee633a758d2e331151e950dd13e4ed (stored in .git/objects/d9/5f3ad14dee633a758d2e331151e950dd13e4ed) is corrupt
  ==> mismatch
  error: sha1 mismatch 1234567890123456789012345678901234567890
  fatal: bad object 1234567890123456789012345678901234567890
  ==> missing
  fatal: bad object 1234abcdef1234abcdef1234abcdef1234abcdef

Note that the "missing" case is the only one that _doesn't_ give further
clarification, and it is likely to be the most common (however just
changing "bad object" to "no such object" would be a bad idea, as it
makes the second case harder to understand).

-Peff