All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Alex Riesen <raa.lkml@gmail.com>
Cc: "skimo@liacs.nl" <skimo@liacs.nl>,
	git@vger.kernel.org, Junio C Hamano <junkio@cox.net>,
	Martin Waitz <tali@admingilde.org>
Subject: Re: [PATCH 10/22] run-command: optionally clear git environment
Date: Thu, 24 May 2007 03:15:27 -0400	[thread overview]
Message-ID: <20070524071527.GM28023@spearce.org> (raw)
In-Reply-To: <81b0412b0705232357i535be2adl6570847942ecb9c0@mail.gmail.com>

Alex Riesen <raa.lkml@gmail.com> wrote:
> 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.

  reply	other threads:[~2007-05-24  7:16 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-23 22:22 [RFC] Fourth round of support for cloning submodules skimo
2007-05-23 22:22 ` [PATCH 01/22] git_connect: unset CONFIG_ENVIRONMENT in child skimo
2007-05-23 22:22 ` [PATCH 02/22] Add dump-config skimo
2007-05-23 22:22 ` [PATCH 03/22] git-config: add --remote option for reading config from remote repo skimo
2007-05-23 22:22 ` [PATCH 04/22] http.h: make fill_active_slots a function pointer skimo
2007-05-23 22:22 ` [PATCH 05/22] git-config: read remote config files over HTTP skimo
2007-05-23 22:22 ` [PATCH 06/22] unpack-trees.c: pass cache_entry * to verify_absent rather than just the name skimo
2007-05-23 22:22 ` [PATCH 07/22] git-read-tree: take --submodules option skimo
2007-05-23 22:22 ` [PATCH 08/22] unpack-trees.c: assume submodules are clean skimo
2007-05-23 22:22 ` [PATCH 09/22] Add run_command_v_opt_cd: chdir into a directory before exec skimo
2007-05-23 22:22 ` [PATCH 10/22] run-command: optionally clear git environment skimo
2007-05-24  6:57   ` Alex Riesen
2007-05-24  7:15     ` Shawn O. Pearce [this message]
2007-05-24  7:19       ` Alex Riesen
2007-05-23 22:23 ` [PATCH 11/22] entry.c: optionally checkout submodules skimo
2007-05-24  6:59   ` Alex Riesen
2007-05-24  7:18     ` Shawn O. Pearce
2007-05-24  7:27       ` Sven Verdoolaege
2007-05-24  7:29       ` Alex Riesen
2007-05-24 16:21       ` Martin Waitz
2007-05-25  0:49         ` Shawn O. Pearce
2007-05-23 22:23 ` [PATCH 12/22] git-checkout: pass --submodules option to git-read-tree skimo
2007-05-23 22:23 ` [PATCH 13/22] git-read-tree: treat null commit as empty tree skimo
2007-05-23 22:23 ` [PATCH 14/22] git_config: add void * for callback data skimo
2007-05-23 22:23 ` [PATCH 15/22] make redirecting stdout to /dev/null available via run_command_v_opt skimo
2007-05-23 22:23 ` [PATCH 16/22] unpack-trees.c: optionally clone submodules for later checkout skimo
2007-05-23 22:23 ` [PATCH 17/22] entry.c: optionally checkout newly cloned submodules skimo
2007-05-24 13:28   ` Johannes Sixt
2007-05-23 22:23 ` [PATCH 18/22] git-clone: add --submodules for cloning submodules skimo
2007-05-23 22:23 ` [PATCH 19/22] test for simple submodule checkout support skimo
2007-05-23 22:23 ` [PATCH 20/22] checkout_submodule: checkout submodule on forced checkout of submodule dir skimo
2007-05-23 22:23 ` [PATCH 21/22] run-command: optionally redirect stderr to /dev/null skimo
2007-05-23 22:23 ` [PATCH 22/22] ensure_submodule: fetch missing revisions skimo
2007-05-23 23:40 ` [RFC] Fourth round of support for cloning submodules Johannes Schindelin
2007-05-24  0:50   ` Junio C Hamano
2007-05-24  7:22     ` Sven Verdoolaege
2007-05-24  7:29       ` Shawn O. Pearce
2007-05-24  7:36         ` Sven Verdoolaege
2007-05-24  9:41           ` Johannes Schindelin
2007-05-24 10:51             ` Sven Verdoolaege
2007-05-24 11:02               ` Johannes Schindelin
2007-05-24 11:16                 ` Sven Verdoolaege
2007-05-24 11:31                   ` Johannes Schindelin
2007-05-24 11:43                     ` Sven Verdoolaege
2007-05-24 12:16                       ` Johannes Schindelin
2007-05-24 12:23                         ` Johannes Sixt
2007-05-24 13:14                           ` Johannes Schindelin
2007-05-24 12:39                         ` Sven Verdoolaege
2007-05-24 13:17                           ` Johannes Schindelin
2007-05-24 13:24                             ` Sven Verdoolaege
2007-05-24 13:52                               ` Johannes Schindelin
2007-05-24 17:42                                 ` Sven Verdoolaege
2007-05-24 18:07                                   ` Johannes Schindelin
2007-05-24 12:41                         ` Lars Hjemli
2007-05-24 13:11                           ` Sven Verdoolaege
2007-05-24 13:32                             ` Lars Hjemli
2007-05-24 17:13                           ` Junio C Hamano
2007-05-24 17:33                             ` Lars Hjemli
2007-05-24 17:38                             ` Sven Verdoolaege
2007-05-24 17:40                             ` Linus Torvalds
2007-05-24 17:55                               ` Sven Verdoolaege
2007-05-24 18:09                                 ` Linus Torvalds
2007-05-24 18:45                                   ` Junio C Hamano
2007-05-24 19:13                                     ` Lars Hjemli
2007-05-24 19:25                                       ` Johannes Schindelin
2007-05-24 18:11                                 ` Johannes Schindelin
2007-05-25 10:00                                   ` Sven Verdoolaege
2007-05-25 16:16                                     ` Junio C Hamano
2007-05-25 16:28                                       ` Sven Verdoolaege
2007-05-25 16:43                                         ` Johannes Schindelin
2007-05-24 18:38                               ` Junio C Hamano
2007-05-25 12:27                               ` Josef Weidendorfer
2007-05-25 12:44                                 ` Johannes Schindelin
2007-05-25 13:59                                   ` Josef Weidendorfer
2007-05-25 14:16                                     ` Johannes Schindelin
2007-05-25 14:38                                       ` Sven Verdoolaege
2007-05-25 14:51                                         ` Johannes Schindelin
2007-05-25 14:51                                       ` Josef Weidendorfer
2007-05-25 14:54                                         ` Johannes Schindelin
2007-05-25 15:35                                     ` Linus Torvalds
2007-05-25 16:23                                       ` Josef Weidendorfer
2007-05-25 16:37                                         ` Johannes Schindelin
2007-05-25 17:09                                           ` Josef Weidendorfer
2007-05-25 12:22                         ` Jakub Narebski
2007-05-25 12:32                           ` Johannes Schindelin
2007-05-24 12:23                       ` Santi Béjar
2007-05-27 20:34                 ` Martin Waitz
2007-05-27 20:40                   ` Sven Verdoolaege
2007-05-24 13:35     ` Martin Waitz
2007-05-24  7:24   ` Sven Verdoolaege
2007-05-24  9:35     ` Johannes Schindelin
2007-05-24 10:54       ` Sven Verdoolaege
2007-05-24 12:38       ` Petr Baudis
2007-05-24 13:13         ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070524071527.GM28023@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    --cc=raa.lkml@gmail.com \
    --cc=skimo@liacs.nl \
    --cc=tali@admingilde.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.