convert submodule.c to not use the index compat macros

PATCH In review Brandon Williams 4 participants 8 messages Dec 12, 2017

Share & tools

Brandon Williams

author

2017-12-12 19:54 UTC #0 lore raw

cover convert submodule.c to not use the index compat macros

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(-)

Signature · 1 line

2.15.1.504.g5279b80103-goog

Brandon Williams

author

2017-12-12 19:54 UTC #1 lore raw

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

Signed-off-by Brandon Williams <[email protected]>

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

builtin/mv.c +1 -1

index ffdd5f01a..cf3684d90 100644
@@ -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))

builtin/rm.c +1 -1

index d91451fea..4a2fcca27 100644
@@ -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) {

submodule.c +2 -2

index 95e6aff2b..7097be806 100644
@@ -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"));
 }

submodule.h +1 -1

index f0da0277a..cd984ecba 100644
@@ -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);

Signature · 1 line

2.15.1.504.g5279b80103-goog

Brandon Williams

author

2017-12-12 19:54 UTC #2 lore raw

PATCH 2/3 submodule: used correct index in is_staging_gitmodules_ok

In reply to 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 <[email protected]>

---
 submodule.c | 5 +++--
 submodule.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

submodule.c +3 -2

index 7097be806..a9670eaae 100644
@@ -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;
 	}

submodule.h +1 -1

index cd984ecba..e2a5de3d8 100644
@@ -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);

Signature · 1 line

2.15.1.504.g5279b80103-goog

Eric Sunshine

2017-12-12 20:17 UTC #3 lore raw

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

Quoted 4 lines · 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

s/relys/relies/

Quoted 4 lines

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 <[email protected]>

Brandon Williams

author

2017-12-12 19:54 UTC #4 lore raw

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

In reply to 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 <[email protected]>

---
 builtin/fetch.c |  4 +++-
 submodule.c     | 23 +++++++++++++----------
 submodule.h     | 10 ++++++----
 3 files changed, 22 insertions(+), 15 deletions(-)

builtin/fetch.c +3 -1

index e705237fa..e656746ab 100644
@@ -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,

submodule.c +13 ~~-10~~

index a9670eaae..59372eada 100644
@@ -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");

submodule.h +6 -4

index e2a5de3d8..b9b7ef003 100644
@@ -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);

Signature · 1 line

2.15.1.504.g5279b80103-goog

Stefan Beller

2017-12-12 20:09 UTC #5 lore raw

In reply to Brandon Williams

Quoted 2 lines · 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.

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

Junio C Hamano

2017-12-12 20:44 UTC #6 lore raw

Quoted 2 lines · Stefan Beller

... 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.

Junio C Hamano

2017-12-12 20:38 UTC #7 lore raw

In reply to Brandon Williams

Quoted 2 lines · 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.

Nice. Will queue.

Thanks.