Subject: Re: [PATCH 10/22] run-command: optionally clear git environment

From: "Shawn O. Pearce" <[email-masked]>
Date: Thu, 24 May 2007 03:15:27 -0400

Alex Riesen <[email-masked]> wrote:
> On 5/24/07, [email-masked] <[email-masked]> wrote:
> >+               if (cmd->clear_git_env) {
> >+                       unsetenv(ALTERNATE_DB_ENVIRONMENT);
> >+                       unsetenv(DB_ENVIRONMENT);
> >+                       unsetenv(CONFIG_ENVIRONMENT);
> >+                       unsetenv(GIT_DIR_ENVIRONMENT);
> >+                       unsetenv(GRAFT_ENVIRONMENT);
> >+                       unsetenv(INDEX_ENVIRONMENT);
> >+               }
> 
> You might want to try the alternative approach from the recently
> proposed patches to do the same, but more generic. Would
> be less code, too.

Unfortunately Alex's approach means the caller must know the list of
"special Git envvars" that should be cleared when entering into a
subproject Git repository to execute a command.  That's horrible code
duplication in the callers of run_command, and is just asking for
trouble later when/if another magic environment variable is added.

As long as the above unsetenv list is, I'd really rather have a
specific clear_git_env bit in struct child_process, just so that the
callers don't have to be bothered with the precise list of names.
Of course declaring those names in a static const char** and
looping over it before doing Alex's env array thing would probably
be less code and let the two play along together rather nicely.

-- 
Shawn.