On Mon, Jul 9, 2018 at 11:10 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Duy Nguyen <pclouds@xxxxxxxxx> writes: > > > On Sun, Jul 8, 2018 at 1:16 AM Kim Gybels <kgybels@xxxxxxxxxxxx> wrote: > >> Should I post a v3 that goes back to the original fix, but uses > >> test_i18ngrep instead of grep? > > > > Yes please. In my comment I did write we didn't need the repo anymore > > (or something along that line) which turns out to be wrong. > > > >> In addition to not breaking any tests, close_all_packs is already used > >> in a similar way in am and fetch just before running "gc --auto". > >> > >> -Kim > > Sound good. > > I recall that "clear repo should treat the_repository special" was > discussed when we saw the patch that became 74373b5f ("repository: > fix free problem with repo_clear(the_repository)", 2018-05-10), > instead of treating only the index portion specially. Perhaps it > was a more correct approach after all? I think it's good that we have a way to "shut down the repo" when we run an external command. But what we lack is "reinitialize the repo" after the external command is done. We could treat the_repository special in this case so shutting down does not require reinitialization. Then repo_clear() should work well here. We could also add a flag in repo_clear() to say "release all the resources you are holding, but keep the repo settings/location..., we're not done with this repo yet" then we don't need to re-initialize the repo afterwards and still don't make the_repository so special. -- Duy