mirror of
https://github.com/git/git
synced 2024-08-25 02:35:46 +00:00
git: simplify environment save/restore logic
The only code that cares about the value of the global variable saved_env_before_alias after the previous fix is handle_builtin() that turns into a glorified no-op when the variable is true, so the logic could safely be lifted to its caller, i.e. the caller can refrain from calling it when the variable is set. This variable tells us if save_env_before_alias() was called (with or without matching restore_env()), but the sole caller of the function, handle_alias(), always calls it as the first thing, so we can consider that the variable essentially keeps track of the fact that handle_alias() has ever been called. It turns out that handle_builtin() and handle_alias() are called only from one function in a way that the value of the variable matters, which is run_argv(), and it already keeps track of the fact that it already called handle_alias(). So we can simplify the whole thing by: - Change handle_builtin() to always make a direct call to the builtin implementation it finds, and make sure the caller refrains from calling it if handle_alias() has ever been called; - Remove saved_env_before_alias variable, and instead use the local "done_alias" variable maintained inside run_argv() to make the same decision. Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
parent
2e1175d43d
commit
441981bc85
27
git.c
27
git.c
|
@ -25,13 +25,11 @@ static const char *env_names[] = {
|
|||
GIT_PREFIX_ENVIRONMENT
|
||||
};
|
||||
static char *orig_env[4];
|
||||
static int saved_env_before_alias;
|
||||
static int save_restore_env_balance;
|
||||
|
||||
static void save_env_before_alias(void)
|
||||
{
|
||||
int i;
|
||||
saved_env_before_alias = 1;
|
||||
|
||||
assert(save_restore_env_balance == 0);
|
||||
save_restore_env_balance = 1;
|
||||
|
@ -533,16 +531,8 @@ static void handle_builtin(int argc, const char **argv)
|
|||
}
|
||||
|
||||
builtin = get_builtin(cmd);
|
||||
if (builtin) {
|
||||
/*
|
||||
* XXX: if we can figure out cases where it is _safe_
|
||||
* to do, we can avoid spawning a new process when
|
||||
* saved_env_before_alias is true
|
||||
* (i.e. setup_git_dir* has been run once)
|
||||
*/
|
||||
if (!saved_env_before_alias)
|
||||
exit(run_builtin(builtin, argc, argv));
|
||||
}
|
||||
if (builtin)
|
||||
exit(run_builtin(builtin, argc, argv));
|
||||
}
|
||||
|
||||
static void execv_dashed_external(const char **argv)
|
||||
|
@ -586,8 +576,17 @@ static int run_argv(int *argcp, const char ***argv)
|
|||
int done_alias = 0;
|
||||
|
||||
while (1) {
|
||||
/* See if it's a builtin */
|
||||
handle_builtin(*argcp, *argv);
|
||||
/*
|
||||
* If we tried alias and futzed with our environment,
|
||||
* it no longer is safe to invoke builtins directly in
|
||||
* general. We have to spawn them as dashed externals.
|
||||
*
|
||||
* NEEDSWORK: if we can figure out cases
|
||||
* where it is safe to do, we can avoid spawning a new
|
||||
* process.
|
||||
*/
|
||||
if (!done_alias)
|
||||
handle_builtin(*argcp, *argv);
|
||||
|
||||
/* .. then try the external ones */
|
||||
execv_dashed_external(*argv);
|
||||
|
|
Loading…
Reference in a new issue