All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: git@vger.kernel.org
Cc: Brandon Williams <bmwill@google.com>,
	Stefan Beller <sbeller@google.com>,
	Jacob Keller <jacob.keller@gmail.com>
Subject: [PATCH v2 1/2] ls-files: fix recurse-submodules with nested submodules
Date: Thu, 13 Apr 2017 10:12:23 -0700	[thread overview]
Message-ID: <20170413171224.3537-1-jacob.e.keller@intel.com> (raw)
In-Reply-To: <CA+P7+xqE-SZ5D6pk6-Dx+-VzHijmgmrUR2F=-370Rh8oKr10Nw@mail.gmail.com>

From: Jacob Keller <jacob.keller@gmail.com>

Since commit e77aa336f116 ("ls-files: optionally recurse into
submodules", 2016-10-07) ls-files has known how to recurse into
submodules when displaying files.

Unfortunately this fails for certain cases, including when nesting more
than one submodule, called from within a submodule that itself has
submodules, or when the GIT_DIR environemnt variable is set.

Prior to commit b58a68c1c187 ("setup: allow for prefix to be passed to
git commands", 2017-03-17) this resulted in an error indicating that
--prefix and --super-prefix were incompatible.

After this commit, instead, the process loops forever with a GIT_DIR set
to the parent and continuously reads the parent submodule files and
recursing forever.

Fix this by preparing the environment properly for submodules when
setting up the child process. This is similar to how other commands such
as grep behave.

This was not caught by the original tests because the scenario is
avoided if the submodules are created separately and not stored as the
standard method of putting the submodule git directory under
.git/modules/<name>. We can update the test to show the failure by the
addition of "git submodule absorbgitdirs" to the test case. However,
note that this new test would run forever without the necessary fix in
this patch.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
I updated and reworded the description so that the problem would be more
obvious. It doesn't occur always, but only when run nested with properly
absorbed gitdirs for the submodules. This explains the reason why the
test case had not caught the issue before.

 builtin/ls-files.c                     | 4 ++++
 t/t3007-ls-files-recurse-submodules.sh | 1 +
 2 files changed, 5 insertions(+)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index d449e46db551..e9b3546ca053 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -15,6 +15,7 @@
 #include "string-list.h"
 #include "pathspec.h"
 #include "run-command.h"
+#include "submodule.h"
 
 static int abbrev;
 static int show_deleted;
@@ -203,6 +204,9 @@ static void show_gitlink(const struct cache_entry *ce)
 	struct child_process cp = CHILD_PROCESS_INIT;
 	int status;
 
+	prepare_submodule_repo_env(&cp.env_array);
+	argv_array_push(&cp.env_array, GIT_DIR_ENVIRONMENT);
+
 	if (prefix_len)
 		argv_array_pushf(&cp.env_array, "%s=%s",
 				 GIT_TOPLEVEL_PREFIX_ENVIRONMENT,
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
index 4cf6ccf5a8ea..c8030dd3299a 100755
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -77,6 +77,7 @@ test_expect_success 'ls-files recurses more than 1 level' '
 	git -C submodule/subsub commit -m "add d" &&
 	git -C submodule submodule add ./subsub &&
 	git -C submodule commit -m "added subsub" &&
+	git submodule absorbgitdirs &&
 	git ls-files --recurse-submodules >actual &&
 	test_cmp expect actual
 '
-- 
2.12.2.776.gded3dc243c29.dirty


  reply	other threads:[~2017-04-13 17:12 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-12  0:39 [PATCH] ls-files: properly prepare submodule environment Jacob Keller
2017-04-12 16:58 ` Brandon Williams
2017-04-12 22:45   ` Jacob Keller
2017-04-13 17:12     ` Jacob Keller [this message]
2017-04-13 17:12       ` [PATCH v2 2/2] ls-files: fix path used when recursing into submodules Jacob Keller
2017-04-13 18:10         ` Brandon Williams
2017-04-13 18:15         ` Stefan Beller
2017-04-13 18:34           ` Jacob Keller
2017-04-18  2:03         ` Junio C Hamano
2017-04-18  7:42           ` Jacob Keller
2017-04-18 18:41           ` Stefan Beller
2017-04-13 18:03       ` [PATCH v2 1/2] ls-files: fix recurse-submodules with nested submodules Brandon Williams
2017-04-13 18:31         ` Jacob Keller
2017-04-13 18:35           ` Stefan Beller
2017-04-13 18:36             ` Brandon Williams
2017-04-13 18:57       ` [PATCH 3/2] ls-files: only recurse on active submodules Brandon Williams
2017-04-13 19:05         ` Stefan Beller
2017-04-13 19:12           ` Jacob Keller
2017-04-13 19:25             ` Stefan Beller
2017-04-13 19:30               ` Jacob Keller
2017-04-14 16:33               ` Jacob Keller
2017-04-14 17:02                 ` Stefan Beller
2017-04-14 23:49                   ` Jacob Keller
2017-04-19  1:03         ` Junio C Hamano
2017-05-12 16:21           ` paul
2017-05-12 17:26             ` Brandon Williams
2017-05-12 18:12               ` Paul Jolly
2017-05-12 18:19                 ` Brandon Williams
2017-08-01 18:16                   ` Paul Jolly
     [not found]                   ` <CACoUkn7i76dEsQa3eoN+7WR8QmsD1pWsRQ0dvhkxzFN0sxTmRQ@mail.gmail.com>
2017-08-01 18:18                     ` Brandon Williams
2017-08-01 18:19                       ` Paul Jolly
2017-08-01 18:20                         ` Brandon Williams
2017-08-01 18:41                           ` Junio C Hamano

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=20170413171224.3537-1-jacob.e.keller@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=jacob.keller@gmail.com \
    --cc=sbeller@google.com \
    /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.