[PATCH 0/3] convert submodule.c to not use the index compat macros

[PATCH 0/3] convert submodule.c to not use the index compat macros

[Brandon Williams]

This series removes the remaining users of the index compatibility macros and
ensures that future uses of the macros will result in compiler errors.

Brandon Williams (3):
  submodule: convert stage_updated_gitmodules to take a struct
    index_state
  submodule: used correct index in is_staging_gitmodules_ok
  submodule: convert get_next_submodule to not rely on the_index

 builtin/fetch.c |  4 +++-
 builtin/mv.c    |  2 +-
 builtin/rm.c    |  2 +-
 submodule.c     | 32 ++++++++++++++++++--------------
 submodule.h     | 14 ++++++++------
 5 files changed, 31 insertions(+), 23 deletions(-)

-- 
2.15.1.504.g5279b80103-goog

[PATCH 1/3] submodule: convert stage_updated_gitmodules to take a struct index_state

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/mv.c | 2 +-
 builtin/rm.c | 2 +-
 submodule.c  | 4 ++--
 submodule.h  | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index ffdd5f01a..cf3684d90 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -291,7 +291,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	}
 
 	if (gitmodules_modified)
-		stage_updated_gitmodules();
+		stage_updated_gitmodules(&the_index);
 
 	if (active_cache_changed &&
 	    write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
diff --git a/builtin/rm.c b/builtin/rm.c
index d91451fea..4a2fcca27 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -382,7 +382,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		}
 		strbuf_release(&buf);
 		if (gitmodules_modified)
-			stage_updated_gitmodules();
+			stage_updated_gitmodules(&the_index);
 	}
 
 	if (active_cache_changed) {
diff --git a/submodule.c b/submodule.c
index 95e6aff2b..7097be806 100644
--- a/submodule.c
+++ b/submodule.c
@@ -143,9 +143,9 @@ int remove_path_from_gitmodules(const char *path)
 	return 0;
 }
 
-void stage_updated_gitmodules(void)
+void stage_updated_gitmodules(struct index_state *istate)
 {
-	if (add_file_to_cache(GITMODULES_FILE, 0))
+	if (add_file_to_index(istate, GITMODULES_FILE, 0))
 		die(_("staging updated .gitmodules failed"));
 }
 
diff --git a/submodule.h b/submodule.h
index f0da0277a..cd984ecba 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,7 +37,7 @@ extern int is_gitmodules_unmerged(const struct index_state *istate);
 extern int is_staging_gitmodules_ok(const struct index_state *istate);
 extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);
 extern int remove_path_from_gitmodules(const char *path);
-extern void stage_updated_gitmodules(void);
+extern void stage_updated_gitmodules(struct index_state *istate);
 extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
 		const char *path);
 extern int git_default_submodule_config(const char *var, const char *value, void *cb);
-- 
2.15.1.504.g5279b80103-goog

[PATCH 2/3] submodule: used correct index in is_staging_gitmodules_ok

[Brandon Williams]

Commit 883e248b8 (fsmonitor: teach git to optionally utilize a file
system monitor to speed up detecting new or changed files., 2017-09-22)
introduced a call to 'ce_match_stat()' in 'is_staging_gitmodules_ok()'
which implicitly relys on the the global 'the_index' instead of the
passed in 'struct index_state'.  Fix this by changing the call to
'ie_match_stat()' and using the passed in index_state struct.

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

diff --git a/submodule.c b/submodule.c
index 7097be806..a9670eaae 100644
--- a/submodule.c
+++ b/submodule.c
@@ -55,14 +55,15 @@ int is_gitmodules_unmerged(const struct index_state *istate)
  * future version when we learn to stage the changes we do ourselves without
  * staging any previous modifications.
  */
-int is_staging_gitmodules_ok(const struct index_state *istate)
+int is_staging_gitmodules_ok(struct index_state *istate)
 {
 	int pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));
 
 	if ((pos >= 0) && (pos < istate->cache_nr)) {
 		struct stat st;
 		if (lstat(GITMODULES_FILE, &st) == 0 &&
-		    ce_match_stat(istate->cache[pos], &st, CE_MATCH_IGNORE_FSMONITOR) & DATA_CHANGED)
+		    ie_match_stat(istate, istate->cache[pos], &st,
+				  CE_MATCH_IGNORE_FSMONITOR) & DATA_CHANGED)
 			return 0;
 	}
 
