Re: [PATCH 04/21] gc: drop the_repository in log location

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxx>

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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux