Re: [PATCH 04/21] gc: drop the_repository in log location
From: Jonathan Tan <hidden>
Date: 2020-07-09 02:23:05
From: Derrick Stolee <redacted> The report_last_gc_error() method use git_pathdup() which implicitly uses the_repository. Replace this with strbuf_repo_path() to get a path buffer we control that uses a given repository pointer. Signed-off-by: Derrick Stolee <redacted>
Regarding the first 4 patches, it would be great if there was a test like the one in test_repository.c (together with a code coverage report, perhaps) that verifies that all code paths do not use the_repository. But set aside that test for now - I don't think gc.c fully supports arbitrary repositories. In particular, when running a subprocess, it inherits the environment from the current process. I see that future patches try to resolve this by passing "-C", but that does not work if environment variables like GIT_DIR are set (because the environment variables override the "-C"). Perhaps we need a function that runs a subprocess in a specific repository. I ran into the same problem when attempting to make fetch-pack (which runs index-pack as a subprocess) support arbitrary repositories, but I haven't looked deeply into resolving this yet (and I haven't looked at that problem in a while). Having said that, I'm fine with these patches being in the set - the only negative is that perhaps a reader would be misled into thinking that GC supports arbitrary repositories.