[WIP/RFC 00/23] repository object

[WIP/RFC 00/23] repository object

[Brandon Williams]

When I first started working on the git project I found it very difficult to
understand parts of the code base because of the inherently global nature of
our code.  It also made working on submodules very difficult.  Since we can
only open up a single repository per process, you need to launch a child
process in order to process a submodule.  But you also need to be able to
communicate other stateful information to the children processes so that the
submodules know how best to format their output or match against a
pathspec...it ends up feeling like layering on hack after hack.  What I would
really like to do, is to have the ability to have a repository object so that I
can open a submodule in-process.

Before this becomes a reality for all commands, much of the library code would
need to be refactored in order to work purely on handles instead of global
state.  As it turned out, ls-files is a pretty simple command and doesn't have
*too* many dependencies.  The biggest thing that needed to be changed was
piping through an index into a couple library routines so that they don't
inherently rely on 'the_index'.  A few of these changes I've sent out and can
be found at 'origin/bw/pathspec-sans-the-index' and
'origin/bw/dir-c-stops-relying-on-the-index' which this series is based on.

Patches 1-16 are refactorings to prepare either library code or ls-files itself
to be ready to handle passing around an index struct.  Patches 17-22 introduce
a repository struct and change a couple of things about how submodule caches
work (getting submodule information from .gitmodules).  And Patch 23 converts
ls-files to use a repository struct. 

The most interesting part of the series is from 17-23.  And 1-16 could be taken
as is without the rest of the series.

This is still very much in a WIP state, though it does pass all tests.  What
I'm hoping for here is to get a discussion started about the feasibility of a
change like this and hopefully to get the ball rolling.  Is this a direction we
want to move in?  Is it worth the pain?

Thanks for taking the time to look at this and entertain my insane ideas :)

Brandon Williams (23):
  convert: convert get_cached_convert_stats_ascii to take an index
  convert: convert crlf_to_git to take an index
  convert: convert convert_to_git_filter_fd to take an index
  convert: convert convert_to_git to take an index
  convert: convert renormalize_buffer to take an index
  tree: convert read_tree to take an index parameter
  ls-files: convert overlay_tree_on_cache to take an index
  ls-files: convert write_eolinfo to take an index
  ls-files: convert show_killed_files to take an index
  ls-files: convert show_other_files to take an index
  ls-files: convert show_ru_info to take an index
  ls-files: convert ce_excluded to take an index
  ls-files: convert prune_cache to take an index
  ls-files: convert show_files to take an index
  ls-files: factor out debug info into a function
  ls-files: factor out tag calculation
  repo: introduce new repository object
  repo: add index_state to struct repo
  repo: add per repo config
  submodule-config: refactor to allow for multiple submodule_cache's
  repo: add repo_read_gitmodules
  submodule: add is_submodule_active
  ls-files: use repository object

 Makefile                               |   1 +
 apply.c                                |   2 +-
 builtin/blame.c                        |   2 +-
 builtin/commit.c                       |   3 +-
 builtin/ls-files.c                     | 348 ++++++++++++++++-----------------
 cache.h                                |   4 +-
 combine-diff.c                         |   2 +-
 config.c                               |   2 +-
 convert.c                              |  31 +--
 convert.h                              |  19 +-
 diff.c                                 |   6 +-
 dir.c                                  |   2 +-
 git.c                                  |   2 +-
 ll-merge.c                             |   2 +-
 merge-recursive.c                      |   4 +-
 repo.c                                 | 112 +++++++++++
 repo.h                                 |  22 +++
 sha1_file.c                            |   6 +-
 submodule-config.c                     |  40 +++-
 submodule-config.h                     |  10 +
 submodule.c                            |  51 +++++
 submodule.h                            |   2 +
 t/t3007-ls-files-recurse-submodules.sh |  39 ++++
 tree.c                                 |  28 ++-
 tree.h                                 |   3 +-
 25 files changed, 513 insertions(+), 230 deletions(-)
 create mode 100644 repo.c
 create mode 100644 repo.h

-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 01/23] convert: convert get_cached_convert_stats_ascii to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 3 ++-
 convert.c          | 5 +++--
 convert.h          | 5 ++++-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 89b3ce8e5..770d8774a 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -63,7 +63,8 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path)
 		const char *w_txt = "";
 		const char *a_txt = get_convert_attr_ascii(path);
 		if (ce && S_ISREG(ce->ce_mode))
-			i_txt = get_cached_convert_stats_ascii(ce->name);
+			i_txt = get_cached_convert_stats_ascii(&the_index,
+							       ce->name);
 		if (!lstat(path, &st) && S_ISREG(st.st_mode))
 			w_txt = get_wt_convert_stats_ascii(path);
 		printf("i/%-5s w/%-5s attr/%-17s\t", i_txt, w_txt, a_txt);
diff --git a/convert.c b/convert.c
index 8d652bf27..b1b90d6e6 100644
--- a/convert.c
+++ b/convert.c
@@ -133,11 +133,12 @@ static const char *gather_convert_stats_ascii(const char *data, unsigned long si
 	}
 }
 
-const char *get_cached_convert_stats_ascii(const char *path)
+const char *get_cached_convert_stats_ascii(const struct index_state *istate,
+					   const char *path)
 {
 	const char *ret;
 	unsigned long sz;
-	void *data = read_blob_data_from_cache(path, &sz);
+	void *data = read_blob_data_from_index(istate, path, &sz);
 	ret = gather_convert_stats_ascii(data, sz);
 	free(data);
 	return ret;
diff --git a/convert.h b/convert.h
index 82871a11d..667b7dfe0 100644
--- a/convert.h
+++ b/convert.h
@@ -4,6 +4,8 @@
 #ifndef CONVERT_H
 #define CONVERT_H
 
+struct index_state;
+
 enum safe_crlf {
 	SAFE_CRLF_FALSE = 0,
 	SAFE_CRLF_FAIL = 1,
@@ -33,7 +35,8 @@ enum eol {
 };
 
 extern enum eol core_eol;
-extern const char *get_cached_convert_stats_ascii(const char *path);
+extern const char *get_cached_convert_stats_ascii(const struct index_state *istate,
+						  const char *path);
 extern const char *get_wt_convert_stats_ascii(const char *path);
 extern const char *get_convert_attr_ascii(const char *path);
 
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 02/23] convert: convert crlf_to_git to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 convert.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/convert.c b/convert.c
index b1b90d6e6..f5773cfe1 100644
--- a/convert.c
+++ b/convert.c
@@ -217,13 +217,13 @@ static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
 	}
 }
 
-static int has_cr_in_index(const char *path)
+static int has_cr_in_index(const struct index_state *istate, const char *path)
 {
 	unsigned long sz;
 	void *data;
 	int has_cr;
 
-	data = read_blob_data_from_cache(path, &sz);
+	data = read_blob_data_from_index(istate, path, &sz);
 	if (!data)
 		return 0;
 	has_cr = memchr(data, '\r', sz) != NULL;
@@ -253,7 +253,8 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
 
 }
 
-static int crlf_to_git(const char *path, const char *src, size_t len,
+static int crlf_to_git(const struct index_state *istate,
+		       const char *path, const char *src, size_t len,
 		       struct strbuf *buf,
 		       enum crlf_action crlf_action, enum safe_crlf checksafe)
 {
@@ -285,7 +286,8 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 		 * unless we want to renormalize in a merge or
 		 * cherry-pick.
 		 */
-		if ((checksafe != SAFE_CRLF_RENORMALIZE) && has_cr_in_index(path))
+		if ((checksafe != SAFE_CRLF_RENORMALIZE) &&
+		    has_cr_in_index(istate, path))
 			convert_crlf_into_lf = 0;
 	}
 	if ((checksafe == SAFE_CRLF_WARN ||
@@ -1193,7 +1195,7 @@ int convert_to_git(const char *path, const char *src, size_t len,
 		src = dst->buf;
 		len = dst->len;
 	}
-	ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);
+	ret |= crlf_to_git(&the_index, path, src, len, dst, ca.crlf_action, checksafe);
 	if (ret && dst) {
 		src = dst->buf;
 		len = dst->len;
@@ -1213,7 +1215,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
 	if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))
 		die("%s: clean filter '%s' failed", path, ca.drv->name);
 
-	crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
+	crlf_to_git(&the_index, path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
 	ident_to_git(path, dst->buf, dst->len, dst, ca.ident);
 }
 
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 03/23] convert: convert convert_to_git_filter_fd to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 convert.c   | 5 +++--
 convert.h   | 3 ++-
 sha1_file.c | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/convert.c b/convert.c
index f5773cfe1..d50e7f7aa 100644
--- a/convert.c
+++ b/convert.c
@@ -1203,7 +1203,8 @@ int convert_to_git(const char *path, const char *src, size_t len,
 	return ret | ident_to_git(path, src, len, dst, ca.ident);
 }
 
-void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
+void convert_to_git_filter_fd(const struct index_state *istate,
+			      const char *path, int fd, struct strbuf *dst,
 			      enum safe_crlf checksafe)
 {
 	struct conv_attrs ca;
@@ -1215,7 +1216,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
 	if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))
 		die("%s: clean filter '%s' failed", path, ca.drv->name);
 
-	crlf_to_git(&the_index, path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
+	crlf_to_git(istate, path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
 	ident_to_git(path, dst->buf, dst->len, dst, ca.ident);
 }
 
diff --git a/convert.h b/convert.h
index 667b7dfe0..3a813a797 100644
--- a/convert.h
+++ b/convert.h
@@ -52,7 +52,8 @@ static inline int would_convert_to_git(const char *path)
 	return convert_to_git(path, NULL, 0, NULL, 0);
 }
 /* Precondition: would_convert_to_git_filter_fd(path) == true */
-extern void convert_to_git_filter_fd(const char *path, int fd,
+extern void convert_to_git_filter_fd(const struct index_state *istate,
+				     const char *path, int fd,
 				     struct strbuf *dst,
 				     enum safe_crlf checksafe);
 extern int would_convert_to_git_filter_fd(const char *path);
diff --git a/sha1_file.c b/sha1_file.c
index 59a4ed2ed..ab09241d2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3580,7 +3580,7 @@ static int index_stream_convert_blob(unsigned char *sha1, int fd,
 	assert(path);
 	assert(would_convert_to_git_filter_fd(path));
 
-	convert_to_git_filter_fd(path, fd, &sbuf,
+	convert_to_git_filter_fd(&the_index, path, fd, &sbuf,
 				 write_object ? safe_crlf : SAFE_CRLF_FALSE);
 
 	if (write_object)
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 04/23] convert: convert convert_to_git to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 apply.c         | 2 +-
 builtin/blame.c | 2 +-
 combine-diff.c  | 2 +-
 convert.c       | 7 ++++---
 convert.h       | 8 +++++---
 diff.c          | 6 +++---
 dir.c           | 2 +-
 sha1_file.c     | 4 ++--
 8 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/apply.c b/apply.c
index e6dbab26a..7813faacf 100644
--- a/apply.c
+++ b/apply.c
@@ -2267,7 +2267,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)
 	case S_IFREG:
 		if (strbuf_read_file(buf, path, st->st_size) != st->st_size)
 			return error(_("unable to open or read %s"), path);
-		convert_to_git(path, buf->buf, buf->len, buf, 0);
+		convert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);
 		return 0;
 	default:
 		return -1;
diff --git a/builtin/blame.c b/builtin/blame.c
index 07506a3e4..8a9c0eb01 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2383,7 +2383,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
 		if (strbuf_read(&buf, 0, 0) < 0)
 			die_errno("failed to read from stdin");
 	}
-	convert_to_git(path, buf.buf, buf.len, &buf, 0);
+	convert_to_git(&the_index, path, buf.buf, buf.len, &buf, 0);
 	origin->file.ptr = buf.buf;
 	origin->file.size = buf.len;
 	pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
