ls-files: fix bug when recursing with relative pathspec

When using the --recurse-submodules flag with a relative pathspec which
includes "..", an error is produced inside the child process spawned for a
submodule.  When creating the pathspec struct in the child, the ".." is
interpreted to mean "go up a directory" which causes an error stating that the
path ".." is outside of the repository.

While it is true that ".." is outside the scope of the submodule, it is
confusing to a user who originally invoked the command where ".." was indeed
still inside the scope of the superproject.  Since the child process launched
for the submodule has some context that it is operating underneath a
superproject, this error could be avoided.

This patch fixes the bug by passing the 'prefix' to the child process.  Now
each child process that works on a submodule has two points of reference to the
superproject: (1) the 'super_prefix' which is the path from the root of the
superproject down to root of the submodule and (2) the 'prefix' which is the
path from the root of the superproject down to the directory where the user
invoked the git command.

With these two pieces of information a child process can correctly interpret
the pathspecs provided by the user as well as being able to properly format its
output relative to the directory the user invoked the original command from.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index ca5b48db04a8e578d38e7d44d4be7223081558dd..d449e46db551fcc6284a454e1122bdee6af1954c 100644 (file)
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -172,7 +172,9 @@ static void show_killed_files(struct dir_struct *dir)
 /*
  * Compile an argv_array with all of the options supported by --recurse_submodules
  */
-static void compile_submodule_options(const struct dir_struct *dir, int show_tag)
+static void compile_submodule_options(const char **argv,
+                                     const struct dir_struct *dir,
+                                     int show_tag)
 {
        if (line_terminator == '\0')
                argv_array_push(&submodule_options, "-z");
@@ -186,6 +188,11 @@ static void compile_submodule_options(const struct dir_struct *dir, int show_tag
                argv_array_push(&submodule_options, "--eol");
        if (debug_mode)
                argv_array_push(&submodule_options, "--debug");
+
+       /* Add Pathspecs */
+       argv_array_push(&submodule_options, "--");
+       for (; *argv; argv++)
+               argv_array_push(&submodule_options, *argv);
 }
 
 /**
@@ -195,8 +202,11 @@ static void show_gitlink(const struct cache_entry *ce)
 {
        struct child_process cp = CHILD_PROCESS_INIT;
        int status;
-       int i;
 
+       if (prefix_len)
+               argv_array_pushf(&cp.env_array, "%s=%s",
+                                GIT_TOPLEVEL_PREFIX_ENVIRONMENT,
+                                prefix);
        argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
                         super_prefix ? super_prefix : "",
                         ce->name);
@@ -206,15 +216,6 @@ static void show_gitlink(const struct cache_entry *ce)
        /* add supported options */
        argv_array_pushv(&cp.args, submodule_options.argv);
 
-       /*
-        * Pass in the original pathspec args.  The submodule will be
-        * responsible for prepending the 'submodule_prefix' prior to comparing
-        * against the pathspec for matches.
-        */
-       argv_array_push(&cp.args, "--");
-       for (i = 0; i < pathspec.nr; i++)
-               argv_array_push(&cp.args, pathspec.items[i].original);
-
        cp.git_cmd = 1;
        cp.dir = ce->name;
        status = run_command(&cp);
@@ -604,7 +605,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
                setup_work_tree();
 
        if (recurse_submodules)
-               compile_submodule_options(&dir, show_tag);
+               compile_submodule_options(argv, &dir, show_tag);
 
        if (recurse_submodules &&
            (show_stage || show_deleted || show_others || show_unmerged ||

diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
index a5426171d3ec652ffc02fc8279bc5802f57204a3..4cf6ccf5a8eadac4cc0ebb8b3091f1f3befa73a9 100755 (executable)
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -188,6 +188,45 @@ test_expect_success '--recurse-submodules and pathspecs' '
        test_cmp expect actual
 '
 
+test_expect_success '--recurse-submodules and relative paths' '
+       # From subdir
+       cat >expect <<-\EOF &&
+       b
+       EOF
+       git -C b ls-files --recurse-submodules >actual &&
+       test_cmp expect actual &&
+
+       # Relative path to top
+       cat >expect <<-\EOF &&
+       ../.gitmodules
+       ../a
+       b
+       ../h.txt
+       ../sib/file
+       ../sub/file
+       ../submodule/.gitmodules
+       ../submodule/c
+       ../submodule/f.TXT
+       ../submodule/g.txt
+       ../submodule/subsub/d
+       ../submodule/subsub/e.txt
+       EOF
+       git -C b ls-files --recurse-submodules -- .. >actual &&
+       test_cmp expect actual &&
+
+       # Relative path to submodule
+       cat >expect <<-\EOF &&
+       ../submodule/.gitmodules
+       ../submodule/c
+       ../submodule/f.TXT
+       ../submodule/g.txt
+       ../submodule/subsub/d
+       ../submodule/subsub/e.txt
+       EOF
+       git -C b ls-files --recurse-submodules -- ../submodule >actual &&
+       test_cmp expect actual
+'
+
 test_expect_success '--recurse-submodules does not support --error-unmatch' '
        test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual &&
        test_i18ngrep "does not support --error-unmatch" actual