Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Oct 31, 2012 at 1:57 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Felipe Contreras wrote:
>
>> They have been marked as UNINTERESTING for a reason, lets respect that.
>
> So, the above description conveyed zero information, as you mentioned.

I meant, this, of course:
>> They have been marked as UNINTERESTING for a reason, lets respect that.
>
> This patch looks unsafe,

Which you know, because you received that message without the mistake.

> A clearer explanation would be the following:
>
>         fast-export: don't emit "reset" command for negative refs

What is a negative ref?

>         When "git fast-export" encounters two refs on the commandline

commandline?

Only two refs? How about four?

>         referring to the same commit, it exports the first during the usual
>         commit walk and the second using a "reset" command in a final pass
>         over extra_refs:

That is not exactly true: (next^{commit}).

>                 $ git fast-export master next
>                 reset refs/heads/master
>                 commit refs/heads/master
>                 mark :1
>                 author Jonathan Nieder <jrnieder@xxxxxxxxx> 1351644412 -0700
>                 committer Jonathan Nieder <jrnieder@xxxxxxxxx> 1351644412 -0700
>                 data 17
>                 My first commit!
>
>                 reset refs/heads/next
>                 from :1

I don't think this example is good. Where does it say that 'next'
points to master? Using 'points-to-master' or a 'git branch stable
master' and using 'master stable'.

Even simpler would be to use 'git fast-export master master'; it would
show the same behavior.

>         Unfortunately the code to do this doesn't distinguish between positive
>         and negative refs, producing confusing results:
>
>                 $ git fast-export ^master next
>                 reset refs/heads/next
>                 from :0
>
>                 $ git fast-export master ^next
>                 reset refs/heads/next
>                 from :0
>
>         Use revs->cmdline instead of revs->pending to iterate over the rev-list
>         arguments, checking the UNINTERESTING flag bit to distinguish between
>         positive (master, --all, etc) and negative (next.., --not --all, etc)
>         revs and avoid enqueueing negative revs in extra_revs.

Use what? You mean, "To solve the problem, lets use".

But this is not correct, cmdline is not being used. Have you even
looked at the patch?

>         This does not affect revs that were excluded from the revision walk
>         because pointed to by a mark, since those use the SHOWN bit on the
>         commit object itself and not UNINTERESTING on the rev_cmdline_entry.

revs? You mean commits?

"excluded because point to by a mark"? Doesn't sound like proper
grammar. Maybe "excluded because they were pointed to by a mark".

And I don't see why this paragraph is needed at all. Why would the
reader think marks have anything to do with this? There's no mention
of marks before.

This might help you, or other people involved in the problem, but not
anybody else. Anything related to marks is completely orthogonal to
this patch, and there's no point in mentioning that.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]