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. Cheers. -- 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
