On Fri, 28 Sep 2007, Shawn O. Pearce wrote: > My understanding of the old code was that it should do what Junio > was reporting as broken: Agreed; that's why I thought there must be something wrong with it. > - when i == 0 this is the first remote.$foo.fetch; > - when has_merge == 0 we have no branch.$name.merge; > - when ref_map != NULL we have at least one ref from this fetch spec; > - when fetch[0].src == ref_map->name it wasn't a wildcard spec; > > The if conditional above was ordered the way it is so we can skip > the more expensive operation (the strcmp) most of them time through > the loop iteration. > > The only way I can see the old code was failing was if ref_map > as returned by get_fetch_map() pointed to more than one refspec. > But for that to be true then fetch[1].src must have been a wildcard, > in which case the strcmp() would fail. So we should only ever > get one entry, it should be the first entry, and dammit it should > have matched. That is the analysis that the original code is based on, yes. But it's not the easiest thing to follow, and I misunderstood it earlier this evening (prompted by the claim that it wasn't working, mostly). It's probably worth checking remote->fetch[0].pattern instead of the strcmp, anyway, since that's clearer and cheaper anyway. > How/why are we getting cases where fetch[0].src isn't in the first > entry in ref_map? What are those other entries? Are they possibly > going to also match fetch[0].src and cause more than one branch > to merge? Beats me; Junio, what's your test case? > BTW, thanks for looking at this. I didn't have time to get to it > this week and now I'm really unlikely to be able to do so until > after I get back from San Jose. I have too many things crammed > into this next week. :-\ No problem. You fixed plenty of my other bugs in this code, and it's getting so close to done. -Daniel *This .sig left intentionally blank* - 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
