git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Tim Schumacher" <timschumi@gmx.de>,
	"Duy Nguyen" <pclouds@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH] alias: detect loops in mixed execution mode
Date: Thu, 18 Oct 2018 22:57:39 +0000	[thread overview]
Message-ID: <20181018225739.28857-1-avarab@gmail.com> (raw)
In-Reply-To: <87o9dar9qc.fsf@evledraar.gmail.com>

Add detection for aliasing loops in cases where one of the aliases
re-invokes git as a shell command. This catches cases like:

    [alias]
    foo = !git bar
    bar = !git foo

Before this change running "git {foo,bar}" would create a
forkbomb. Now using the aliasing loop detection and call history
reporting added in 82f71d9a5a ("alias: show the call history when an
alias is looping", 2018-09-16) and c6d75bc17a ("alias: add support for
aliases of an alias", 2018-09-16) we'll instead report:

    fatal: alias loop detected: expansion of 'foo' does not terminate:
      foo <==
      bar ==>

Since the implementation carries the call history in an environment
variable, using the same sort of trick as used for -c (see
2b64fc894d ("pass "git -c foo=bar" params through environment",
2010-08-23) ). For example:

    [alias]
    one = two
    two = !git three
    three = four
    four = !git five
    five = two

Will, on "git one" report:

    fatal: alias loop detected: expansion of 'one' does not terminate:
      one
      two <==
      three
      four
      five ==>

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Implements what I suggested in
https://public-inbox.org/git/87o9dar9qc.fsf@evledraar.gmail.com/

 cache.h          |  1 +
 git.c            | 36 ++++++++++++++++++++++++++++++++++--
 t/t0001-init.sh  |  1 +
 t/t0014-alias.sh | 15 ++++++---------
 4 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index d508f3d4f8..00cbd25f1c 100644
--- a/cache.h
+++ b/cache.h
@@ -478,6 +478,7 @@ static inline enum object_type object_type(unsigned int mode)
 #define TEMPLATE_DIR_ENVIRONMENT "GIT_TEMPLATE_DIR"
 #define CONFIG_ENVIRONMENT "GIT_CONFIG"
 #define CONFIG_DATA_ENVIRONMENT "GIT_CONFIG_PARAMETERS"
+#define COMMAND_HISTORY_ENVIRONMENT "GIT_COMMAND_HISTORY"
 #define EXEC_PATH_ENVIRONMENT "GIT_EXEC_PATH"
 #define CEILING_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
 #define NO_REPLACE_OBJECTS_ENVIRONMENT "GIT_NO_REPLACE_OBJECTS"
diff --git a/git.c b/git.c
index 5920f8019b..cba242836c 100644
--- a/git.c
+++ b/git.c
@@ -672,12 +672,43 @@ static void execv_dashed_external(const char **argv)
 		exit(128);
 }
 
+static void init_cmd_history(struct strbuf *env, struct string_list *cmd_list)
+{
+	const char *old = getenv(COMMAND_HISTORY_ENVIRONMENT);
+	struct strbuf **cmd_history, **ptr;
+
+	if (!old || !*old)
+		return;
+
+	strbuf_addstr(env, old);
+	strbuf_rtrim(env);
+
+	cmd_history = strbuf_split_buf(old, strlen(old), ' ', 0);
+	for (ptr = cmd_history; *ptr; ptr++) {
+		strbuf_rtrim(*ptr);
+		string_list_append(cmd_list, (*ptr)->buf);
+	}
+	strbuf_list_free(cmd_history);
+}
+
+static void add_cmd_history(struct strbuf *env, struct string_list *cmd_list,
+			    const char *cmd)
+{
+	string_list_append(cmd_list, cmd);
+	if (env->len)
+		strbuf_addch(env, ' ');
+	strbuf_addstr(env, cmd);
+	setenv(COMMAND_HISTORY_ENVIRONMENT, env->buf, 1);
+}
+
 static int run_argv(int *argcp, const char ***argv)
 {
 	int done_alias = 0;
-	struct string_list cmd_list = STRING_LIST_INIT_NODUP;
+	struct string_list cmd_list = STRING_LIST_INIT_DUP;
 	struct string_list_item *seen;
+	struct strbuf env = STRBUF_INIT;
 
+	init_cmd_history(&env, &cmd_list);
 	while (1) {
 		/*
 		 * If we tried alias and futzed with our environment,
@@ -711,7 +742,7 @@ static int run_argv(int *argcp, const char ***argv)
 			      " not terminate:%s"), cmd_list.items[0].string, sb.buf);
 		}
 
-		string_list_append(&cmd_list, *argv[0]);
+		add_cmd_history(&env, &cmd_list, *argv[0]);
 
 		/*
 		 * It could be an alias -- this works around the insanity
@@ -724,6 +755,7 @@ static int run_argv(int *argcp, const char ***argv)
 	}
 
 	string_list_clear(&cmd_list, 0);
+	strbuf_release(&env);
 
 	return done_alias;
 }
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 182da069f1..eb2ca8a172 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -93,6 +93,7 @@ test_expect_success 'No extra GIT_* on alias scripts' '
 		sed -n \
 			-e "/^GIT_PREFIX=/d" \
 			-e "/^GIT_TEXTDOMAINDIR=/d" \
+			-e "/^GIT_COMMAND_HISTORY=/d" \
 			-e "/^GIT_/s/=.*//p" |
 		sort
 	EOF
diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
index a070e645d7..9ed03a4a4f 100755
--- a/t/t0014-alias.sh
+++ b/t/t0014-alias.sh
@@ -27,14 +27,11 @@ test_expect_success 'looping aliases - internal execution' '
 	test_i18ngrep "^fatal: alias loop detected: expansion of" output
 '
 
-# This test is disabled until external loops are fixed, because would block
-# the test suite for a full minute.
-#
-#test_expect_failure 'looping aliases - mixed execution' '
-#	git config alias.loop-mixed-1 loop-mixed-2 &&
-#	git config alias.loop-mixed-2 "!git loop-mixed-1" &&
-#	test_must_fail git loop-mixed-1 2>output &&
-#	test_i18ngrep "^fatal: alias loop detected: expansion of" output
-#'
+test_expect_success 'looping aliases - mixed execution' '
+	git config alias.loop-mixed-1 loop-mixed-2 &&
+	git config alias.loop-mixed-2 "!git loop-mixed-1" &&
+	test_must_fail git loop-mixed-1 2>output &&
+	test_i18ngrep "^fatal: alias loop detected: expansion of" output
+'
 
 test_done
-- 
2.19.1.568.g152ad8e336


  reply	other threads:[~2018-10-18 22:57 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-05  8:54 [RFC PATCH v2] Allow aliases that include other aliases Tim Schumacher
2018-09-05 15:48 ` Duy Nguyen
2018-09-05 19:02   ` Tim Schumacher
2018-09-05 17:12 ` Junio C Hamano
2018-09-05 19:12   ` Tim Schumacher
2018-09-05 17:34 ` Jeff King
2018-09-05 20:02   ` Tim Schumacher
2018-09-06 13:38     ` Ævar Arnfjörð Bjarmason
2018-09-06 14:17     ` Ævar Arnfjörð Bjarmason
2018-10-18 22:57       ` Ævar Arnfjörð Bjarmason [this message]
2018-10-19  8:28         ` [PATCH] alias: detect loops in mixed execution mode Ævar Arnfjörð Bjarmason
2018-10-19 22:09           ` Jeff King
2018-10-20 10:52             ` Ævar Arnfjörð Bjarmason
2018-10-19 22:07         ` Jeff King
2018-10-20 11:14           ` Ævar Arnfjörð Bjarmason
2018-10-20 18:58             ` Jeff King
2018-10-20 19:18               ` Ævar Arnfjörð Bjarmason
2018-10-22 21:15                 ` Jeff King
2018-10-22 21:28                   ` Ævar Arnfjörð Bjarmason
2018-10-22  1:23               ` Junio C Hamano
2018-10-26  8:39               ` Jeff King
2018-10-26 12:44                 ` Ævar Arnfjörð Bjarmason
2018-10-29  3:44                 ` Junio C Hamano
2018-10-29 14:17                   ` Jeff King
2018-09-05 21:51   ` [RFC PATCH v2] Allow aliases that include other aliases Junio C Hamano
2018-09-06 10:16 ` [PATCH v3] " Tim Schumacher
2018-09-06 14:01   ` Ævar Arnfjörð Bjarmason
2018-09-06 14:57     ` Jeff King
2018-09-06 15:10       ` Ævar Arnfjörð Bjarmason
2018-09-06 16:18         ` Jeff King
2018-09-06 19:05       ` Tim Schumacher
2018-09-06 19:17         ` Jeff King
2018-09-06 14:59   ` Jeff King
2018-09-06 18:40     ` Junio C Hamano
2018-09-06 19:05       ` Jeff King
2018-09-06 19:31       ` Tim Schumacher
2018-09-07 22:44 ` [RFC PATCH v4 1/3] Add support for nested aliases Tim Schumacher
2018-09-07 22:44   ` [RFC PATCH v4 2/3] Show the call history when an alias is looping Tim Schumacher
2018-09-08 13:34     ` Duy Nguyen
2018-09-08 16:29       ` Jeff King
2018-09-07 22:44   ` [RFC PATCH v4 3/3] t0014: Introduce alias testing suite Tim Schumacher
2018-09-07 23:38     ` Eric Sunshine
2018-09-14 23:12       ` Tim Schumacher
2018-09-16  7:21         ` Eric Sunshine
2018-09-08 13:28   ` [RFC PATCH v4 1/3] Add support for nested aliases Duy Nguyen
2018-09-16  7:46     ` Tim Schumacher
2018-09-17 15:37       ` Junio C Hamano
2018-09-21 12:45         ` Tim Schumacher
2018-09-21 15:59           ` Junio C Hamano
2018-09-16  7:50   ` [PATCH v5 " Tim Schumacher
2018-09-16  7:50     ` [PATCH v5 2/3] Show the call history when an alias is looping Tim Schumacher
2018-09-16  7:50     ` [PATCH v5 3/3] t0014: Introduce an alias testing suite Tim Schumacher

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20181018225739.28857-1-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=timschumi@gmx.de \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).