Re: [BUG] t9801 and t9803 broken on next

From: Lars Schneider <larsxschneider@gmail.com>
Date: May 17, 2016, 08:07 UTC
Message-ID: <5E7631C9-DD59-4358-B907-D7C7AEA1739C@gmail.com>
In-Reply-To: <xmqq37pk8q0h.fsf@gitster.mtv.corp.google.com>

Show 35 quoted lines

> On 14 May 2016, at 20:15, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
>>> On 13 May 2016, at 18:37, Junio C Hamano <gitster@pobox.com> wrote:
>>> 
>>> Are you saying that "git p4" itself breaks unless fast-import always
>>> writes a new (tiny) packfile?  That sounds quite broken, and setting
>>> unpacklimit to 0 does not sound like a sensible "fix".  Of course,
>>> if the test script is somehow looking at the number of packs or
>>> loose objects and declaring a failure, even when the resulting
>>> history in p4 and git are correct, then that is a different issue,
>>> and forcing to explode a tiny pack is a reasonable workaround.  I
>>> couldn't quite tell which the case is.
>>> 
>>> Puzzled.  Please help.
>> 
>> t9801 "import depot, branch detection" is the first test that fails
>> with a fast import error:
>> https://github.com/git/git/blob/78b384c29366e199741393e56030a8384110760d/t/t9801-git-p4-branch.sh#L110
>> 
>> fast-import crash report:
>>    fast-import process: 77079
>>    parent process     : 77077
>>    at 2016-05-14 07:48:40 +0000
>> 
>> fatal: offset beyond end of packfile (truncated pack?)
> 
> Hmm, does that suggest Eric's "explode them into loose instead of
> keeping a small pack" insufficient?  It sounds like that somebody
> wanted to read some data back from its packfile without knowing that
> the updated code may make it available in a loose object form, which
> would mean that somebody needs to be told about what is going on,
> namely, these objects are not in a half-written pack but are found
> as loose objects.

I think that is pretty much the problem. Here is what is happening:

1.  git-p4 imports all changelists for the "main" branch

2.  git-p4 starts to import a second branch and creates a fast-import
    "progress checkpoint". This triggers:

    --> fast-import.c: cycle_packfile
    --> fast-import.c: end_packfile
    --> fast-import.c: loosen_small_pack

    At this point we have no packfile anymore.

3.  git-p4 sends the command "commit refs/remotes/p4/depot/branch2"
    to fast-import in order to create a new branch. This triggers:

    --> fast-import.c: parse_new_commit
    --> fast-import.c: load_branch
    --> fast-import.c: load_tree

    "load_tree" calls "find_object" and the result has a "pack_id" of 0.
    I think this indicates that the object is in the packfile. Shouldn't
    the "pack_id" be "MAX_PACK_ID" instead?

        myoe = find_object(sha1);
        if (myoe && myoe->pack_id != MAX_PACK_ID) {

    --> fast-import.c: gfi_unpack_entry

    In "gfi_unpack_entry" this condition evaluates to "true":

        struct packed_git *p = all_packs[oe->pack_id];
        if (p == pack_data && p->pack_size < (pack_size + 20))

    In the comment below Git thinks that the object is stored in the
    packfile. This seems wrong but the flow continues:

    --> sha1_filec: unpack_entry
    --> sha1_filec: unpack_object_header
    --> sha1_filec: use_pack

    At this point fast-import dies with "offset beyond end of packfile 
    (truncated pack?)".

I am no expert in "fast-import". Does anyone have a few hints how to proceed?

Thanks, Lars