From: Junio C Hamano Date: Fri, 07 Oct 2005 18:08:17 GMT Subject: Re: [PATCH] Add support for parallel HTTP transfers Message-ID: <7vhdbt42um.fsf@assigned-by-dhcp.cox.net> In-Reply-To: Daniel Barkalow writes: >> The check was added in 029f6de377c7e0484f5c4cf070934599580f1784 >> because back then calling fetch() on an object that we already >> had had a funny interaction with what http-fetch.c did. I >> suspect that Nick's curl-multi changes made it unnecessary, but >> you should double check for other transports. > > Hmm; my intended convention was that fetch() would always be called if > prefetch() was called, even if something had happened to make it appear in > between (e.g., prefetch() causing it to be fetched or a different call to > fetch() speculatively also getting it). When I re-read the code, I think that check was probably a wrong fix to begin with. The original problem sequence, when the http-fetch was still synchronous, was this: (1) we ask for an object, fetch_object() did not find one and fetch_pack() got a pack that contained the object and installed it; the pack is removed from the "yet to be downloaded from this repository" list. (2) we ask for another object, fetch_object() did not find one and fetch_pack() was asked to see if there is a pack we have not downloaded that contained the object -- the pack downloaded in step (1) did not count, and this request failed. Overall fetch() said "Nope, I cannot get it", when it already had one. We should remove that check as you suggested, and fix fetch() implementation in http-fetch.c to notice the above situation, perhaps?