diff --git a/submodule.h b/submodule.h
index cd984ecba..e2a5de3d8 100644
--- a/submodule.h
+++ b/submodule.h
@@ -34,7 +34,7 @@ struct submodule_update_strategy {
 #define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
 
 extern int is_gitmodules_unmerged(const struct index_state *istate);
-extern int is_staging_gitmodules_ok(const struct index_state *istate);
+extern int is_staging_gitmodules_ok(struct index_state *istate);
 extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);
 extern int remove_path_from_gitmodules(const char *path);
 extern void stage_updated_gitmodules(struct index_state *istate);
-- 
2.15.1.504.g5279b80103-goog

Re: [PATCH 2/3] submodule: used correct index in is_staging_gitmodules_ok

[Eric Sunshine]

On Tue, Dec 12, 2017 at 2:53 PM, Brandon Williams <bmwill@google.com> wrote:
> Commit 883e248b8 (fsmonitor: teach git to optionally utilize a file
> system monitor to speed up detecting new or changed files., 2017-09-22)
> introduced a call to 'ce_match_stat()' in 'is_staging_gitmodules_ok()'
> which implicitly relys on the the global 'the_index' instead of the

s/relys/relies/

> passed in 'struct index_state'.  Fix this by changing the call to
> 'ie_match_stat()' and using the passed in index_state struct.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>

[PATCH 3/3] submodule: convert get_next_submodule to not rely on the_index

[Brandon Williams]

Instead of implicitly relying on the global 'the_index', convert
'get_next_submodule()' to use the index of the repository stored in the
callback data 'struct submodule_parallel_fetch'.

Since this removes the last user of the index compatibility macros,
define 'NO_THE_INDEX_COMPATIBILITY_MACROS' to prevent future users of
these macros in submodule.c.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/fetch.c |  4 +++-
 submodule.c     | 23 +++++++++++++----------
 submodule.h     | 10 ++++++----
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index e705237fa..e656746ab 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -3,6 +3,7 @@
  */
 #include "cache.h"
 #include "config.h"
+#include "repository.h"
 #include "refs.h"
 #include "commit.h"
 #include "builtin.h"
@@ -1397,7 +1398,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		struct argv_array options = ARGV_ARRAY_INIT;
 
 		add_options_to_argv(&options);
