On Thu, Sep 09 2021, Ævar Arnfjörð Bjarmason wrote: > On Wed, Sep 08 2021, Taylor Blau wrote: > >> On Thu, Sep 09, 2021 at 02:50:59AM +0200, Ævar Arnfjörð Bjarmason wrote: >>> >>> On Thu, Sep 09 2021, Ævar Arnfjörð Bjarmason wrote: >>> >>> > On Wed, Sep 08 2021, Taylor Blau wrote: >>> > >>> >> Here is a very small reroll of a couple of patches to make sure that packs are >>> >> not read before all of their auxiliary files exist and are moved into place, by >>> >> renaming the .idx file into place last. >>> >> >>> >> New since the original version is a patch to apply similar treatment to >>> >> index-pack, as noticed by Peff in [1]. This should be queued before Ævar's >>> >> series on top. >>> > >>> > I read Junio's earlier <xmqq8s063m7m.fsf@gitster.g>[1] as a suggestion >>> > that we combine the two into a singe series. I do think that yields a >>> > better end-result, in particular your 1/3 is much more readable if the >>> > refactoring in my 2/4. >>> > >>> > I'm mot of the way done with such a rewrite, which also incorporates >>> > your suggestion for how to manage memory in rename_tmp_packfile(), but >>> > I'll hold of on what you & Junio have to say about next steps before >>> > adding to the re-roll competition Junio mentioned... >>> > >>> > 1. https://lore.kernel.org/git/xmqq8s063m7m.fsf@gitster.g >>> >>> I've got that at >>> https://github.com/git/git/compare/master...avar:avar-tb/idx-rename-race-3 >> >> Beautiful :-). >> >> I mentioned in my response to [1] that I missed that message when >> sending v2 of my series to fix a couple of these races. And I was even >> happy to unify our two series, but you did all of the work for me while >> I was eating dinner. Thank you! >> >> I fetched your branch, reviewed, and am happy with the result. So I >> would be content to apply my s-o-b to your patches and resubmit them as >> a unified series. > > Sounds good to me, also in particular any typo/grammar etc. fixes to my > changes are most welcome, I tend to sneak those in, and any code changes > you see fit of course. > >> But I did wonder if you wanted to include [2] in this series as well. >> It's kind of different, but also related enough that it probably makes >> sense to just pull it in. So I'm inclined to just do that, unless you >> have any objections. > > In this latest push-out of "next" Junio's merged that one down already, > see 1972c5931bb (Merge branch 'ab/reverse-midx-optim' into next, > 2021-09-08). > > So I think at this point it could be built on top of that, but given > that the two don't conflict in any way (textually or semantically) it's > probably better to base this larger topic on "master" and proceed with > them independently. > >> [2]: https://lore.kernel.org/git/patch-1.1-366ba928bd-20210908T010743Z-avarab@xxxxxxxxx/ Also: That avar-tb/idx-rename-race-3 has a bug where I didn't update the bulk-checkin.c code accordingly, missed it because I was running a glob of tests that included "*pack*,*rev*", but missed "*large*". I've got a avar-tb/idx-rename-race-4 which fixes it. Sorry about that. https://github.com/git/git/compare/master...avar:avar-tb/idx-rename-race-4
