Jacob Keller <jacob.keller@xxxxxxxxx> writes:
> So, I think this direction is good. I imagine a full patch would
> include also dropping the specialized helper function that is no
> longer needed, and possibly adding new tests for the behavior of
> GIT_CONFIG_COUNT?
Yeah, coding that is the easiest part. Thinking through
ramifications of making (or not making) such a change is much
harder.
I said "assuming" number of times, because I am not so sure if the
subprocesses spawned from other codepaths do or do not want to see
the one-shot custom configuration settings. If that assumption
turns out to be wrong and the processes spawned using the helper in
various helper functions in submodule.c are the oddball cases that
want to see the custom configuration, then such a change would break
existing users.
I _think_ the one in connect.c, which runs either the ssh transport
(for which the processes that run on the other side in the other
repository won't be affected by our environment anyway) or the file
transport that runs another process and talks with it over a pipe is
probably OK if the configuration on the "client" side leaks through
to the "server" side, e.g.
$ git -c advice.ignoredHook=false clone file:///the/repo.git/ here
would probably want the other end (i.e. the one that runs upload-pack
in /the/repo.git/ directory) to see the one-shot configuration, too.
I do not think it makes much difference to the use of local_repo_env
in object-file.c::for_each_alternate_ref() either way; it could be
used (via core.alternateRefsCommand) an arbitrary command in each
alternate repository, but by default it runs for-each-ref in them,
and I do not think of any configuration variables that would be
useful on "the other side".
And I suspect that trailers.c::apply_command() excludes these
environment variables just out of habit without much deep thinking.
It is not going in a different repository to run the command, and
santitizing the environment that pertains to this repository should
not have any meaningful effect [*].
So, I would not be surprised if it were a totally safe change, but I
am not yet sure.
Thanks.
[Footnote]
* Christian CC'ed for 85039fb6 (trailer: execute command from
'trailer.<name>.command', 2014-10-13) that introduced the code
together with its use of local_repo_env[].
