Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Oct 30, 2012, 22:18 UTC
Message-ID: <CAMP44s2KNmr7zAvFo2gOR8G=YaoBWiGPCjPY47x00eev6MOAFw@mail.gmail.com>
In-Reply-To: <CAGdFq_h3L-1rPvb=dSYeXqEea+f+g2kRHp7aAjaU-AxjZHB7dQ@mail.gmail.com>

On Tue, Oct 30, 2012 at 10:59 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:

Show 16 quoted lines

> On Tue, Oct 30, 2012 at 2:35 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Tue, Oct 30, 2012 at 10:17 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
>>> On Tue, Oct 30, 2012 at 11:47 AM, Felipe Contreras
>>> <felipe.contreras@gmail.com> wrote:
>>>> Why would it? We are not changing the way objects are exported, the
>>>> only difference is what happens at the end
>>>> (handle_tags_and_duplicates()).
>>>
>>> Because the marking is per-commit, not per-ref, right?
>>
>> Oh, you meant using marks?
>
> No, I meant the 'SHOWN' flag, doesn't it get added per commit, not per
> ref? That is, commit->object.flags & SHOWN refers to the object
> underlying the ref. So I suspect this scenario doesn't pass the tests:

Without marks you cannot have the SHOWN mark at that point; we haven't traversed the commits.

Show 16 quoted lines

> git init &&
> echo first > content &&
> git add content &&
> git commit -m "first" &&
> git branch first &&
> echo two > content &&
> git commit -m "second" &&
> git branch second &&
> git fast-export first > actual &&
> test_cmp actual expected_first &&
> git fast-export second > actual &&
> test_cmp actual expected_second
>
> With expected_first being something like:
> <fast-export stream with the first commit>
> <reset command to set first to the right commit>

Why would a 'reset' command be expected if the 'first' branch is already pointing to the 'first' commit?

> And expected_second being something like
> <fast export stream with the first and second command>
> <reset command to set first and second to their respective branches>

Ditto, plus, why would 'git fast-export second' do anything regarding 'first'? It wasn't specified in the committish; it's not relevant.

Before an after my patch the output is the same:

% git fast-export first: reset refs/heads/first commit refs/heads/first

% git fast-export second: reset refs/heads/second commit refs/heads/second commit refs/heads/second

Which is expected and correct; the branch already points to the right commit, no need for an extra reset.

Cheers.

-- 
Felipe Contreras