diff --git a/combine-diff.c b/combine-diff.c
index 2848034fe..74f723af3 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1053,7 +1053,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			if (is_file) {
 				struct strbuf buf = STRBUF_INIT;
 
-				if (convert_to_git(elem->path, result, len, &buf, safe_crlf)) {
+				if (convert_to_git(&the_index, elem->path, result, len, &buf, safe_crlf)) {
 					free(result);
 					result = strbuf_detach(&buf, &len);
 					result_size = len;
diff --git a/convert.c b/convert.c
index d50e7f7aa..3d1c7e2b6 100644
--- a/convert.c
+++ b/convert.c
@@ -1179,7 +1179,8 @@ const char *get_convert_attr_ascii(const char *path)
 	return "";
 }
 
-int convert_to_git(const char *path, const char *src, size_t len,
+int convert_to_git(const struct index_state *istate,
+		   const char *path, const char *src, size_t len,
                    struct strbuf *dst, enum safe_crlf checksafe)
 {
 	int ret = 0;
@@ -1195,7 +1196,7 @@ int convert_to_git(const char *path, const char *src, size_t len,
 		src = dst->buf;
 		len = dst->len;
 	}
-	ret |= crlf_to_git(&the_index, path, src, len, dst, ca.crlf_action, checksafe);
+	ret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);
 	if (ret && dst) {
 		src = dst->buf;
 		len = dst->len;
@@ -1266,7 +1267,7 @@ int renormalize_buffer(const char *path, const char *src, size_t len, struct str
 		src = dst->buf;
 		len = dst->len;
 	}
-	return ret | convert_to_git(path, src, len, dst, SAFE_CRLF_RENORMALIZE);
+	return ret | convert_to_git(&the_index, path, src, len, dst, SAFE_CRLF_RENORMALIZE);
 }
 
 /*****************************************************************
diff --git a/convert.h b/convert.h
index 3a813a797..60cb41d6a 100644
--- a/convert.h
+++ b/convert.h
@@ -41,15 +41,17 @@ extern const char *get_wt_convert_stats_ascii(const char *path);
 extern const char *get_convert_attr_ascii(const char *path);
 
 /* returns 1 if *dst was used */
-extern int convert_to_git(const char *path, const char *src, size_t len,
+extern int convert_to_git(const struct index_state *istate,
+			  const char *path, const char *src, size_t len,
 			  struct strbuf *dst, enum safe_crlf checksafe);
 extern int convert_to_working_tree(const char *path, const char *src,
 				   size_t len, struct strbuf *dst);
 extern int renormalize_buffer(const char *path, const char *src, size_t len,
 			      struct strbuf *dst);
-static inline int would_convert_to_git(const char *path)
+static inline int would_convert_to_git(const struct index_state *istate,
+				       const char *path)
 {
-	return convert_to_git(path, NULL, 0, NULL, 0);
+	return convert_to_git(istate, path, NULL, 0, NULL, 0);
 }
 /* Precondition: would_convert_to_git_filter_fd(path) == true */
 extern void convert_to_git_filter_fd(const struct index_state *istate,
diff --git a/diff.c b/diff.c
index 74283d900..ab19453e1 100644
--- a/diff.c
+++ b/diff.c
@@ -2755,7 +2755,7 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
 	 * Similarly, if we'd have to convert the file contents anyway, that
 	 * makes the optimization not worthwhile.
 	 */
-	if (!want_file && would_convert_to_git(name))
+	if (!want_file && would_convert_to_git(&the_index, name))
 		return 0;
 
 	len = strlen(name);
@@ -2877,7 +2877,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
 		 * point if the path requires us to run the content
 		 * conversion.
 		 */
-		if (size_only && !would_convert_to_git(s->path))
+		if (size_only && !would_convert_to_git(&the_index, s->path))
 			return 0;
 
 		/*
@@ -2904,7 +2904,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
 		/*
 		 * Convert from working tree format to canonical git format
 		 */
-		if (convert_to_git(s->path, s->data, s->size, &buf, crlf_warn)) {
+		if (convert_to_git(&the_index, s->path, s->data, s->size, &buf, crlf_warn)) {
 			size_t size = 0;
 			munmap(s->data, s->size);
 			s->should_munmap = 0;
diff --git a/dir.c b/dir.c
index 3f3167e55..128fc2f8d 100644
--- a/dir.c
+++ b/dir.c
@@ -795,7 +795,7 @@ static int add_excludes(const char *fname, const char *base, int baselen,
 				 (pos = index_name_pos(istate, fname, strlen(fname))) >= 0 &&
 				 !ce_stage(istate->cache[pos]) &&
 				 ce_uptodate(istate->cache[pos]) &&
-				 !would_convert_to_git(fname))
+				 !would_convert_to_git(istate, fname))
 				hashcpy(sha1_stat->sha1,
 					istate->cache[pos]->oid.hash);
 			else
diff --git a/sha1_file.c b/sha1_file.c
index ab09241d2..feb227a83 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3546,7 +3546,7 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,
 	 */
 	if ((type == OBJ_BLOB) && path) {
 		struct strbuf nbuf = STRBUF_INIT;
-		if (convert_to_git(path, buf, size, &nbuf,
+		if (convert_to_git(&the_index, path, buf, size, &nbuf,
 				   write_object ? safe_crlf : SAFE_CRLF_FALSE)) {
 			buf = strbuf_detach(&nbuf, &size);
 			re_allocated = 1;
@@ -3668,7 +3668,7 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,
 	else if (!S_ISREG(st->st_mode))
 		ret = index_pipe(sha1, fd, type, path, flags);
 	else if (st->st_size <= big_file_threshold || type != OBJ_BLOB ||
-		 (path && would_convert_to_git(path)))
+		 (path && would_convert_to_git(&the_index, path)))
 		ret = index_core(sha1, fd, xsize_t(st->st_size), type, path,
 				 flags);
 	else
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 05/23] convert: convert renormalize_buffer to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 convert.c         | 6 ++++--
 convert.h         | 3 ++-
 ll-merge.c        | 2 +-
 merge-recursive.c | 4 ++--
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/convert.c b/convert.c
index 3d1c7e2b6..e4c8e6ad3 100644
--- a/convert.c
+++ b/convert.c
@@ -1,3 +1,4 @@
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "attr.h"
 #include "run-command.h"
@@ -1260,14 +1261,15 @@ int convert_to_working_tree(const char *path, const char *src, size_t len, struc
 	return convert_to_working_tree_internal(path, src, len, dst, 0);
 }
 
-int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst)
+int renormalize_buffer(const struct index_state *istate, const char *path,
+		       const char *src, size_t len, struct strbuf *dst)
 {
 	int ret = convert_to_working_tree_internal(path, src, len, dst, 1);
 	if (ret) {
 		src = dst->buf;
 		len = dst->len;
 	}
-	return ret | convert_to_git(&the_index, path, src, len, dst, SAFE_CRLF_RENORMALIZE);
+	return ret | convert_to_git(istate, path, src, len, dst, SAFE_CRLF_RENORMALIZE);
 }
 
 /*****************************************************************
diff --git a/convert.h b/convert.h
index 60cb41d6a..cecf59d1a 100644
--- a/convert.h
+++ b/convert.h
@@ -46,7 +46,8 @@ extern int convert_to_git(const struct index_state *istate,
 			  struct strbuf *dst, enum safe_crlf checksafe);
 extern int convert_to_working_tree(const char *path, const char *src,
 				   size_t len, struct strbuf *dst);
-extern int renormalize_buffer(const char *path, const char *src, size_t len,
+extern int renormalize_buffer(const struct index_state *istate,
+			      const char *path, const char *src, size_t len,
 			      struct strbuf *dst);
 static inline int would_convert_to_git(const struct index_state *istate,
 				       const char *path)
diff --git a/ll-merge.c b/ll-merge.c
index ac0d4a5d7..d7eafb61a 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -339,7 +339,7 @@ static const struct ll_merge_driver *find_ll_merge_driver(const char *merge_attr
 static void normalize_file(mmfile_t *mm, const char *path)
 {
 	struct strbuf strbuf = STRBUF_INIT;
-	if (renormalize_buffer(path, mm->ptr, mm->size, &strbuf)) {
+	if (renormalize_buffer(&the_index, path, mm->ptr, mm->size, &strbuf)) {
 		free(mm->ptr);
 		mm->size = strbuf.len;
 		mm->ptr = strbuf_detach(&strbuf, NULL);
diff --git a/merge-recursive.c b/merge-recursive.c
index 62decd51c..cdf34c524 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1639,8 +1639,8 @@ static int blob_unchanged(struct merge_options *opt,
 	 * performed.  Comparison can be skipped if both files are
 	 * unchanged since their sha1s have already been compared.
 	 */
-	if (renormalize_buffer(path, o.buf, o.len, &o) |
-	    renormalize_buffer(path, a.buf, a.len, &a))
+	if (renormalize_buffer(&the_index, path, o.buf, o.len, &o) |
+	    renormalize_buffer(&the_index, path, a.buf, a.len, &a))
 		ret = (o.len == a.len && !memcmp(o.buf, a.buf, o.len));
 
 error_return:
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 06/23] tree: convert read_tree to take an index parameter

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c |  2 +-
 tree.c             | 28 ++++++++++++++++++----------
 tree.h             |  3 ++-
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 770d8774a..a4ced5a9c 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -460,7 +460,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 			       PATHSPEC_PREFER_CWD, prefix, matchbuf);
 	} else
 		memset(&pathspec, 0, sizeof(pathspec));
-	if (read_tree(tree, 1, &pathspec))
+	if (read_tree(tree, 1, &pathspec, &the_index))
 		die("unable to read tree entries %s", tree_name);
 
 	for (i = 0; i < active_nr; i++) {
diff --git a/tree.c b/tree.c
index ce345c551..cb5d40320 100644
--- a/tree.c
+++ b/tree.c
@@ -1,3 +1,4 @@
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "cache-tree.h"
 #include "tree.h"
@@ -8,7 +9,11 @@
 
 const char *tree_type = "tree";
 
-static int read_one_entry_opt(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, int opt)
+static int read_one_entry_opt(struct index_state *istate,
+			      const unsigned char *sha1,
+			      const char *base, int baselen,
+			      const char *pathname,
+			      unsigned mode, int stage, int opt)
 {
 	int len;
 	unsigned int size;
@@ -27,14 +32,15 @@ static int read_one_entry_opt(const unsigned char *sha1, const char *base, int b
 	memcpy(ce->name, base, baselen);
 	memcpy(ce->name + baselen, pathname, len+1);
 	hashcpy(ce->oid.hash, sha1);
-	return add_cache_entry(ce, opt);
+	return add_index_entry(istate, ce, opt);
 }
 
 static int read_one_entry(const unsigned char *sha1, struct strbuf *base,
 			  const char *pathname, unsigned mode, int stage,
 			  void *context)
 {
-	return read_one_entry_opt(sha1, base->buf, base->len, pathname,
+	struct index_state *istate = context;
+	return read_one_entry_opt(istate, sha1, base->buf, base->len, pathname,
 				  mode, stage,
 				  ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK);
 }
@@ -47,7 +53,8 @@ static int read_one_entry_quick(const unsigned char *sha1, struct strbuf *base,
 				const char *pathname, unsigned mode, int stage,
 				void *context)
 {
-	return read_one_entry_opt(sha1, base->buf, base->len, pathname,
+	struct index_state *istate = context;
+	return read_one_entry_opt(istate, sha1, base->buf, base->len, pathname,
 				  mode, stage,
 				  ADD_CACHE_JUST_APPEND);
 }
@@ -144,7 +151,8 @@ static int cmp_cache_name_compare(const void *a_, const void *b_)
 				  ce2->name, ce2->ce_namelen, ce_stage(ce2));
 }
 
-int read_tree(struct tree *tree, int stage, struct pathspec *match)
+int read_tree(struct tree *tree, int stage, struct pathspec *match,
+	      struct index_state *istate)
 {
 	read_tree_fn_t fn = NULL;
 	int i, err;
@@ -164,23 +172,23 @@ int read_tree(struct tree *tree, int stage, struct pathspec *match)
 	 * do it the original slow way, otherwise, append and then
 	 * sort at the end.
 	 */
-	for (i = 0; !fn && i < active_nr; i++) {
-		const struct cache_entry *ce = active_cache[i];
+	for (i = 0; !fn && i < istate->cache_nr; i++) {
+		const struct cache_entry *ce = istate->cache[i];
 		if (ce_stage(ce) == stage)
 			fn = read_one_entry;
 	}
 
 	if (!fn)
 		fn = read_one_entry_quick;
-	err = read_tree_recursive(tree, "", 0, stage, match, fn, NULL);
+	err = read_tree_recursive(tree, "", 0, stage, match, fn, istate);
 	if (fn == read_one_entry || err)
 		return err;
 
 	/*
 	 * Sort the cache entry -- we need to nuke the cache tree, though.
 	 */
-	cache_tree_free(&active_cache_tree);
-	QSORT(active_cache, active_nr, cmp_cache_name_compare);
+	cache_tree_free(&istate->cache_tree);
+	QSORT(istate->cache, istate->cache_nr, cmp_cache_name_compare);
 	return 0;
 }
 
diff --git a/tree.h b/tree.h
index d24786cba..99f5b7ec1 100644
--- a/tree.h
+++ b/tree.h
@@ -34,6 +34,7 @@ extern int read_tree_recursive(struct tree *tree,
 			       int stage, const struct pathspec *pathspec,
 			       read_tree_fn_t fn, void *context);
 
-extern int read_tree(struct tree *tree, int stage, struct pathspec *pathspec);
+extern int read_tree(struct tree *tree, int stage, struct pathspec *pathspec,
+		     struct index_state *istate);
 
 #endif /* TREE_H */
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 07/23] ls-files: convert overlay_tree_on_cache to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/commit.c   |  3 ++-
 builtin/ls-files.c | 15 ++++++++-------
 cache.h            |  3 ++-
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1d805f5da..38993bd24 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -253,7 +253,8 @@ static int list_paths(struct string_list *list, const char *with_tree,
 
 	if (with_tree) {
 		char *max_prefix = common_prefix(pattern);
-		overlay_tree_on_cache(with_tree, max_prefix ? max_prefix : prefix);
+		overlay_tree_on_index(&the_index, with_tree,
+				      max_prefix ? max_prefix : prefix);
 		free(max_prefix);
 	}
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a4ced5a9c..7d306f418 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -431,7 +431,8 @@ static int get_common_prefix_len(const char *common_prefix)
  * that were given from the command line.  We are not
  * going to write this index out.
  */
-void overlay_tree_on_cache(const char *tree_name, const char *prefix)
+void overlay_tree_on_index(struct index_state *istate,
+			   const char *tree_name, const char *prefix)
 {
 	struct tree *tree;
 	unsigned char sha1[20];
@@ -446,8 +447,8 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 		die("bad tree-ish %s", tree_name);
 
 	/* Hoist the unmerged entries up to stage #3 to make room */
-	for (i = 0; i < active_nr; i++) {
-		struct cache_entry *ce = active_cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		struct cache_entry *ce = istate->cache[i];
 		if (!ce_stage(ce))
 			continue;
 		ce->ce_flags |= CE_STAGEMASK;
@@ -460,11 +461,11 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 			       PATHSPEC_PREFER_CWD, prefix, matchbuf);
 	} else
 		memset(&pathspec, 0, sizeof(pathspec));
-	if (read_tree(tree, 1, &pathspec, &the_index))
+	if (read_tree(tree, 1, &pathspec, istate))
 		die("unable to read tree entries %s", tree_name);
 
-	for (i = 0; i < active_nr; i++) {
-		struct cache_entry *ce = active_cache[i];
+	for (i = 0; i < istate->cache_nr; i++) {
+		struct cache_entry *ce = istate->cache[i];
 		switch (ce_stage(ce)) {
 		case 0:
 			last_stage0 = ce;
@@ -679,7 +680,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		 */
 		if (show_stage || show_unmerged)
 			die("ls-files --with-tree is incompatible with -s or -u");
-		overlay_tree_on_cache(with_tree, max_prefix);
+		overlay_tree_on_index(&the_index, with_tree, max_prefix);
 	}
 	show_files(&dir);
 	if (show_resolve_undo)
diff --git a/cache.h b/cache.h
index e1f0e182a..175e58f01 100644
--- a/cache.h
+++ b/cache.h
@@ -2179,7 +2179,8 @@ extern int ws_blank_line(const char *line, int len, unsigned ws_rule);
 #define ws_tab_width(rule)     ((rule) & WS_TAB_WIDTH_MASK)
 
 /* ls-files */
-void overlay_tree_on_cache(const char *tree_name, const char *prefix);
+void overlay_tree_on_index(struct index_state *istate,
+			   const char *tree_name, const char *prefix);
 
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 08/23] ls-files: convert write_eolinfo to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 7d306f418..9313452e3 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -53,17 +53,16 @@ static const char *tag_modified = "";
 static const char *tag_skip_worktree = "";
 static const char *tag_resolve_undo = "";
 
-static void write_eolinfo(const struct cache_entry *ce, const char *path)
+static void write_eolinfo(const struct index_state *istate,
+			  const struct cache_entry *ce, const char *path)
 {
-	if (!show_eol)
-		return;
-	else {
+	if (show_eol) {
 		struct stat st;
 		const char *i_txt = "";
 		const char *w_txt = "";
 		const char *a_txt = get_convert_attr_ascii(path);
 		if (ce && S_ISREG(ce->ce_mode))
-			i_txt = get_cached_convert_stats_ascii(&the_index,
+			i_txt = get_cached_convert_stats_ascii(istate,
 							       ce->name);
 		if (!lstat(path, &st) && S_ISREG(st.st_mode))
 			w_txt = get_wt_convert_stats_ascii(path);
@@ -105,7 +104,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
 		return;
 
 	fputs(tag, stdout);
-	write_eolinfo(NULL, ent->name);
+	write_eolinfo(NULL, NULL, ent->name);
 	write_name(ent->name);
 }
 
@@ -275,7 +274,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 			       find_unique_abbrev(ce->oid.hash, abbrev),
 			       ce_stage(ce));
 		}
-		write_eolinfo(ce, ce->name);
+		write_eolinfo(&the_index, ce, ce->name);
 		write_name(ce->name);
 		if (debug_mode) {
 			const struct stat_data *sd = &ce->ce_stat_data;
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 09/23] ls-files: convert show_killed_files to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 9313452e3..7cdee2359 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -120,7 +120,8 @@ static void show_other_files(struct dir_struct *dir)
 	}
 }
 
-static void show_killed_files(struct dir_struct *dir)
+static void show_killed_files(const struct index_state *istate,
+			      const struct dir_struct *dir)
 {
 	int i;
 	for (i = 0; i < dir->nr; i++) {
@@ -134,29 +135,29 @@ static void show_killed_files(struct dir_struct *dir)
 				/* If ent->name is prefix of an entry in the
 				 * cache, it will be killed.
 				 */
-				pos = cache_name_pos(ent->name, ent->len);
+				pos = index_name_pos(istate, ent->name, ent->len);
 				if (0 <= pos)
 					die("BUG: killed-file %.*s not found",
 						ent->len, ent->name);
 				pos = -pos - 1;
-				while (pos < active_nr &&
-				       ce_stage(active_cache[pos]))
+				while (pos < istate->cache_nr &&
+				       ce_stage(istate->cache[pos]))
 					pos++; /* skip unmerged */
-				if (active_nr <= pos)
+				if (istate->cache_nr <= pos)
 					break;
 				/* pos points at a name immediately after
 				 * ent->name in the cache.  Does it expect
 				 * ent->name to be a directory?
 				 */
-				len = ce_namelen(active_cache[pos]);
+				len = ce_namelen(istate->cache[pos]);
 				if ((ent->len < len) &&
-				    !strncmp(active_cache[pos]->name,
+				    !strncmp(istate->cache[pos]->name,
 					     ent->name, ent->len) &&
-				    active_cache[pos]->name[ent->len] == '/')
+				    istate->cache[pos]->name[ent->len] == '/')
 					killed = 1;
 				break;
 			}
-			if (0 <= cache_name_pos(ent->name, sp - ent->name)) {
+			if (0 <= index_name_pos(istate, ent->name, sp - ent->name)) {
 				/* If any of the leading directories in
 				 * ent->name is registered in the cache,
 				 * ent->name will be killed.
@@ -337,7 +338,7 @@ static void show_files(struct dir_struct *dir)
 		if (show_others)
 			show_other_files(dir);
 		if (show_killed)
-			show_killed_files(dir);
+			show_killed_files(&the_index, dir);
 	}
 	if (show_cached || show_stage) {
 		for (i = 0; i < active_nr; i++) {
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 10/23] ls-files: convert show_other_files to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 7cdee2359..3d4e497cc 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -108,13 +108,14 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
 	write_name(ent->name);
 }
 
-static void show_other_files(struct dir_struct *dir)
+static void show_other_files(const struct index_state *istate,
+			     const struct dir_struct *dir)
 {
 	int i;
 
 	for (i = 0; i < dir->nr; i++) {
 		struct dir_entry *ent = dir->entries[i];
-		if (!cache_name_is_other(ent->name, ent->len))
+		if (!index_name_is_other(istate, ent->name, ent->len))
 			continue;
 		show_dir_entry(tag_other, ent);
 	}
@@ -336,7 +337,7 @@ static void show_files(struct dir_struct *dir)
 			dir->flags |= DIR_COLLECT_KILLED_ONLY;
 		fill_directory(dir, &the_index, &pathspec);
 		if (show_others)
-			show_other_files(dir);
+			show_other_files(&the_index, dir);
 		if (show_killed)
 			show_killed_files(&the_index, dir);
 	}
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 11/23] ls-files: convert show_ru_info to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 3d4e497cc..755dfc8d6 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -292,14 +292,14 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 	strbuf_release(&name);
 }
 
-static void show_ru_info(void)
+static void show_ru_info(const struct index_state *istate)
 {
 	struct string_list_item *item;
 
-	if (!the_index.resolve_undo)
+	if (!istate->resolve_undo)
 		return;
 
-	for_each_string_list_item(item, the_index.resolve_undo) {
+	for_each_string_list_item(item, istate->resolve_undo) {
 		const char *path = item->string;
 		struct resolve_undo_info *ui = item->util;
 		int i, len;
@@ -685,7 +685,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	}
 	show_files(&dir);
 	if (show_resolve_undo)
-		show_ru_info();
+		show_ru_info(&the_index);
 
 	if (ps_matched) {
 		int bad;
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 12/23] ls-files: convert ce_excluded to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 755dfc8d6..de02819c6 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -321,10 +321,11 @@ static void show_ru_info(const struct index_state *istate)
 	}
 }
 
-static int ce_excluded(struct dir_struct *dir, const struct cache_entry *ce)
+static int ce_excluded(struct dir_struct *dir, struct index_state *istate,
+		       const struct cache_entry *ce)
 {
 	int dtype = ce_to_dtype(ce);
-	return is_excluded(dir, &the_index, ce->name, &dtype);
+	return is_excluded(dir, istate, ce->name, &dtype);
 }
 
 static void show_files(struct dir_struct *dir)
@@ -345,7 +346,7 @@ static void show_files(struct dir_struct *dir)
 		for (i = 0; i < active_nr; i++) {
 			const struct cache_entry *ce = active_cache[i];
 			if ((dir->flags & DIR_SHOW_IGNORED) &&
-			    !ce_excluded(dir, ce))
+			    !ce_excluded(dir, &the_index, ce))
 				continue;
 			if (show_unmerged && !ce_stage(ce))
 				continue;
@@ -361,7 +362,7 @@ static void show_files(struct dir_struct *dir)
 			struct stat st;
 			int err;
 			if ((dir->flags & DIR_SHOW_IGNORED) &&
-			    !ce_excluded(dir, ce))
+			    !ce_excluded(dir, &the_index, ce))
 				continue;
 			if (ce->ce_flags & CE_UPDATE)
 				continue;
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 13/23] ls-files: convert prune_cache to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index de02819c6..8448b04e8 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -380,30 +380,31 @@ static void show_files(struct dir_struct *dir)
 /*
  * Prune the index to only contain stuff starting with "prefix"
  */
-static void prune_cache(const char *prefix, size_t prefixlen)
+static void prune_index(struct index_state *istate,
+			const char *prefix, size_t prefixlen)
 {
 	int pos;
 	unsigned int first, last;
 
 	if (!prefix)
 		return;
-	pos = cache_name_pos(prefix, prefixlen);
+	pos = index_name_pos(istate, prefix, prefixlen);
 	if (pos < 0)
 		pos = -pos-1;
 	first = pos;
-	last = active_nr;
+	last = istate->cache_nr;
 	while (last > first) {
 		int next = (last + first) >> 1;
-		const struct cache_entry *ce = active_cache[next];
+		const struct cache_entry *ce = istate->cache[next];
 		if (!strncmp(ce->name, prefix, prefixlen)) {
 			first = next+1;
 			continue;
 		}
 		last = next;
 	}
-	memmove(active_cache, active_cache + pos,
+	memmove(istate->cache, istate->cache + pos,
 		(last - pos) * sizeof(struct cache_entry *));
-	active_nr = last - pos;
+	istate->cache_nr = last - pos;
 }
 
 static int get_common_prefix_len(const char *common_prefix)
@@ -661,7 +662,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		max_prefix = common_prefix(&pathspec);
 	max_prefix_len = get_common_prefix_len(max_prefix);
 
-	prune_cache(max_prefix, max_prefix_len);
+	prune_index(&the_index, max_prefix, max_prefix_len);
 
 	/* Treat unmatching pathspec elements as errors */
 	if (pathspec.nr && error_unmatch)
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 14/23] ls-files: convert show_files to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 8448b04e8..56fd6644f 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -328,7 +328,7 @@ static int ce_excluded(struct dir_struct *dir, struct index_state *istate,
 	return is_excluded(dir, istate, ce->name, &dtype);
 }
 
-static void show_files(struct dir_struct *dir)
+static void show_files(struct index_state *istate, struct dir_struct *dir)
 {
 	int i;
 
@@ -336,17 +336,17 @@ static void show_files(struct dir_struct *dir)
 	if (show_others || show_killed) {
 		if (!show_others)
 			dir->flags |= DIR_COLLECT_KILLED_ONLY;
-		fill_directory(dir, &the_index, &pathspec);
+		fill_directory(dir, istate, &pathspec);
 		if (show_others)
-			show_other_files(&the_index, dir);
+			show_other_files(istate, dir);
 		if (show_killed)
-			show_killed_files(&the_index, dir);
+			show_killed_files(istate, dir);
 	}
 	if (show_cached || show_stage) {
-		for (i = 0; i < active_nr; i++) {
-			const struct cache_entry *ce = active_cache[i];
+		for (i = 0; i < istate->cache_nr; i++) {
+			const struct cache_entry *ce = istate->cache[i];
 			if ((dir->flags & DIR_SHOW_IGNORED) &&
-			    !ce_excluded(dir, &the_index, ce))
+			    !ce_excluded(dir, istate, ce))
 				continue;
 			if (show_unmerged && !ce_stage(ce))
 				continue;
@@ -357,12 +357,12 @@ static void show_files(struct dir_struct *dir)
 		}
 	}
 	if (show_deleted || show_modified) {
-		for (i = 0; i < active_nr; i++) {
-			const struct cache_entry *ce = active_cache[i];
+		for (i = 0; i < istate->cache_nr; i++) {
+			const struct cache_entry *ce = istate->cache[i];
 			struct stat st;
 			int err;
 			if ((dir->flags & DIR_SHOW_IGNORED) &&
-			    !ce_excluded(dir, &the_index, ce))
+			    !ce_excluded(dir, istate, ce))
 				continue;
 			if (ce->ce_flags & CE_UPDATE)
 				continue;
@@ -371,7 +371,7 @@ static void show_files(struct dir_struct *dir)
 			err = lstat(ce->name, &st);
 			if (show_deleted && err)
 				show_ce_entry(tag_removed, ce);
-			if (show_modified && ce_modified(ce, &st, 0))
+			if (show_modified && ie_modified(istate, ce, &st, 0))
 				show_ce_entry(tag_modified, ce);
 		}
 	}
@@ -685,7 +685,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 			die("ls-files --with-tree is incompatible with -s or -u");
 		overlay_tree_on_index(&the_index, with_tree, max_prefix);
 	}
-	show_files(&dir);
+	show_files(&the_index, &dir);
 	if (show_resolve_undo)
 		show_ru_info(&the_index);
 
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 15/23] ls-files: factor out debug info into a function

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 56fd6644f..6603538ec 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -93,6 +93,19 @@ static void write_name(const char *name)
 	strbuf_release(&full_name);
 }
 
+static void print_debug(const struct cache_entry *ce)
+{
+	if (debug_mode) {
+		const struct stat_data *sd = &ce->ce_stat_data;
+
+		printf("  ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
+		printf("  mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
+		printf("  dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
+		printf("  uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
+		printf("  size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
+	}
+}
+
 static void show_dir_entry(const char *tag, struct dir_entry *ent)
 {
 	int len = max_prefix_len;
@@ -278,15 +291,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 		}
 		write_eolinfo(&the_index, ce, ce->name);
 		write_name(ce->name);
-		if (debug_mode) {
-			const struct stat_data *sd = &ce->ce_stat_data;
-
-			printf("  ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
-			printf("  mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
-			printf("  dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
-			printf("  uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
-			printf("  size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
-		}
+		print_debug(ce);
 	}
 
 	strbuf_release(&name);
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 16/23] ls-files: factor out tag calculation

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 6603538ec..456df61e4 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -93,6 +93,30 @@ static void write_name(const char *name)
 	strbuf_release(&full_name);
 }
 
+static const char *get_tag(const struct cache_entry *ce, const char *tag)
+{
+	static char alttag[4];
+
+	if (tag && *tag && show_valid_bit && (ce->ce_flags & CE_VALID)) {
+		memcpy(alttag, tag, 3);
+
+		if (isalpha(tag[0])) {
+			alttag[0] = tolower(tag[0]);
+		} else if (tag[0] == '?') {
+			alttag[0] = '!';
+		} else {
+			alttag[0] = 'v';
+			alttag[1] = tag[0];
+			alttag[2] = ' ';
+			alttag[3] = 0;
+		}
+
+		tag = alttag;
+	}
+
+	return tag;
+}
+
 static void print_debug(const struct cache_entry *ce)
 {
 	if (debug_mode) {
@@ -263,22 +287,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 				  len, ps_matched,
 				  S_ISDIR(ce->ce_mode) ||
 				  S_ISGITLINK(ce->ce_mode))) {
-		if (tag && *tag && show_valid_bit &&
-		    (ce->ce_flags & CE_VALID)) {
-			static char alttag[4];
-			memcpy(alttag, tag, 3);
-			if (isalpha(tag[0]))
-				alttag[0] = tolower(tag[0]);
-			else if (tag[0] == '?')
-				alttag[0] = '!';
-			else {
-				alttag[0] = 'v';
-				alttag[1] = tag[0];
-				alttag[2] = ' ';
-				alttag[3] = 0;
-			}
-			tag = alttag;
-		}
+		tag = get_tag(ce, tag);
 
 		if (!show_stage) {
 			fputs(tag, stdout);
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 17/23] repo: introduce new repository object

[Brandon Williams]

Introduce 'struct repo' an object used to represent a repository.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 Makefile |  1 +
 repo.c   | 42 ++++++++++++++++++++++++++++++++++++++++++
 repo.h   | 15 +++++++++++++++
 3 files changed, 58 insertions(+)
 create mode 100644 repo.c
 create mode 100644 repo.h

diff --git a/Makefile b/Makefile
index e35542e63..a49d2f96a 100644
--- a/Makefile
+++ b/Makefile
@@ -821,6 +821,7 @@ LIB_OBJS += refs/ref-cache.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace_object.o
+LIB_OBJS += repo.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
 LIB_OBJS += revision.o
diff --git a/repo.c b/repo.c
new file mode 100644
index 000000000..d47e98d95
--- /dev/null
+++ b/repo.c
@@ -0,0 +1,42 @@
+#include "cache.h"
+#include "repo.h"
+
+int
+repo_init(struct repo *repo, const char *gitdir, const char *worktree)
+{
+	int error = 0;
+	char *abspath = real_pathdup(gitdir, 1);
+	char *suspect = xstrfmt("%s/.git", abspath);
+	const char *resolved_gitdir;
+
+	memset(repo, 0, sizeof(struct repo));
+
+	/* First assume 'gitdir' references the gitdir directly */
+	resolved_gitdir = resolve_gitdir_gently(abspath, &error);
+	/* otherwise; try 'gitdir'.git */
+	if (!resolved_gitdir) {
+		resolved_gitdir = resolve_gitdir_gently(suspect, &error);
+		if (!resolved_gitdir) {
+			die("'%s' is not a repository\n", abspath);
+			return error;
+		}
+	}
+
+	repo->gitdir = xstrdup(resolved_gitdir);
+	/* Maybe need a check to verify that a worktree is indeed a worktree? */
+	repo->worktree = xstrdup_or_null(worktree);
+
+	free(abspath);
+	free(suspect);
+
+	return error;
+}
+
+void
+repo_clear(struct repo *repo)
+{
+	free(repo->gitdir);
+	repo->gitdir = NULL;
+	free(repo->worktree);
+	repo->worktree = NULL;
+}
diff --git a/repo.h b/repo.h
new file mode 100644
index 000000000..55f2dbec6
--- /dev/null
+++ b/repo.h
@@ -0,0 +1,15 @@
+#ifndef REPO_H
+#define REPO_H
+
+struct index_state;
+
+struct repo {
+	char *gitdir;
+	char *worktree;
+	const char *submodule_prefix;
+};
+
+extern int repo_init(struct repo *repo, const char *gitdir, const char *worktree);
+extern void repo_clear(struct repo *repo);
+
+#endif /* REPO_H */
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 18/23] repo: add index_state to struct repo

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 repo.c | 19 +++++++++++++++++++
 repo.h |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/repo.c b/repo.c
index d47e98d95..7e5c03ac5 100644
--- a/repo.c
+++ b/repo.c
@@ -2,6 +2,20 @@
 #include "repo.h"
 
 int
+repo_read_index(struct repo *repo, const char *index_file)
+{
+	char *index_path = xstrfmt("%s/index", repo->gitdir);
+	const char *file = index_file ? index_file : index_path;
+
+	repo->index = xcalloc(1, sizeof(struct index_state));
+	if (read_index_from(repo->index, file) < 0)
+		die("failure reading index\n");
+
+	free(index_path);
+	return 0;
+}
+
+int
 repo_init(struct repo *repo, const char *gitdir, const char *worktree)
 {
 	int error = 0;
@@ -39,4 +53,9 @@ repo_clear(struct repo *repo)
 	repo->gitdir = NULL;
 	free(repo->worktree);
 	repo->worktree = NULL;
+
+	if (repo->index) {
+		discard_index(repo->index);
+		free(repo->index);
+	}
 }
diff --git a/repo.h b/repo.h
index 55f2dbec6..15a0bdee9 100644
--- a/repo.h
+++ b/repo.h
@@ -7,8 +7,10 @@ struct repo {
 	char *gitdir;
 	char *worktree;
 	const char *submodule_prefix;
+	struct index_state *index;
 };
 
+extern int repo_read_index(struct repo *repo, const char *index_file);
 extern int repo_init(struct repo *repo, const char *gitdir, const char *worktree);
 extern void repo_clear(struct repo *repo);
 
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 19/23] repo: add per repo config

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 cache.h  |  1 +
 config.c |  2 +-
 repo.c   | 27 ++++++++++++++++++++++++++-
 repo.h   |  2 ++
 4 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index 175e58f01..af9ae1173 100644
--- a/cache.h
+++ b/cache.h
@@ -2015,6 +2015,7 @@ struct config_set {
 };
 
 extern void git_configset_init(struct config_set *cs);
+extern int config_set_callback(const char *key, const char *value, void *cb);
 extern int git_configset_add_file(struct config_set *cs, const char *filename);
 extern int git_configset_get_value(struct config_set *cs, const char *key, const char **value);
 extern const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key);
diff --git a/config.c b/config.c
index b4a3205da..d24baec50 100644
--- a/config.c
+++ b/config.c
@@ -1765,7 +1765,7 @@ void git_configset_clear(struct config_set *cs)
 	cs->list.items = NULL;
 }
 
-static int config_set_callback(const char *key, const char *value, void *cb)
+int config_set_callback(const char *key, const char *value, void *cb)
 {
 	struct config_set *cs = cb;
 	configset_add_value(cs, key, value);
diff --git a/repo.c b/repo.c
index 7e5c03ac5..223adf4c8 100644
--- a/repo.c
+++ b/repo.c
@@ -1,10 +1,16 @@
 #include "cache.h"
 #include "repo.h"
 
+char *
+repo_git_pathdup(const struct repo *repo, const char *file)
+{
+	return xstrfmt("%s/%s", repo->gitdir, file);
+}
+
 int
 repo_read_index(struct repo *repo, const char *index_file)
 {
-	char *index_path = xstrfmt("%s/index", repo->gitdir);
+	char *index_path = repo_git_pathdup(repo, "index");
 	const char *file = index_file ? index_file : index_path;
 
 	repo->index = xcalloc(1, sizeof(struct index_state));
@@ -16,6 +22,18 @@ repo_read_index(struct repo *repo, const char *index_file)
 }
 
 int
+repo_read_config(struct repo *repo)
+{
+	struct config_options opts = { 1, repo->gitdir };
+
+	repo->config = xcalloc(1, sizeof(struct config_set));
+	git_configset_init(repo->config);
+
+	return git_config_with_options(config_set_callback, repo->config,
+				       NULL, &opts);
+}
+
+int
 repo_init(struct repo *repo, const char *gitdir, const char *worktree)
 {
 	int error = 0;
@@ -40,6 +58,8 @@ repo_init(struct repo *repo, const char *gitdir, const char *worktree)
 	/* Maybe need a check to verify that a worktree is indeed a worktree? */
 	repo->worktree = xstrdup_or_null(worktree);
 
+	repo_read_config(repo);
+
 	free(abspath);
 	free(suspect);
 
@@ -58,4 +78,9 @@ repo_clear(struct repo *repo)
 		discard_index(repo->index);
 		free(repo->index);
 	}
+
+	if (repo->config) {
+		git_configset_clear(repo->config);
+		free(repo->config);
+	}
 }
diff --git a/repo.h b/repo.h
index 15a0bdee9..b4df774c3 100644
--- a/repo.h
+++ b/repo.h
@@ -2,12 +2,14 @@
 #define REPO_H
 
 struct index_state;
+struct config_set;
 
 struct repo {
 	char *gitdir;
 	char *worktree;
 	const char *submodule_prefix;
 	struct index_state *index;
+	struct config_set *config;
 };
 
 extern int repo_read_index(struct repo *repo, const char *index_file);
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 20/23] submodule-config: refactor to allow for multiple submodule_cache's

[Brandon Williams]

A repository object will have its own submodule cache so lay the ground
work for allowing multiple submodule cache structs.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 submodule-config.c | 40 ++++++++++++++++++++++++++++++++--------
 submodule-config.h | 10 ++++++++++
 2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 4f58491dd..666643d52 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -30,7 +30,7 @@ enum lookup_type {
 	lookup_path
 };
 
-static struct submodule_cache the_submodule_cache;
+struct submodule_cache the_submodule_cache;
 static int is_cache_init;
 
 static int config_path_cmp(const struct submodule_entry *a,
@@ -49,7 +49,12 @@ static int config_name_cmp(const struct submodule_entry *a,
 	       hashcmp(a->config->gitmodules_sha1, b->config->gitmodules_sha1);
 }
 
-static void cache_init(struct submodule_cache *cache)
+struct submodule_cache *submodule_cache_alloc(void)
+{
+	return xcalloc(1, sizeof(struct submodule_cache));
+}
+
+void submodule_cache_init(struct submodule_cache *cache)
 {
 	hashmap_init(&cache->for_path, (hashmap_cmp_fn) config_path_cmp, 0);
 	hashmap_init(&cache->for_name, (hashmap_cmp_fn) config_name_cmp, 0);
@@ -64,7 +69,7 @@ static void free_one_config(struct submodule_entry *entry)
 	free(entry->config);
 }
 
-static void cache_free(struct submodule_cache *cache)
+static void submodule_cache_clear(struct submodule_cache *cache)
 {
 	struct hashmap_iter iter;
 	struct submodule_entry *entry;
@@ -82,6 +87,12 @@ static void cache_free(struct submodule_cache *cache)
 	hashmap_free(&cache->for_name, 1);
 }
 
+void submodule_cache_free(struct submodule_cache *cache)
+{
+	submodule_cache_clear(cache);
+	free(cache);
+}
+
 static unsigned int hash_sha1_string(const unsigned char *sha1,
 				     const char *string)
 {
@@ -493,27 +504,40 @@ static const struct submodule *config_from(struct submodule_cache *cache,
 	return submodule;
 }
 
+const struct submodule *
+submodule_from_cache(struct submodule_cache *cache,
+		     const unsigned char *treeish_name,
+		     const char *key)
+{
+	return config_from(cache, treeish_name, key, lookup_path);
+}
+
 static void ensure_cache_init(void)
 {
 	if (is_cache_init)
 		return;
 
-	cache_init(&the_submodule_cache);
+	submodule_cache_init(&the_submodule_cache);
 	is_cache_init = 1;
 }
 
-int parse_submodule_config_option(const char *var, const char *value)
+int parse_submodule_config_option_cache(struct submodule_cache *cache, const char *var, const char *value)
 {
 	struct parse_config_parameter parameter;
-	parameter.cache = &the_submodule_cache;
+	parameter.cache = cache;
 	parameter.treeish_name = NULL;
 	parameter.gitmodules_sha1 = null_sha1;
 	parameter.overwrite = 1;
 
-	ensure_cache_init();
 	return parse_config(var, value, &parameter);
 }
 
+int parse_submodule_config_option(const char *var, const char *value)
+{
+	ensure_cache_init();
+	return parse_submodule_config_option_cache(&the_submodule_cache, var, value);
+}
+
 const struct submodule *submodule_from_name(const unsigned char *treeish_name,
 		const char *name)
 {
@@ -530,6 +554,6 @@ const struct submodule *submodule_from_path(const unsigned char *treeish_name,
 
 void submodule_free(void)
 {
-	cache_free(&the_submodule_cache);
+	submodule_cache_clear(&the_submodule_cache);
 	is_cache_init = 0;
 }
diff --git a/submodule-config.h b/submodule-config.h
index d434ecdb4..ed598aadd 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -22,14 +22,24 @@ struct submodule {
 	int recommend_shallow;
 };
 
+struct submodule_cache;
+
+extern struct submodule_cache *submodule_cache_alloc(void);
+extern void submodule_cache_init(struct submodule_cache *cache);
+extern void submodule_cache_free(struct submodule_cache *cache);
+
 extern int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
 extern int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
 extern int parse_push_recurse_submodules_arg(const char *opt, const char *arg);
 extern int parse_submodule_config_option(const char *var, const char *value);
+extern int parse_submodule_config_option_cache(struct submodule_cache *cache, const char *var, const char *value);
 extern const struct submodule *submodule_from_name(
 		const unsigned char *commit_or_tree, const char *name);
 extern const struct submodule *submodule_from_path(
 		const unsigned char *commit_or_tree, const char *path);
+extern const struct submodule *submodule_from_cache(struct submodule_cache *cache,
+						    const unsigned char *treeish_name,
+						    const char *key);
 extern int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
 				      unsigned char *gitmodules_sha1,
 				      struct strbuf *rev);
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 21/23] repo: add repo_read_gitmodules

[Brandon Williams]

Add ability for a repository to poulate its own submodule_cache by
reading the repository's gitmodules file.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 repo.c | 26 ++++++++++++++++++++++++++
 repo.h |  3 +++
 2 files changed, 29 insertions(+)

diff --git a/repo.c b/repo.c
index 223adf4c8..5449eb212 100644
--- a/repo.c
+++ b/repo.c
@@ -1,4 +1,6 @@
 #include "cache.h"
+#include "submodule.h"
+#include "submodule-config.h"
 #include "repo.h"
 
 char *
@@ -33,6 +35,27 @@ repo_read_config(struct repo *repo)
 				       NULL, &opts);
 }
 
+static int
+gitmodules_cb(const char *var, const char *value, void *data)
+{
+	struct repo *repo = data;
+	return parse_submodule_config_option_cache(repo->submodule_cache, var, value);
+}
+
+int
+repo_read_gitmodules(struct repo *repo)
+{
+	char *gitmodules_path = xstrfmt("%s/.gitmodules", repo->worktree);
+
+	if (!repo->worktree)
+		die("BUG: no worktree");
+
+	repo->submodule_cache = submodule_cache_alloc();
+	submodule_cache_init(repo->submodule_cache);
+	git_config_from_file(gitmodules_cb, gitmodules_path, repo);
+	return 0;
+}
+
 int
 repo_init(struct repo *repo, const char *gitdir, const char *worktree)
 {
@@ -83,4 +106,7 @@ repo_clear(struct repo *repo)
 		git_configset_clear(repo->config);
 		free(repo->config);
 	}
+
+	if (repo->submodule_cache)
+		submodule_cache_free(repo->submodule_cache);
 }
diff --git a/repo.h b/repo.h
index b4df774c3..9ff144957 100644
--- a/repo.h
+++ b/repo.h
@@ -10,10 +10,13 @@ struct repo {
 	const char *submodule_prefix;
 	struct index_state *index;
 	struct config_set *config;
+	struct submodule_cache *submodule_cache;
 };
 
 extern int repo_read_index(struct repo *repo, const char *index_file);
 extern int repo_init(struct repo *repo, const char *gitdir, const char *worktree);
 extern void repo_clear(struct repo *repo);
 
+extern int repo_read_gitmodules(struct repo *repo);
+
 #endif /* REPO_H */
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 22/23] submodule: add is_submodule_active

[Brandon Williams]

Add method which checks if a submodule is active given the provided
repository object.

NOTE: this is almost a copy-paste of is_submodule_initialized.  I tried
to convert is_submodule_initialized to take a submodule_cache and
config_set parameters but that turned out to break some other code paths
which I didn't want to change yet.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 submodule.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 submodule.h |  2 ++
 2 files changed, 53 insertions(+)

diff --git a/submodule.c b/submodule.c
index 80851d044..9a9c52292 100644
--- a/submodule.c
+++ b/submodule.c
@@ -16,6 +16,7 @@
 #include "quote.h"
 #include "remote.h"
 #include "worktree.h"
+#include "repo.h"
 
 static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
 static int config_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
@@ -270,6 +271,56 @@ int is_submodule_initialized(const char *path)
 	return ret;
 }
 
+int is_submodule_active(struct repo *repo, const char *path)
+{
+	int ret = 0;
+	char *key = NULL;
+	char *value = NULL;
+	const struct string_list *sl;
+	const struct submodule *module;
+
+	module = submodule_from_cache(repo->submodule_cache, null_sha1, path);
+
+	/* early return if there isn't a path->module mapping */
+	if (!module)
+		return 0;
+
+	/* submodule.<name>.active is set */
+	key = xstrfmt("submodule.%s.active", module->name);
+	if (!git_configset_get_bool(repo->config, key, &ret)) {
+		free(key);
+		return ret;
+	}
+	free(key);
+
+	/* submodule.active is set */
+	sl = git_configset_get_value_multi(repo->config, "submodule.active");
+	if (sl) {
+		struct pathspec ps;
+		struct argv_array args = ARGV_ARRAY_INIT;
+		const struct string_list_item *item;
+
+		for_each_string_list_item(item, sl) {
+			argv_array_push(&args, item->string);
+		}
+
+		parse_pathspec(&ps, 0, 0, NULL, args.argv);
+		ret = match_pathspec(&ps, path, strlen(path), 0, NULL, 1);
+
+		argv_array_clear(&args);
+		clear_pathspec(&ps);
+		return ret;
+	}
+
+	/* fallback to checking if the URL is set */
+	key = xstrfmt("submodule.%s.url", module->name);
+	ret = !git_configset_get_string(repo->config, key, &value);
+
+	free(value);
+	free(key);
+	return ret;
+}
+
 int is_submodule_populated_gently(const char *path, int *return_error_code)
 {
 	int ret = 0;
diff --git a/submodule.h b/submodule.h
index 266d81f1c..083f16ce5 100644
--- a/submodule.h
+++ b/submodule.h
@@ -5,6 +5,7 @@ struct diff_options;
 struct argv_array;
 struct oid_array;
 struct remote;
+struct repo;
 
 enum {
 	RECURSE_SUBMODULES_ONLY = -5,
@@ -42,6 +43,7 @@ extern int submodule_config(const char *var, const char *value, void *cb);
 extern void gitmodules_config(void);
 extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
 extern int is_submodule_initialized(const char *path);
+extern int is_submodule_active(struct repo *repo, const char *path);
 /*
  * Determine if a submodule has been populated at a given 'path' by checking if
  * the <path>/.git resolves to a valid git repository.
-- 
2.13.0.303.g4ebf302169-goog

[WIP/RFC 23/23] ls-files: use repository object

[Brandon Williams]

Convert ls-files to use a repository struct and recurse submodules
inprocess.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c                     | 217 +++++++++++++++------------------
 git.c                                  |   2 +-
 t/t3007-ls-files-recurse-submodules.sh |  39 ++++++
 3 files changed, 137 insertions(+), 121 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 456df61e4..98d448b4e 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -5,6 +5,7 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "quote.h"
 #include "dir.h"
@@ -16,6 +17,7 @@
 #include "pathspec.h"
 #include "run-command.h"
 #include "submodule.h"
+#include "repo.h"
 
 static int abbrev;
 static int show_deleted;
@@ -31,10 +33,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,24 +73,11 @@ 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)
@@ -209,101 +196,60 @@ 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 repo *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(const struct repo *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 repo submodule;
+	char *gitdir = mkpathdup("%s/%s", superproject->worktree, path);
+	repo_init(&submodule, gitdir, gitdir);
+
+	repo_read_index(&submodule, NULL);
+	repo_read_gitmodules(&submodule);
+
+	if (superproject->submodule_prefix)
+		submodule.submodule_prefix = xstrfmt("%s%s/", superproject->submodule_prefix, path);
+	else
+		submodule.submodule_prefix = xstrfmt("%s/", path);
+	show_files(&submodule, dir);
+
+	free((char *) submodule.submodule_prefix);
+	repo_clear(&submodule);
+	free(gitdir);
 }
 
-static void show_ce_entry(const char *tag, const struct cache_entry *ce)
+static void show_ce(struct repo *repo, struct dir_struct *dir,
+		    const struct cache_entry *ce, const char *fullname,
+		    const char *tag)
 {
-	struct strbuf name = STRBUF_INIT;
-	int len = max_prefix_len;
-	if (super_prefix)
-		strbuf_addstr(&name, super_prefix);
-	strbuf_addstr(&name, ce->name);
-
-	if (len > ce_namelen(ce))
+	if (max_prefix_len > strlen(fullname))
 		die("git ls-files: internal error - cache entry not superset of prefix");
 
 	if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
-	    submodule_path_match(&pathspec, name.buf, ps_matched)) {
-		show_gitlink(ce);
-	} else if (match_pathspec(&pathspec, name.buf, name.len,
-				  len, ps_matched,
+	    submodule_path_match(&pathspec, fullname, ps_matched) &&
+	    is_submodule_active(repo, ce->name)) {
+		show_submodule(repo, dir, ce->name);
+	} else if (match_pathspec(&pathspec, fullname, strlen(fullname),
+				  max_prefix_len, ps_matched,
 				  S_ISDIR(ce->ce_mode) ||
 				  S_ISGITLINK(ce->ce_mode))) {
 		tag = get_tag(ce, tag);
 
-		if (!show_stage) {
-			fputs(tag, stdout);
-		} else {
+		if (show_stage) {
 			printf("%s%06o %s %d\t",
 			       tag,
 			       ce->ce_mode,
 			       find_unique_abbrev(ce->oid.hash, abbrev),
 			       ce_stage(ce));
+		} else {
+			fputs(tag, stdout);
 		}
-		write_eolinfo(&the_index, ce, ce->name);
-		write_name(ce->name);
+
+		write_eolinfo(repo->index, ce, fullname);
+		write_name(fullname);
 		print_debug(ce);
 	}
-
-	strbuf_release(&name);
 }
 
 static void show_ru_info(const struct index_state *istate)
@@ -336,59 +282,80 @@ static void show_ru_info(const struct index_state *istate)
 }
 
 static int ce_excluded(struct dir_struct *dir, struct index_state *istate,
-		       const struct cache_entry *ce)
+		       const char *fullname, const struct cache_entry *ce)
 {
 	int dtype = ce_to_dtype(ce);
-	return is_excluded(dir, istate, ce->name, &dtype);
+	return is_excluded(dir, istate, fullname, &dtype);
 }
 
-static void show_files(struct index_state *istate, struct dir_struct *dir)
+static void construct_fullname(struct strbuf *out, const struct repo *repo,
+			       const struct cache_entry *ce)
+{
+	strbuf_reset(out);
+	if (repo->submodule_prefix)
+		strbuf_addstr(out, repo->submodule_prefix);
+	strbuf_addstr(out, ce->name);
+
+}
+
+static void show_files(struct repo *repo, struct dir_struct *dir)
 {
 	int i;
+	struct strbuf fullname = STRBUF_INIT;
 
 	/* For cached/deleted files we don't need to even do the readdir */
 	if (show_others || show_killed) {
 		if (!show_others)
 			dir->flags |= DIR_COLLECT_KILLED_ONLY;
-		fill_directory(dir, istate, &pathspec);
+		fill_directory(dir, repo->index, &pathspec);
 		if (show_others)
-			show_other_files(istate, dir);
+			show_other_files(repo->index, dir);
 		if (show_killed)
-			show_killed_files(istate, dir);
+			show_killed_files(repo->index, dir);
 	}
 	if (show_cached || show_stage) {
-		for (i = 0; i < istate->cache_nr; i++) {
-			const struct cache_entry *ce = istate->cache[i];
+		for (i = 0; i < repo->index->cache_nr; i++) {
+			const struct cache_entry *ce = repo->index->cache[i];
+
+			construct_fullname(&fullname, repo, ce);
+
 			if ((dir->flags & DIR_SHOW_IGNORED) &&
-			    !ce_excluded(dir, istate, ce))
+			    !ce_excluded(dir, repo->index, fullname.buf, ce))
 				continue;
 			if (show_unmerged && !ce_stage(ce))
 				continue;
 			if (ce->ce_flags & CE_UPDATE)
 				continue;
-			show_ce_entry(ce_stage(ce) ? tag_unmerged :
-				(ce_skip_worktree(ce) ? tag_skip_worktree : tag_cached), ce);
+			show_ce(repo, dir, ce, fullname.buf,
+				ce_stage(ce) ? tag_unmerged :
+				(ce_skip_worktree(ce) ? tag_skip_worktree :
+				 tag_cached));
 		}
 	}
 	if (show_deleted || show_modified) {
-		for (i = 0; i < istate->cache_nr; i++) {
-			const struct cache_entry *ce = istate->cache[i];
+		for (i = 0; i < repo->index->cache_nr; i++) {
+			const struct cache_entry *ce = repo->index->cache[i];
 			struct stat st;
 			int err;
+
+			construct_fullname(&fullname, repo, ce);
+
 			if ((dir->flags & DIR_SHOW_IGNORED) &&
-			    !ce_excluded(dir, istate, ce))
+			    !ce_excluded(dir, repo->index, fullname.buf, ce))
 				continue;
 			if (ce->ce_flags & CE_UPDATE)
 				continue;
 			if (ce_skip_worktree(ce))
 				continue;
-			err = lstat(ce->name, &st);
+			err = lstat(fullname.buf, &st);
 			if (show_deleted && err)
-				show_ce_entry(tag_removed, ce);
-			if (show_modified && ie_modified(istate, ce, &st, 0))
-				show_ce_entry(tag_modified, ce);
+				show_ce(repo, dir, ce, fullname.buf, tag_removed);
+			if (show_modified && ie_modified(repo->index, ce, &st, 0))
+				show_ce(repo, dir, ce, fullname.buf, tag_modified);
 		}
 	}
+
+	strbuf_release(&fullname);
 }
 
 /*
@@ -542,8 +509,9 @@ static int option_parse_exclude_standard(const struct option *opt,
 
 int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 {
+	struct repo main_repository;
 	int require_work_tree = 0, show_tag = 0, i;
-	const char *max_prefix;
+	char *max_prefix;
 	struct dir_struct dir;
 	struct exclude_list *el;
 	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
@@ -613,11 +581,11 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	prefix = cmd_prefix;
 	if (prefix)
 		prefix_len = strlen(prefix);
-	super_prefix = get_super_prefix();
 	git_config(git_default_config, NULL);
 
-	if (read_cache() < 0)
-		die("index file corrupt");
+	repo_init(&main_repository, get_git_dir(), get_git_work_tree());
+
+	repo_read_index(&main_repository, get_index_file());
 
 	argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
 			ls_files_usage, 0);
@@ -649,12 +617,14 @@ 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(&main_repository);
+	}
 
 	if (recurse_submodules &&
 	    (show_stage || show_deleted || show_others || show_unmerged ||
-	     show_killed || show_modified || show_resolve_undo || with_tree))
+	     show_killed || show_modified || show_resolve_undo || with_tree ||
+	     show_eol))
 		die("ls-files --recurse-submodules unsupported mode");
 
 	if (recurse_submodules && error_unmatch)
@@ -668,7 +638,10 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	/*
 	 * Find common prefix for all pathspec's
 	 * This is used as a performance optimization which unfortunately cannot
-	 * be done when recursing into submodules
+	 * be done when recursing into submodules because when a pathspec is
+	 * given which spans repository boundaries you can't simply remove the
+	 * submodule entry because the pathspec may match something inside the
+	 * submodule.
 	 */
 	if (recurse_submodules)
 		max_prefix = NULL;
@@ -676,7 +649,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		max_prefix = common_prefix(&pathspec);
 	max_prefix_len = get_common_prefix_len(max_prefix);
 
-	prune_index(&the_index, max_prefix, max_prefix_len);
+	prune_index(main_repository.index, max_prefix, max_prefix_len);
 
 	/* Treat unmatching pathspec elements as errors */
 	if (pathspec.nr && error_unmatch)
@@ -697,11 +670,15 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		 */
 		if (show_stage || show_unmerged)
 			die("ls-files --with-tree is incompatible with -s or -u");
-		overlay_tree_on_index(&the_index, with_tree, max_prefix);
+		overlay_tree_on_index(main_repository.index, with_tree, max_prefix);
 	}
-	show_files(&the_index, &dir);
+
+	show_files(&main_repository, &dir);
+
 	if (show_resolve_undo)
-		show_ru_info(&the_index);
+		show_ru_info(main_repository.index);
+
+	repo_clear(&main_repository);
 
 	if (ps_matched) {
 		int bad;
diff --git a/git.c b/git.c
index 8ff44f081..8852ae944 100644
--- a/git.c
+++ b/git.c
@@ -441,7 +441,7 @@ static struct cmd_struct commands[] = {
 	{ "init-db", cmd_init_db },
 	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
 	{ "log", cmd_log, RUN_SETUP },
-	{ "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },
+	{ "ls-files", cmd_ls_files, RUN_SETUP },
 	{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
 	{ "ls-tree", cmd_ls_tree, RUN_SETUP },
 	{ "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY },
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
index ebb956fd1..318b5bce7 100755
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -135,6 +135,45 @@ test_expect_success '--recurse-submodules and pathspecs setup' '
 	test_cmp expect actual
 '
 
+test_expect_success 'inactive submodule' '
+	test_when_finished "git config --bool submodule.submodule.active true" &&
+	test_when_finished "git -C submodule config --bool submodule.subsub.active true" &&
+	git config --bool submodule.submodule.active "false" &&
+
+	cat >expect <<-\EOF &&
+	.gitmodules
+	a
+	b/b
+	h.txt
+	sib/file
+	sub/file
+	submodule
+	EOF
+
+	git ls-files --recurse-submodules >actual &&
+	test_cmp expect actual &&
+
+	git config --bool submodule.submodule.active "true" &&
+	git -C submodule config --bool submodule.subsub.active "false" &&
+
+	cat >expect <<-\EOF &&
+	.gitmodules
+	a
+	b/b
+	h.txt
+	sib/file
+	sub/file
+	submodule/.gitmodules
+	submodule/c
+	submodule/f.TXT
+	submodule/g.txt
+	submodule/subsub
+	EOF
+
+	git ls-files --recurse-submodules >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '--recurse-submodules and pathspecs' '
 	cat >expect <<-\EOF &&
 	h.txt
-- 
2.13.0.303.g4ebf302169-goog

Re: [WIP/RFC 00/23] repository object

[Jeff Hostetler]

On 5/18/2017 7:21 PM, Brandon Williams wrote:
> When I first started working on the git project I found it very difficult to
> understand parts of the code base because of the inherently global nature of
> our code.  It also made working on submodules very difficult.  Since we can
> only open up a single repository per process, you need to launch a child
> process in order to process a submodule.  But you also need to be able to
> communicate other stateful information to the children processes so that the
> submodules know how best to format their output or match against a
> pathspec...it ends up feeling like layering on hack after hack.  What I would
> really like to do, is to have the ability to have a repository object so that I
> can open a submodule in-process.
> 
> Before this becomes a reality for all commands, much of the library code would
> need to be refactored in order to work purely on handles instead of global
> state.  As it turned out, ls-files is a pretty simple command and doesn't have
> *too* many dependencies.  The biggest thing that needed to be changed was
> piping through an index into a couple library routines so that they don't
> inherently rely on 'the_index'.  A few of these changes I've sent out and can
> be found at 'origin/bw/pathspec-sans-the-index' and
> 'origin/bw/dir-c-stops-relying-on-the-index' which this series is based on.
> 
> Patches 1-16 are refactorings to prepare either library code or ls-files itself
> to be ready to handle passing around an index struct.  Patches 17-22 introduce
> a repository struct and change a couple of things about how submodule caches
> work (getting submodule information from .gitmodules).  And Patch 23 converts
> ls-files to use a repository struct.
> 
> The most interesting part of the series is from 17-23.  And 1-16 could be taken
> as is without the rest of the series.
> 
> This is still very much in a WIP state, though it does pass all tests.  What
> I'm hoping for here is to get a discussion started about the feasibility of a
> change like this and hopefully to get the ball rolling.  Is this a direction we
> want to move in?  Is it worth the pain?
> 
> Thanks for taking the time to look at this and entertain my insane ideas :)

Very nice and thanks for starting this.
Jeff

Re: [WIP/RFC 00/23] repository object

[Ben Peart]

Glad to see you tackling this.  This is definitely a step in the right 
direction.

I realize that it will take a lot of work and that intermediate steps 
may just be pushing it the global state one level higher but eventually 
it would be great to see an entire code path global state free!

I'm personally interested because reducing the reliance on global state 
also helps us in our performance work as it makes it more possible to 
use threading to scale up the performance.

Ben

On 5/18/2017 7:21 PM, Brandon Williams wrote:
> When I first started working on the git project I found it very difficult to
> understand parts of the code base because of the inherently global nature of
> our code.  It also made working on submodules very difficult.  Since we can
> only open up a single repository per process, you need to launch a child
> process in order to process a submodule.  But you also need to be able to
> communicate other stateful information to the children processes so that the
> submodules know how best to format their output or match against a
> pathspec...it ends up feeling like layering on hack after hack.  What I would
> really like to do, is to have the ability to have a repository object so that I
> can open a submodule in-process.
>
> Before this becomes a reality for all commands, much of the library code would
> need to be refactored in order to work purely on handles instead of global
> state.  As it turned out, ls-files is a pretty simple command and doesn't have
> *too* many dependencies.  The biggest thing that needed to be changed was
> piping through an index into a couple library routines so that they don't
> inherently rely on 'the_index'.  A few of these changes I've sent out and can
> be found at 'origin/bw/pathspec-sans-the-index' and
> 'origin/bw/dir-c-stops-relying-on-the-index' which this series is based on.
>
> Patches 1-16 are refactorings to prepare either library code or ls-files itself
> to be ready to handle passing around an index struct.  Patches 17-22 introduce
> a repository struct and change a couple of things about how submodule caches
> work (getting submodule information from .gitmodules).  And Patch 23 converts
> ls-files to use a repository struct.
>
> The most interesting part of the series is from 17-23.  And 1-16 could be taken
> as is without the rest of the series.
>
> This is still very much in a WIP state, though it does pass all tests.  What
> I'm hoping for here is to get a discussion started about the feasibility of a
> change like this and hopefully to get the ball rolling.  Is this a direction we
> want to move in?  Is it worth the pain?
>
> Thanks for taking the time to look at this and entertain my insane ideas :)
>
> Brandon Williams (23):
>   convert: convert get_cached_convert_stats_ascii to take an index
>   convert: convert crlf_to_git to take an index
>   convert: convert convert_to_git_filter_fd to take an index
>   convert: convert convert_to_git to take an index
>   convert: convert renormalize_buffer to take an index
>   tree: convert read_tree to take an index parameter
>   ls-files: convert overlay_tree_on_cache to take an index
>   ls-files: convert write_eolinfo to take an index
>   ls-files: convert show_killed_files to take an index
>   ls-files: convert show_other_files to take an index
>   ls-files: convert show_ru_info to take an index
>   ls-files: convert ce_excluded to take an index
>   ls-files: convert prune_cache to take an index
>   ls-files: convert show_files to take an index
>   ls-files: factor out debug info into a function
>   ls-files: factor out tag calculation
>   repo: introduce new repository object
>   repo: add index_state to struct repo
>   repo: add per repo config
>   submodule-config: refactor to allow for multiple submodule_cache's
>   repo: add repo_read_gitmodules
>   submodule: add is_submodule_active
>   ls-files: use repository object
>
>  Makefile                               |   1 +
>  apply.c                                |   2 +-
>  builtin/blame.c                        |   2 +-
>  builtin/commit.c                       |   3 +-
>  builtin/ls-files.c                     | 348 ++++++++++++++++-----------------
>  cache.h                                |   4 +-
>  combine-diff.c                         |   2 +-
>  config.c                               |   2 +-
>  convert.c                              |  31 +--
>  convert.h                              |  19 +-
>  diff.c                                 |   6 +-
>  dir.c                                  |   2 +-
>  git.c                                  |   2 +-
>  ll-merge.c                             |   2 +-
>  merge-recursive.c                      |   4 +-
>  repo.c                                 | 112 +++++++++++
>  repo.h                                 |  22 +++
>  sha1_file.c                            |   6 +-
>  submodule-config.c                     |  40 +++-
>  submodule-config.h                     |  10 +
>  submodule.c                            |  51 +++++
>  submodule.h                            |   2 +
>  t/t3007-ls-files-recurse-submodules.sh |  39 ++++
>  tree.c                                 |  28 ++-
>  tree.h                                 |   3 +-
>  25 files changed, 513 insertions(+), 230 deletions(-)
>  create mode 100644 repo.c
>  create mode 100644 repo.h
>

Re: [WIP/RFC 17/23] repo: introduce new repository object

[Stefan Beller]

On Thu, May 18, 2017 at 4:21 PM, Brandon Williams <bmwill@google.com> wrote:
> Introduce 'struct repo' an object used to represent a repository.

Is this the right place to outline what you expect from a repo object?
Are you planning to use it everywhere?
Is it lazy-init'd and it takes care of it itself, or would the caller
have to take
care of the state of the repo? ("the repo object is just a place to put the
current globals")

>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  Makefile |  1 +
>  repo.c   | 42 ++++++++++++++++++++++++++++++++++++++++++
>  repo.h   | 15 +++++++++++++++
>  3 files changed, 58 insertions(+)
>  create mode 100644 repo.c
>  create mode 100644 repo.h
>
> diff --git a/Makefile b/Makefile
> index e35542e63..a49d2f96a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -821,6 +821,7 @@ LIB_OBJS += refs/ref-cache.o
>  LIB_OBJS += ref-filter.o
>  LIB_OBJS += remote.o
>  LIB_OBJS += replace_object.o
> +LIB_OBJS += repo.o
>  LIB_OBJS += rerere.o
>  LIB_OBJS += resolve-undo.o
>  LIB_OBJS += revision.o
> diff --git a/repo.c b/repo.c
> new file mode 100644
> index 000000000..d47e98d95
> --- /dev/null
> +++ b/repo.c
> @@ -0,0 +1,42 @@
> +#include "cache.h"
> +#include "repo.h"
> +
> +int
> +repo_init(struct repo *repo, const char *gitdir, const char *worktree)

style ;)


> +       /* Maybe need a check to verify that a worktree is indeed a worktree? */

add NEEDSWORK/FIXME prefix to comment?

> +void
> +repo_clear(struct repo *repo)

style ;)

Re: [WIP/RFC 18/23] repo: add index_state to struct repo

[Stefan Beller]

>  int
> +repo_read_index(struct repo *repo, const char *index_file)
...
> +
> +int
>  repo_init(struct repo *repo, const char *gitdir, const char *worktree)

The version 2.13.0.303.g4ebf302169-goog doesn't have diff slider
heuristics on by default, and you also did not enable it?
(I am curious if the heuristics would have helped here)

Re: [WIP/RFC 00/23] repository object

[Stefan Beller]

On Thu, May 18, 2017 at 4:21 PM, Brandon Williams <bmwill@google.com> wrote:

> This is still very much in a WIP state, though it does pass all tests.  What
> I'm hoping for here is to get a discussion started about the feasibility of a
> change like this and hopefully to get the ball rolling.  Is this a direction we
> want to move in?  Is it worth the pain?

I would be really happy to see this series land eventually.

The introduction of a repo object will deliver performance at a higher
level, such as
* (remarked by Ben): enabling of threading
* submodules do not need to spawn processes
* I would imagine that developer velocity will go up by having less global state
  in the long run.

Thanks for working on this.
Stefan

Re: [WIP/RFC 00/23] repository object

[Jacob Keller]

On Thu, May 18, 2017 at 4:21 PM, Brandon Williams <bmwill@google.com> wrote:
> This is still very much in a WIP state, though it does pass all tests.  What
> I'm hoping for here is to get a discussion started about the feasibility of a
> change like this and hopefully to get the ball rolling.  Is this a direction we
> want to move in?  Is it worth the pain?
>
> Thanks for taking the time to look at this and entertain my insane ideas :)
>

I haven't had time to read the patches yet, but the goal I think is
worthy, and worth the pain. The ultimate goal allows us to more easily
write submodule features (some of which are quite difficult today, due
to having to pass in all kinds of state to the submodule, and
launching a process is expensive when you have a lot of them).

I can't say much about this particular code yet, but I hope to be able
to look in more detail in the next few days.

Thanks,
Jake

Re: [WIP/RFC 00/23] repository object

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 808 bytes --]

On Thu, May 18, 2017 at 04:21:11PM -0700, Brandon Williams wrote:
> This is still very much in a WIP state, though it does pass all tests.  What
> I'm hoping for here is to get a discussion started about the feasibility of a
> change like this and hopefully to get the ball rolling.  Is this a direction we
> want to move in?  Is it worth the pain?

I haven't spent a lot of time looking at the patches, but I like this
idea.  I expect that moving in this direction will help us avoid a lot
of globals and move us in a direction of more re-entrant code.  I can
think of a lot of things that could really benefit from this.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [WIP/RFC 00/23] repository object

[Johannes Schindelin]

Hi Stefan,

On Sat, 20 May 2017, Stefan Beller wrote:

> On Thu, May 18, 2017 at 4:21 PM, Brandon Williams <bmwill@google.com> wrote:
> 
> > This is still very much in a WIP state, though it does pass all tests.  What
> > I'm hoping for here is to get a discussion started about the feasibility of a
> > change like this and hopefully to get the ball rolling.  Is this a direction we
> > want to move in?  Is it worth the pain?
> 
> I would be really happy to see this series land eventually.
> 
> The introduction of a repo object will deliver performance at a higher
> level, such as
> * (remarked by Ben): enabling of threading
> * submodules do not need to spawn processes
> * I would imagine that developer velocity will go up by having less global state
>   in the long run.

* avoiding really ugly hacks such as save_env_before_alias() which are
  even incomplete, as git_dir is not reset upon restore_env().

Ciao,
Dscho

Re: [WIP/RFC 00/23] repository object

[Jeff King]

On Thu, May 18, 2017 at 04:21:11PM -0700, Brandon Williams wrote:

> When I first started working on the git project I found it very difficult to
> understand parts of the code base because of the inherently global nature of
> our code.  It also made working on submodules very difficult.  Since we can
> only open up a single repository per process, you need to launch a child
> process in order to process a submodule.  But you also need to be able to
> communicate other stateful information to the children processes so that the
> submodules know how best to format their output or match against a
> pathspec...it ends up feeling like layering on hack after hack.  What I would
> really like to do, is to have the ability to have a repository object so that I
> can open a submodule in-process.

We could always buy in fully to the multi-process model and just
implement a generic RPC protocol between the parent and submodule gits.
Does CORBA still exist?

(No, I am not serious about any of that).

> This is still very much in a WIP state, though it does pass all tests.  What
> I'm hoping for here is to get a discussion started about the feasibility of a
> change like this and hopefully to get the ball rolling.  Is this a direction we
> want to move in?  Is it worth the pain?

I think the really painful part is going to be all of the system calls
that rely on global state provided by the OS. Like, say, every
filesystem call that expects to find working tree files without
prepending the working tree path.

That said, even if we never reached the point where we could handle all
submodule requests in-process, I think sticking the repo-related global
state in a struct certainly could not hurt general readability. So it's
a good direction regardless of whether we take it all the way.

-Peff

Re: [WIP/RFC 00/23] repository object

[Brandon Williams]

On 05/22, Jeff King wrote:
> On Thu, May 18, 2017 at 04:21:11PM -0700, Brandon Williams wrote:
> 
> > When I first started working on the git project I found it very difficult to
> > understand parts of the code base because of the inherently global nature of
> > our code.  It also made working on submodules very difficult.  Since we can
> > only open up a single repository per process, you need to launch a child
> > process in order to process a submodule.  But you also need to be able to
> > communicate other stateful information to the children processes so that the
> > submodules know how best to format their output or match against a
> > pathspec...it ends up feeling like layering on hack after hack.  What I would
> > really like to do, is to have the ability to have a repository object so that I
> > can open a submodule in-process.
> 
> We could always buy in fully to the multi-process model and just
> implement a generic RPC protocol between the parent and submodule gits.
> Does CORBA still exist?
> 
> (No, I am not serious about any of that).
> 
> > This is still very much in a WIP state, though it does pass all tests.  What
> > I'm hoping for here is to get a discussion started about the feasibility of a
> > change like this and hopefully to get the ball rolling.  Is this a direction we
> > want to move in?  Is it worth the pain?
> 
> I think the really painful part is going to be all of the system calls
> that rely on global state provided by the OS. Like, say, every
> filesystem call that expects to find working tree files without
> prepending the working tree path.

Yeah that's going to be one of the more challenging things to deal
with...

> 
> That said, even if we never reached the point where we could handle all
> submodule requests in-process, I think sticking the repo-related global
> state in a struct certainly could not hurt general readability. So it's
> a good direction regardless of whether we take it all the way.

Glad you think so!

-- 
Brandon Williams

Re: [WIP/RFC 00/23] repository object

[Brandon Williams]

On 05/19, Ben Peart wrote:
> Glad to see you tackling this.  This is definitely a step in the
> right direction.
> 
> I realize that it will take a lot of work and that intermediate
> steps may just be pushing it the global state one level higher but
> eventually it would be great to see an entire code path global state
> free!
> 
> I'm personally interested because reducing the reliance on global
> state also helps us in our performance work as it makes it more
> possible to use threading to scale up the performance.

I'm glad that more people than just me are interesting in moving in this
direction.  It's definitely going to take some work and hopefully I can
convince some more people to help achieve this goal :D

> 
> Ben
> 
> On 5/18/2017 7:21 PM, Brandon Williams wrote:
> >When I first started working on the git project I found it very difficult to
> >understand parts of the code base because of the inherently global nature of
> >our code.  It also made working on submodules very difficult.  Since we can
> >only open up a single repository per process, you need to launch a child
> >process in order to process a submodule.  But you also need to be able to
> >communicate other stateful information to the children processes so that the
> >submodules know how best to format their output or match against a
> >pathspec...it ends up feeling like layering on hack after hack.  What I would
> >really like to do, is to have the ability to have a repository object so that I
> >can open a submodule in-process.
> >
> >Before this becomes a reality for all commands, much of the library code would
> >need to be refactored in order to work purely on handles instead of global
> >state.  As it turned out, ls-files is a pretty simple command and doesn't have
> >*too* many dependencies.  The biggest thing that needed to be changed was
> >piping through an index into a couple library routines so that they don't
> >inherently rely on 'the_index'.  A few of these changes I've sent out and can
> >be found at 'origin/bw/pathspec-sans-the-index' and
> >'origin/bw/dir-c-stops-relying-on-the-index' which this series is based on.
> >
> >Patches 1-16 are refactorings to prepare either library code or ls-files itself
> >to be ready to handle passing around an index struct.  Patches 17-22 introduce
> >a repository struct and change a couple of things about how submodule caches
> >work (getting submodule information from .gitmodules).  And Patch 23 converts
> >ls-files to use a repository struct.
> >
> >The most interesting part of the series is from 17-23.  And 1-16 could be taken
> >as is without the rest of the series.
> >
> >This is still very much in a WIP state, though it does pass all tests.  What
> >I'm hoping for here is to get a discussion started about the feasibility of a
> >change like this and hopefully to get the ball rolling.  Is this a direction we
> >want to move in?  Is it worth the pain?
> >
> >Thanks for taking the time to look at this and entertain my insane ideas :)
> >
> >Brandon Williams (23):
> >  convert: convert get_cached_convert_stats_ascii to take an index
> >  convert: convert crlf_to_git to take an index
> >  convert: convert convert_to_git_filter_fd to take an index
> >  convert: convert convert_to_git to take an index
> >  convert: convert renormalize_buffer to take an index
> >  tree: convert read_tree to take an index parameter
> >  ls-files: convert overlay_tree_on_cache to take an index
> >  ls-files: convert write_eolinfo to take an index
> >  ls-files: convert show_killed_files to take an index
> >  ls-files: convert show_other_files to take an index
> >  ls-files: convert show_ru_info to take an index
> >  ls-files: convert ce_excluded to take an index
> >  ls-files: convert prune_cache to take an index
> >  ls-files: convert show_files to take an index
> >  ls-files: factor out debug info into a function
> >  ls-files: factor out tag calculation
> >  repo: introduce new repository object
> >  repo: add index_state to struct repo
> >  repo: add per repo config
> >  submodule-config: refactor to allow for multiple submodule_cache's
> >  repo: add repo_read_gitmodules
> >  submodule: add is_submodule_active
> >  ls-files: use repository object
> >
> > Makefile                               |   1 +
> > apply.c                                |   2 +-
> > builtin/blame.c                        |   2 +-
> > builtin/commit.c                       |   3 +-
> > builtin/ls-files.c                     | 348 ++++++++++++++++-----------------
> > cache.h                                |   4 +-
> > combine-diff.c                         |   2 +-
> > config.c                               |   2 +-
> > convert.c                              |  31 +--
> > convert.h                              |  19 +-
> > diff.c                                 |   6 +-
> > dir.c                                  |   2 +-
> > git.c                                  |   2 +-
> > ll-merge.c                             |   2 +-
> > merge-recursive.c                      |   4 +-
> > repo.c                                 | 112 +++++++++++
> > repo.h                                 |  22 +++
> > sha1_file.c                            |   6 +-
> > submodule-config.c                     |  40 +++-
> > submodule-config.h                     |  10 +
> > submodule.c                            |  51 +++++
> > submodule.h                            |   2 +
> > t/t3007-ls-files-recurse-submodules.sh |  39 ++++
> > tree.c                                 |  28 ++-
> > tree.h                                 |   3 +-
> > 25 files changed, 513 insertions(+), 230 deletions(-)
> > create mode 100644 repo.c
> > create mode 100644 repo.h
> >

-- 
Brandon Williams

Re: [WIP/RFC 17/23] repo: introduce new repository object

[Brandon Williams]

On 05/20, Stefan Beller wrote:
> On Thu, May 18, 2017 at 4:21 PM, Brandon Williams <bmwill@google.com> wrote:
> > Introduce 'struct repo' an object used to represent a repository.
> 
> Is this the right place to outline what you expect from a repo object?
> Are you planning to use it everywhere?
> Is it lazy-init'd and it takes care of it itself, or would the caller
> have to take
> care of the state of the repo? ("the repo object is just a place to put the
> current globals")

Those are all great questions, questions that I don't think I have all
the answers for right now.  Since this is still in the idea phase I'm
hoping to hear what other people think this would look like in an ideal
world.  I don't think everything would need to be lazy-init'd or can
easily be done that way.  At least the index stuff isn't set up to
be able to do that.  I can see the config being lazy-init'd in one way
or another, though that would require passing in a repo object instead
of a config-set when you want to look-up a config value (which is
probably very reasonable) that way the config-set stored in the repo
object can be properly initialized.

> 
> >
> > Signed-off-by: Brandon Williams <bmwill@google.com>
> > ---
> >  Makefile |  1 +
> >  repo.c   | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  repo.h   | 15 +++++++++++++++
> >  3 files changed, 58 insertions(+)
> >  create mode 100644 repo.c
> >  create mode 100644 repo.h
> >
> > diff --git a/Makefile b/Makefile
> > index e35542e63..a49d2f96a 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -821,6 +821,7 @@ LIB_OBJS += refs/ref-cache.o
> >  LIB_OBJS += ref-filter.o
> >  LIB_OBJS += remote.o
> >  LIB_OBJS += replace_object.o
> > +LIB_OBJS += repo.o
> >  LIB_OBJS += rerere.o
> >  LIB_OBJS += resolve-undo.o
> >  LIB_OBJS += revision.o
> > diff --git a/repo.c b/repo.c
> > new file mode 100644
> > index 000000000..d47e98d95
> > --- /dev/null
> > +++ b/repo.c
> > @@ -0,0 +1,42 @@
> > +#include "cache.h"
> > +#include "repo.h"
> > +
> > +int
> > +repo_init(struct repo *repo, const char *gitdir, const char *worktree)
> 
> style ;)

Darn, I forgot to change this before sending out.  An easy fix, though I
still like this style better :P

> 
> 
> > +       /* Maybe need a check to verify that a worktree is indeed a worktree? */
> 
> add NEEDSWORK/FIXME prefix to comment?
> 
> > +void
> > +repo_clear(struct repo *repo)
> 
> style ;)

-- 
Brandon Williams

Re: [WIP/RFC 00/23] repository object

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> On 05/22, Jeff King wrote:
>> That said, even if we never reached the point where we could handle all
>> submodule requests in-process, I think sticking the repo-related global
>> state in a struct certainly could not hurt general readability. So it's
>> a good direction regardless of whether we take it all the way.
>
> Glad you think so!

Yup, what Jeff said ;-)

Re: [WIP/RFC 00/23] repository object

[Duy Nguyen]

On Tue, May 23, 2017 at 2:35 AM, Jeff King <peff@peff.net> wrote:
> On Thu, May 18, 2017 at 04:21:11PM -0700, Brandon Williams wrote:
>
>> When I first started working on the git project I found it very difficult to
>> understand parts of the code base because of the inherently global nature of
>> our code.  It also made working on submodules very difficult.  Since we can
>> only open up a single repository per process, you need to launch a child
>> process in order to process a submodule.  But you also need to be able to
>> communicate other stateful information to the children processes so that the
>> submodules know how best to format their output or match against a
>> pathspec...it ends up feeling like layering on hack after hack.  What I would
>> really like to do, is to have the ability to have a repository object so that I
>> can open a submodule in-process.
>
> We could always buy in fully to the multi-process model and just
> implement a generic RPC protocol between the parent and submodule gits.
> Does CORBA still exist?
>
> (No, I am not serious about any of that).

CORBA or not, submodule IPC is a real pain. That was what I felt
reading the super-prefix changes a few weeks ago. Some operations
might benefit from staying in the same process, but probably not all
(and we lose process protection, which sometimes is a good thing)

>> This is still very much in a WIP state, though it does pass all tests.  What
>> I'm hoping for here is to get a discussion started about the feasibility of a
>> change like this and hopefully to get the ball rolling.  Is this a direction we
>> want to move in?  Is it worth the pain?
>
> I think the really painful part is going to be all of the system calls
> that rely on global state provided by the OS. Like, say, every
> filesystem call that expects to find working tree files without
> prepending the working tree path.
>
> That said, even if we never reached the point where we could handle all
> submodule requests in-process, I think sticking the repo-related global
> state in a struct certainly could not hurt general readability. So it's
> a good direction regardless of whether we take it all the way.

I doubt we would reach the point where libgit.a can handle all
submodule operations in-process either. That would put libgit.a in a
direct competitor position with libgit2. I do hope though that having
clearer/modular data structure will improve readability, not hurt it
(e.g. you see the data model and could largely guess how the code
interacts before digging deep in).
-- 
Duy

Re: [WIP/RFC 00/23] repository object

[Ævar Arnfjörð Bjarmason]

On Mon, May 29, 2017 at 12:36 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Tue, May 23, 2017 at 2:35 AM, Jeff King <peff@peff.net> wrote:
>> On Thu, May 18, 2017 at 04:21:11PM -0700, Brandon Williams wrote:
>>
>>> When I first started working on the git project I found it very difficult to
>>> understand parts of the code base because of the inherently global nature of
>>> our code.  It also made working on submodules very difficult.  Since we can
>>> only open up a single repository per process, you need to launch a child
>>> process in order to process a submodule.  But you also need to be able to
>>> communicate other stateful information to the children processes so that the
>>> submodules know how best to format their output or match against a
>>> pathspec...it ends up feeling like layering on hack after hack.  What I would
>>> really like to do, is to have the ability to have a repository object so that I
>>> can open a submodule in-process.
>>
>> We could always buy in fully to the multi-process model and just
>> implement a generic RPC protocol between the parent and submodule gits.
>> Does CORBA still exist?
>>
>> (No, I am not serious about any of that).
>
> CORBA or not, submodule IPC is a real pain. That was what I felt
> reading the super-prefix changes a few weeks ago. Some operations
> might benefit from staying in the same process, but probably not all
> (and we lose process protection, which sometimes is a good thing)
>
>>> This is still very much in a WIP state, though it does pass all tests.  What
>>> I'm hoping for here is to get a discussion started about the feasibility of a
>>> change like this and hopefully to get the ball rolling.  Is this a direction we
>>> want to move in?  Is it worth the pain?
>>
>> I think the really painful part is going to be all of the system calls
>> that rely on global state provided by the OS. Like, say, every
>> filesystem call that expects to find working tree files without
>> prepending the working tree path.
>>
>> That said, even if we never reached the point where we could handle all
>> submodule requests in-process, I think sticking the repo-related global
>> state in a struct certainly could not hurt general readability. So it's
>> a good direction regardless of whether we take it all the way.
>
> I doubt we would reach the point where libgit.a can handle all
> submodule operations in-process either. That would put libgit.a in a
> direct competitor position with libgit2.

Wouldn't that be a good thing? We already have some users (e.g.
Microsoft) choosing not to use libgit and instead use git.git because
the latter is generally more mature, if git.git gains more libgit
features without harming other things it'll be more useful to more
people.

Re: [WIP/RFC 00/23] repository object

[Duy Nguyen]

On Mon, May 29, 2017 at 6:23 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>>> That said, even if we never reached the point where we could handle all
>>> submodule requests in-process, I think sticking the repo-related global
>>> state in a struct certainly could not hurt general readability. So it's
>>> a good direction regardless of whether we take it all the way.
>>
>> I doubt we would reach the point where libgit.a can handle all
>> submodule operations in-process either. That would put libgit.a in a
>> direct competitor position with libgit2.
>
> Wouldn't that be a good thing? We already have some users (e.g.
> Microsoft) choosing not to use libgit and instead use git.git because
> the latter is generally more mature, if git.git gains more libgit
> features without harming other things it'll be more useful to more
> people.

Whether it's a good thing might depend on view point. Similar to linux
kernel, we might want some freedom to break ABI and refactor the code.
But I think it's too early  to discuss this right now. There's lots of
work (the "die()" minefield comes to mind) before we reach the point
where libgit.a may be good enough for external use.
-- 
Duy

Re: [WIP/RFC 00/23] repository object

[Brandon Williams]

On 05/29, Duy Nguyen wrote:
> On Mon, May 29, 2017 at 6:23 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> >>> That said, even if we never reached the point where we could handle all
> >>> submodule requests in-process, I think sticking the repo-related global
> >>> state in a struct certainly could not hurt general readability. So it's
> >>> a good direction regardless of whether we take it all the way.
> >>
> >> I doubt we would reach the point where libgit.a can handle all
> >> submodule operations in-process either. That would put libgit.a in a
> >> direct competitor position with libgit2.
> >
> > Wouldn't that be a good thing? We already have some users (e.g.
> > Microsoft) choosing not to use libgit and instead use git.git because
> > the latter is generally more mature, if git.git gains more libgit
> > features without harming other things it'll be more useful to more
> > people.
> 
> Whether it's a good thing might depend on view point. Similar to linux
> kernel, we might want some freedom to break ABI and refactor the code.
> But I think it's too early  to discuss this right now. There's lots of
> work (the "die()" minefield comes to mind) before we reach the point
> where libgit.a may be good enough for external use.

My personal end goal is making the code easier to parse and to make
submodule work easier.  I know I've only worked on the project for a
short time but I already regret some of the submodule changes that I've
made because they are straight up hacks which I want to eradicate sooner
rather than later (just look at my patches to get 'git grep' to handle
submodules...).

I wasn't thinking about libgit2 and don't really haven't thought too
much about the possibility of being in competition with that project.

-- 
Brandon Williams