On Thu, Nov 29, 2012 at 2:16 AM, Max Horn <post...@quendi.de> wrote:
> On 28.11.2012, at 23:23, Felipe Contreras wrote:
>> They have been marked as UNINTERESTING for a reason, lets respect that.
>> Currently the first ref is handled properly, but not the rest:
>>  % git fast-export master ^uninteresting ^foo ^bar
> All these refs are assumed to point to the same object, right? I think it 
> would be better if the commit message stated that explicitly. To make up for 
> the lost space, you could then get rid of one of the four refs, I think three 
> are sufficient to drive the message home ;-).

Yeah, they point to the same object.

> <snip>
>> The reason this happens is that before traversing the commits,
>> fast-export checks if any of the refs point to the same object, and any
>> duplicated ref gets added to a list in order to issue 'reset' commands
>> after the traversing. Unfortunately, it's not even checking if the
>> commit is flagged as UNINTERESTING. The fix of course, is to do
>> precisely that.
> Hm... So this might be me being a stupid n00b (I am not yet that familiar 
> with the internal rep of things in git and all...)... but I found the 
> "precisely that" par very confusing, because right afterwards, you say:

Yeah, the next part was added afterwards.

>> However, in order to do it properly we need to get the UNINTERESTING flag
>> from the command line ref, not from the commit object.
> So this sounds like you are saying "we do *precisely* that, except we don't, 
> because it is more complicated, so we actually don't do this *precisely*, 
> just manner of speaking..."

Well, we do check fro the UNINTERESTING flag, but on the ref, not on the commit.

> Some details here are beyond my knowledge, I am afraid, so I have to resort 
> to guess: In particular it is not clear to me why the "however" part pops up: 
> Reading it makes it sound as if the commit object also carries an 
> UNINTERESTING flag, but we can't use it because of some reason (perhaps it 
> doesn't have the semantics we need?), so we have to look at revs.pending 
> instead. Right? Wrong? Or is it because the commit objects actually do *not* 
> carry the UNINTERESTING bits, hence we need to look at revs.pending. Or is it 
> due to yet another reason?

It's actually revs.cmdline, I typed the wrong one.

If you have two refs pointing to the same object, and you do 'one
^two', the object (e.g. 8c7a786) will get the UNINTERESTING flag, but
that doesn't tell us anything about the ref being a positive or a
negative one, and revs.pending only has the object flags. On the other
hand revs.cmdline does have the flags for the refs.

Does that explain it?

> Anyway, other than these nitpicky questions, this whole thing looks very 
> logical to me, description and code alike. I also played around with tons of 
> "fast-export" invocations, with and without this patch, and it seems to do 
> what the description says. Finally, I went to the various long threads 
> discussion prior versions of this patch, in particular those starting at
>   http://thread.gmane.org/gmane.comp.version-control.git/208725
> and
>   http://thread.gmane.org/gmane.comp.version-control.git/209355/focus=209370
> These contained some concerns. Sadly, several of those discussions ultimately 
> degenerated into not-so-pleasant exchanges :-(, and my impression is that as 
> a result some people are not so inclined to comment on these patches anymore 
> at all. Which is a pity :-(. But overall, it seems this patch makes nothing 
> worse, but fixes some things; and it is simple enough that it shouldn't make 
> future improvements harder.
> So *I* at least am quite happy with this, it helps me! My impression is that 
> Felipe's latest patch addresses most concerns people raised by means of an 
> improved description. I couldn't find any in those threads that I feel still 
> applies -- but of course those people should speak for themselves, I am 
> simply afraid they don't want to be part of this anymore :-(.

Indeed. For all the concerns given I made a response to how that
either is not true, or doesn't really matter, and in the case of the
latter, I asked for examples where it would matter, only to receive
nothing. For whatever reason involved people are not responding, not a
single valid concern has been raised and remained.

So I think it's good.


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

Reply via email to