-		result = fetch_populated_submodules(&options,
+		result = fetch_populated_submodules(the_repository,
+						    &options,
 						    submodule_prefix,
 						    recurse_submodules,
 						    recurse_submodules_default,
diff --git a/submodule.c b/submodule.c
index a9670eaae..59372eada 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1,3 +1,5 @@
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
+
 #include "cache.h"
 #include "repository.h"
 #include "config.h"
@@ -1179,7 +1181,7 @@ int submodule_touches_in_range(struct object_id *excl_oid,
 struct submodule_parallel_fetch {
 	int count;
 	struct argv_array args;
-	const char *work_tree;
+	struct repository *r;
 	const char *prefix;
 	int command_line_option;
 	int default_option;
@@ -1200,7 +1202,7 @@ static int get_fetch_recurse_config(const struct submodule *submodule,
 
 		int fetch_recurse = submodule->fetch_recurse;
 		key = xstrfmt("submodule.%s.fetchRecurseSubmodules", submodule->name);
-		if (!repo_config_get_string_const(the_repository, key, &value)) {
+		if (!repo_config_get_string_const(spf->r, key, &value)) {
 			fetch_recurse = parse_fetch_recurse_submodules_arg(key, value);
 		}
 		free(key);
@@ -1219,11 +1221,11 @@ static int get_next_submodule(struct child_process *cp,
 	int ret = 0;
 	struct submodule_parallel_fetch *spf = data;
 
-	for (; spf->count < active_nr; spf->count++) {
+	for (; spf->count < spf->r->index->cache_nr; spf->count++) {
 		struct strbuf submodule_path = STRBUF_INIT;
 		struct strbuf submodule_git_dir = STRBUF_INIT;
 		struct strbuf submodule_prefix = STRBUF_INIT;
-		const struct cache_entry *ce = active_cache[spf->count];
+		const struct cache_entry *ce = spf->r->index->cache[spf->count];
 		const char *git_dir, *default_argv;
 		const struct submodule *submodule;
 		struct submodule default_submodule = SUBMODULE_INIT;
@@ -1231,7 +1233,7 @@ static int get_next_submodule(struct child_process *cp,
 		if (!S_ISGITLINK(ce->ce_mode))
 			continue;
 
-		submodule = submodule_from_path(&null_oid, ce->name);
+		submodule = submodule_from_cache(spf->r, &null_oid, ce->name);
 		if (!submodule) {
 			const char *name = default_name_or_path(ce->name);
 			if (name) {
@@ -1257,7 +1259,7 @@ static int get_next_submodule(struct child_process *cp,
 			continue;
 		}
 
-		strbuf_addf(&submodule_path, "%s/%s", spf->work_tree, ce->name);
+		strbuf_repo_worktree_path(&submodule_path, spf->r, "%s", ce->name);
 		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
 		strbuf_addf(&submodule_prefix, "%s%s/", spf->prefix, ce->name);
 		git_dir = read_gitfile(submodule_git_dir.buf);
@@ -1310,7 +1312,8 @@ static int fetch_finish(int retvalue, struct strbuf *err,
 	return 0;
 }
 
-int fetch_populated_submodules(const struct argv_array *options,
+int fetch_populated_submodules(struct repository *r,
+			       const struct argv_array *options,
 			       const char *prefix, int command_line_option,
 			       int default_option,
 			       int quiet, int max_parallel_jobs)
@@ -1318,16 +1321,16 @@ int fetch_populated_submodules(const struct argv_array *options,
 	int i;
 	struct submodule_parallel_fetch spf = SPF_INIT;
 
-	spf.work_tree = get_git_work_tree();
+	spf.r = r;
 	spf.command_line_option = command_line_option;
 	spf.default_option = default_option;
 	spf.quiet = quiet;
 	spf.prefix = prefix;
 
-	if (!spf.work_tree)
+	if (!r->worktree)
 		goto out;
 
-	if (read_cache() < 0)
+	if (repo_read_index(r) < 0)
 		die("index file corrupt");
 
 	argv_array_push(&spf.args, "fetch");
diff --git a/submodule.h b/submodule.h
index e2a5de3d8..b9b7ef003 100644
--- a/submodule.h
+++ b/submodule.h
@@ -76,10 +76,12 @@ extern int should_update_submodules(void);
  */
 extern const struct submodule *submodule_from_ce(const struct cache_entry *ce);
 extern void check_for_new_submodule_commits(struct object_id *oid);
-extern int fetch_populated_submodules(const struct argv_array *options,
-			       const char *prefix, int command_line_option,
-			       int default_option,
-			       int quiet, int max_parallel_jobs);
+extern int fetch_populated_submodules(struct repository *r,
+				      const struct argv_array *options,
+				      const char *prefix,
+				      int command_line_option,
+				      int default_option,
+				      int quiet, int max_parallel_jobs);
 extern unsigned is_submodule_modified(const char *path, int ignore_untracked);
 extern int submodule_uses_gitfile(const char *path);
 
-- 
2.15.1.504.g5279b80103-goog

Re: [PATCH 0/3] convert submodule.c to not use the index compat macros

[Stefan Beller]

On Tue, Dec 12, 2017 at 11:53 AM, Brandon Williams <bmwill@google.com> wrote:
> This series removes the remaining users of the index compatibility macros and
> ensures that future uses of the macros will result in compiler errors.

Thanks for converting the submodule code to avoid these old macros.
Specifically the submodule code will benefit once we have a repository
object available throughout the code base, so it is a great start.

I reviewed the series and would call out patch 2 to be a bugfix that could
go independently, but the whole series is fine as-is with me.

Thanks,
Stefan

Re: [PATCH 0/3] convert submodule.c to not use the index compat macros

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> ... would call out patch 2 to be a bugfix that could
> go independently, but the whole series is fine as-is with me.

Good eyes.  

I agree that it makes sense to treat 2/3 as a follow-up fix for an
already graduated topic, and make the other two depend on a result
of applying that first.  In practice it should not matter all that
much in this case, because the breakage is only in 'master' and is
not going to be merged down to 'maint', but it nevertheless was a
good point to raise.

Thanks for carefully reading the patches through.

Re: [PATCH 0/3] convert submodule.c to not use the index compat macros

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> This series removes the remaining users of the index compatibility macros and
> ensures that future uses of the macros will result in compiler errors.

Nice.  Will queue.

Thanks.