Re: [PATCH v3 20/20] ls-files: use repository object

2017-06-21 Thread Jonathan Tan
On Tue, 20 Jun 2017 12:19:51 -0700
Brandon Williams  wrote:

> -static void show_ce_entry(const struct index_state *istate,
> -   const char *tag, const struct cache_entry *ce)
> +static void show_ce(struct repository *repo, struct dir_struct *dir,
> + const struct cache_entry *ce, const char *fullname,
> + const char *tag)

This function is getting complicated - in particular, it's getting a
"fullname" which is, in a sense, redundant with "repo" and "ce" (but
that seems necessary for efficiency). Maybe add a comment describing the
function?

[snip]

> @@ -651,8 +610,9 @@ int cmd_ls_files(int argc, const char **argv, const char 
> *cmd_prefix)
>   if (require_work_tree && !is_inside_work_tree())
>   setup_work_tree();
>  
> - if (recurse_submodules)
> - compile_submodule_options(argv, &dir, show_tag);
> + if (recurse_submodules) {
> + repo_read_gitmodules(the_repository);
> + }

No need for braces.


[PATCH v3 20/20] ls-files: use repository object

2017-06-20 Thread Brandon Williams
Convert ls-files to use a repository struct and recurse submodules
inprocess.

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

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b12d0bb61..f5f36ddb4 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -5,7 +5,9 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
+#include "repository.h"
 #include "config.h"
 #include "quote.h"
 #include "dir.h"
@@ -32,10 +34,8 @@ static int line_terminator = '\n';
 static int debug_mode;
 static int show_eol;
 static int recurse_submodules;
-static struct argv_array submodule_options = ARGV_ARRAY_INIT;
 
 static const char *prefix;
-static const char *super_prefix;
 static int max_prefix_len;
 static int prefix_len;
 static struct pathspec pathspec;
@@ -73,25 +73,12 @@ static void write_eolinfo(const struct index_state *istate,
 
 static void write_name(const char *name)
 {
-   /*
-* Prepend the super_prefix to name to construct the full_name to be
-* written.
-*/
-   struct strbuf full_name = STRBUF_INIT;
-   if (super_prefix) {
-   strbuf_addstr(&full_name, super_prefix);
-   strbuf_addstr(&full_name, name);
-   name = full_name.buf;
-   }
-
/*
 * With "--full-name", prefix_len=0; this caller needs to pass
 * an empty string in that case (a NULL is good for "").
 */
write_name_quoted_relative(name, prefix_len ? prefix : NULL,
   stdout, line_terminator);
-
-   strbuf_release(&full_name);
 }
 
 static const char *get_tag(const struct cache_entry *ce, const char *tag)
@@ -210,83 +197,38 @@ static void show_killed_files(const struct index_state 
*istate,
}
 }
 
-/*
- * Compile an argv_array with all of the options supported by 
--recurse_submodules
- */
-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");
-   if (show_tag)
-   argv_array_push(&submodule_options, "-t");
-   if (show_valid_bit)
-   argv_array_push(&submodule_options, "-v");
-   if (show_cached)
-   argv_array_push(&submodule_options, "--cached");
-   if (show_eol)
-   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);
-}
+static void show_files(struct repository *repo, struct dir_struct *dir);
 
-/**
- * Recursively call ls-files on a submodule
- */
-static void show_gitlink(const struct cache_entry *ce)
+static void show_submodule(struct repository *superproject,
+  struct dir_struct *dir, const char *path)
 {
-   struct child_process cp = CHILD_PROCESS_INIT;
-   int status;
-   char *dir;
-
-   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,
-prefix);
-   argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
-super_prefix ? super_prefix : "",
-ce->name);
-   argv_array_push(&cp.args, "ls-files");
-   argv_array_push(&cp.args, "--recurse-submodules");
-
-   /* add supported options */
-   argv_array_pushv(&cp.args, submodule_options.argv);
-
-   cp.git_cmd = 1;
-   dir = mkpathdup("%s/%s", get_git_work_tree(), ce->name);
-   cp.dir = dir;
-   status = run_command(&cp);
-   free(dir);
-   if (status)
-   exit(status);
+   struct repository submodule;
+
+   if (repo_submodule_init(&submodule, superproject, path))
+   return;
+
+   if (repo_read_index(&submodule) < 0)
+   die("index file corrupt");
+
+   repo_read_gitmodules(&submodule);
+
+   show_files(&submodule, dir);
+
+   repo_clear(&submodule);
 }
 
-static void show_ce_entry(const struct index_state *istate,
- const char *tag, const struct cache_entry *ce)
+static void show_ce(struct repository *repo, struct dir_struct *dir,
+   const struct cache_entry *ce, const char *fullname,
+   const char *tag)
 {
-   struct strbuf name = STRBUF_INIT;