Re: [PATCH 04/21] gc: drop the_repository in log location
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Jul 9, 2020, 11:13 UTC
- Message-ID
- <4170de4d-c77e-e8a9-1df4-e258e5785360@gmail.com>
- In-Reply-To
- <20200709022257.36743-1-jonathantanmy@google.com>
On 7/8/2020 10:22 PM, Jonathan Tan wrote:
Show 26 quoted lines
>> From: Derrick Stolee <dstolee@microsoft.com> >> >> 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 <dstolee@microsoft.com> > > 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.
I agree. I hope that I do not give the impression that GC is now safe for arbitrary repositories. I only thought that this was prudent to do before I start taking new dependencies on the code.
It' probably time for someone to do a round of the_repository cleanups again, and perhaps the RC window is a good time to think about that (for submission after the release).
Thanks, -Stolee