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

From: Shawn O. Pearce <spearce@spearce.org>
Date: May 24, 2007, 07:15 UTC
Message-ID: <20070524071527.GM28023@spearce.org>
In-Reply-To: <81b0412b0705232357i535be2adl6570847942ecb9c0@mail.gmail.com>

Alex Riesen <raa.lkml@gmail.com> wrote:

Show 13 quoted lines

> On 5/24/07, skimo@liacs.nl <skimo@liacs.nl> 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.