Re: [PATCH 5/5] ls-files: fix bug when recuring with relative pathspec

[Brandon Williams]

On 02/28, Junio C Hamano wrote:
> Brandon Williams  writes:
> 
> > Fix a bug which causes a child process for a submodule to error out when a
> > relative pathspec with a ".." is provided in the superproject.
> >
> > While at it, correctly construct the super-prefix to be used in a submodule
> > when not at the root of the repository.
> >
> > Signed-off-by: Brandon Williams 
> > ---
> >  builtin/ls-files.c | 8 ++--
> >  t/t3007-ls-files-recurse-submodules.sh | 2 +-
> >  2 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> > index 159229081..89533ab8e 100644
> > --- a/builtin/ls-files.c
> > +++ b/builtin/ls-files.c
> > @@ -194,12 +194,15 @@ static void compile_submodule_options(const struct 
> > dir_struct *dir, int show_tag
> >  static void show_gitlink(const struct cache_entry *ce)
> >  {
> > struct child_process cp = CHILD_PROCESS_INIT;
> > +   struct strbuf name = STRBUF_INIT;
> > int status;
> > int i;
> >  
> > +   quote_path_relative(ce->name, prefix, );
> > argv_array_pushf(, "--super-prefix=%s%s/",
> 
> Same comment as 3/5.  quote_path is to produce c-quote and is not
> even meant for shell script quoting.  run_command() interface would
> do its own shell quoting when needed, so  I think you just want the
> exact string you want to pass here.
> 

Yeah I don't know what I was thinking when using that instead of
'relative_path()'.  Will change for this and 3/5.

-- 
Brandon Williams

Re: [PATCH 5/5] ls-files: fix bug when recuring with relative pathspec

[Junio C Hamano]

Brandon Williams  writes:

> Fix a bug which causes a child process for a submodule to error out when a
> relative pathspec with a ".." is provided in the superproject.
>
> While at it, correctly construct the super-prefix to be used in a submodule
> when not at the root of the repository.
>
> Signed-off-by: Brandon Williams 
> ---
>  builtin/ls-files.c | 8 ++--
>  t/t3007-ls-files-recurse-submodules.sh | 2 +-
>  2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 159229081..89533ab8e 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -194,12 +194,15 @@ static void compile_submodule_options(const struct 
> dir_struct *dir, int show_tag
>  static void show_gitlink(const struct cache_entry *ce)
>  {
>   struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf name = STRBUF_INIT;
>   int status;
>   int i;
>  
> + quote_path_relative(ce->name, prefix, );
>   argv_array_pushf(, "--super-prefix=%s%s/",

Same comment as 3/5.  quote_path is to produce c-quote and is not
even meant for shell script quoting.  run_command() interface would
do its own shell quoting when needed, so  I think you just want the
exact string you want to pass here.

[PATCH 5/5] ls-files: fix bug when recuring with relative pathspec

[Brandon Williams]

Fix a bug which causes a child process for a submodule to error out when a
relative pathspec with a ".." is provided in the superproject.

While at it, correctly construct the super-prefix to be used in a submodule
when not at the root of the repository.

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 8 ++--
 t/t3007-ls-files-recurse-submodules.sh | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 159229081..89533ab8e 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -194,12 +194,15 @@ static void compile_submodule_options(const struct 
dir_struct *dir, int show_tag
 static void show_gitlink(const struct cache_entry *ce)
 {
struct child_process cp = CHILD_PROCESS_INIT;
+   struct strbuf name = STRBUF_INIT;
int status;
int i;
 
+   quote_path_relative(ce->name, prefix, );
argv_array_pushf(, "--super-prefix=%s%s/",
 super_prefix ? super_prefix : "",
-ce->name);
+name.buf);
+   strbuf_release();
argv_array_push(, "ls-files");
argv_array_push(, "--recurse-submodules");
 
@@ -615,7 +618,8 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
 
parse_pathspec(, 0,
   PATHSPEC_PREFER_CWD |
-  PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+  PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP |
+  (super_prefix ? PATHSPEC_FROMROOT : 0),
   prefix, argv);
 
/*
diff --git a/t/t3007-ls-files-recurse-submodules.sh 
b/t/t3007-ls-files-recurse-submodules.sh
index d8ab10866..1522ed235 100755
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -188,7 +188,7 @@ test_expect_success '--recurse-submodules and pathspecs' '
test_cmp expect actual
 '
 
-test_expect_failure '--recurse-submodules and relative paths' '
+test_expect_success '--recurse-submodules and relative paths' '
# From top works
cat >expect <<-\EOF &&
.gitmodules
-- 
2.11.0.483.g087da7b7c-goog