{"thread":{"id":"47434","subject":"[PATCH 0/3] convert submodule.c to not use the index compat macros","startedAt":"2017-12-12T19:54:07Z","lastAt":"2017-12-12T20:44:22Z","messageCount":8,"participants":["Brandon Williams","Stefan Beller","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"334699","messageId":"20171212195352.146675-1-bmwill@google.com","threadId":"47434","inReplyTo":null,"subject":"[PATCH 0/3] convert submodule.c to not use the index compat macros","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-12-12T19:53:49Z","receivedAt":"2017-12-12T19:54:07Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"This series removes the remaining users of the index compatibility macros and\nensures that future uses of the macros will result in compiler errors.\n\nBrandon Williams (3):\n  submodule: convert stage_updated_gitmodules to take a struct\n    index_state\n  submodule: used correct index in is_staging_gitmodules_ok\n  submodule: convert get_next_submodule to not rely on the_index\n\n builtin/fetch.c |  4 +++-\n builtin/mv.c    |  2 +-\n builtin/rm.c    |  2 +-\n submodule.c     | 32 ++++++++++++++++++--------------\n submodule.h     | 14 ++++++++------\n 5 files changed, 31 insertions(+), 23 deletions(-)\n\n-- \n2.15.1.504.g5279b80103-goog\n\n"},{"id":"334700","messageId":"20171212195352.146675-2-bmwill@google.com","threadId":"47434","inReplyTo":"20171212195352.146675-1-bmwill@google.com","subject":"[PATCH 1/3] submodule: convert stage_updated_gitmodules to take a struct index_state","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-12-12T19:53:50Z","receivedAt":"2017-12-12T19:54:09Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Signed-off-by: Brandon Williams <bmwill@google.com>\n---\n builtin/mv.c | 2 +-\n builtin/rm.c | 2 +-\n submodule.c  | 4 ++--\n submodule.h  | 2 +-\n 4 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex ffdd5f01a..cf3684d90 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -291,7 +291,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (gitmodules_modified)\n-\t\tstage_updated_gitmodules();\n+\t\tstage_updated_gitmodules(&the_index);\n \n \tif (active_cache_changed &&\n \t    write_locked_index(&the_index, &lock_file, COMMIT_LOCK))\ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex d91451fea..4a2fcca27 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -382,7 +382,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t\t}\n \t\tstrbuf_release(&buf);\n \t\tif (gitmodules_modified)\n-\t\t\tstage_updated_gitmodules();\n+\t\t\tstage_updated_gitmodules(&the_index);\n \t}\n \n \tif (active_cache_changed) {\ndiff --git a/submodule.c b/submodule.c\nindex 95e6aff2b..7097be806 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -143,9 +143,9 @@ int remove_path_from_gitmodules(const char *path)\n \treturn 0;\n }\n \n-void stage_updated_gitmodules(void)\n+void stage_updated_gitmodules(struct index_state *istate)\n {\n-\tif (add_file_to_cache(GITMODULES_FILE, 0))\n+\tif (add_file_to_index(istate, GITMODULES_FILE, 0))\n \t\tdie(_(\"staging updated .gitmodules failed\"));\n }\n \ndiff --git a/submodule.h b/submodule.h\nindex f0da0277a..cd984ecba 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -37,7 +37,7 @@ extern int is_gitmodules_unmerged(const struct index_state *istate);\n extern int is_staging_gitmodules_ok(const struct index_state *istate);\n extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n extern int remove_path_from_gitmodules(const char *path);\n-extern void stage_updated_gitmodules(void);\n+extern void stage_updated_gitmodules(struct index_state *istate);\n extern void set_diffopt_flags_from_submodule_config(struct diff_options *,\n \t\tconst char *path);\n extern int git_default_submodule_config(const char *var, const char *value, void *cb);\n-- \n2.15.1.504.g5279b80103-goog\n\n"},{"id":"334701","messageId":"20171212195352.146675-3-bmwill@google.com","threadId":"47434","inReplyTo":"20171212195352.146675-1-bmwill@google.com","subject":"[PATCH 2/3] submodule: used correct index in is_staging_gitmodules_ok","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-12-12T19:53:51Z","receivedAt":"2017-12-12T19:54:12Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Commit 883e248b8 (fsmonitor: teach git to optionally utilize a file\nsystem monitor to speed up detecting new or changed files., 2017-09-22)\nintroduced a call to 'ce_match_stat()' in 'is_staging_gitmodules_ok()'\nwhich implicitly relys on the the global 'the_index' instead of the\npassed in 'struct index_state'.  Fix this by changing the call to\n'ie_match_stat()' and using the passed in index_state struct.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n submodule.c | 5 +++--\n submodule.h | 2 +-\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 7097be806..a9670eaae 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -55,14 +55,15 @@ int is_gitmodules_unmerged(const struct index_state *istate)\n  * future version when we learn to stage the changes we do ourselves without\n  * staging any previous modifications.\n  */\n-int is_staging_gitmodules_ok(const struct index_state *istate)\n+int is_staging_gitmodules_ok(struct index_state *istate)\n {\n \tint pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));\n \n \tif ((pos >= 0) && (pos < istate->cache_nr)) {\n \t\tstruct stat st;\n \t\tif (lstat(GITMODULES_FILE, &st) == 0 &&\n-\t\t    ce_match_stat(istate->cache[pos], &st, CE_MATCH_IGNORE_FSMONITOR) & DATA_CHANGED)\n+\t\t    ie_match_stat(istate, istate->cache[pos], &st,\n+\t\t\t\t  CE_MATCH_IGNORE_FSMONITOR) & DATA_CHANGED)\n \t\t\treturn 0;\n \t}\n \ndiff --git a/submodule.h b/submodule.h\nindex cd984ecba..e2a5de3d8 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -34,7 +34,7 @@ struct submodule_update_strategy {\n #define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}\n \n extern int is_gitmodules_unmerged(const struct index_state *istate);\n-extern int is_staging_gitmodules_ok(const struct index_state *istate);\n+extern int is_staging_gitmodules_ok(struct index_state *istate);\n extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n extern int remove_path_from_gitmodules(const char *path);\n extern void stage_updated_gitmodules(struct index_state *istate);\n-- \n2.15.1.504.g5279b80103-goog\n\n"},{"id":"334702","messageId":"20171212195352.146675-4-bmwill@google.com","threadId":"47434","inReplyTo":"20171212195352.146675-1-bmwill@google.com","subject":"[PATCH 3/3] submodule: convert get_next_submodule to not rely on the_index","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-12-12T19:53:52Z","receivedAt":"2017-12-12T19:54:15Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Instead of implicitly relying on the global 'the_index', convert\n'get_next_submodule()' to use the index of the repository stored in the\ncallback data 'struct submodule_parallel_fetch'.\n\nSince this removes the last user of the index compatibility macros,\ndefine 'NO_THE_INDEX_COMPATIBILITY_MACROS' to prevent future users of\nthese macros in submodule.c.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n builtin/fetch.c |  4 +++-\n submodule.c     | 23 +++++++++++++----------\n submodule.h     | 10 ++++++----\n 3 files changed, 22 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex e705237fa..e656746ab 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -3,6 +3,7 @@\n  */\n #include \"cache.h\"\n #include \"config.h\"\n+#include \"repository.h\"\n #include \"refs.h\"\n #include \"commit.h\"\n #include \"builtin.h\"\n@@ -1397,7 +1398,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\tstruct argv_array options = ARGV_ARRAY_INIT;\n \n \t\tadd_options_to_argv(&options);\n-\t\tresult = fetch_populated_submodules(&options,\n+\t\tresult = fetch_populated_submodules(the_repository,\n+\t\t\t\t\t\t    &options,\n \t\t\t\t\t\t    submodule_prefix,\n \t\t\t\t\t\t    recurse_submodules,\n \t\t\t\t\t\t    recurse_submodules_default,\ndiff --git a/submodule.c b/submodule.c\nindex a9670eaae..59372eada 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1,3 +1,5 @@\n+#define NO_THE_INDEX_COMPATIBILITY_MACROS\n+\n #include \"cache.h\"\n #include \"repository.h\"\n #include \"config.h\"\n@@ -1179,7 +1181,7 @@ int submodule_touches_in_range(struct object_id *excl_oid,\n struct submodule_parallel_fetch {\n \tint count;\n \tstruct argv_array args;\n-\tconst char *work_tree;\n+\tstruct repository *r;\n \tconst char *prefix;\n \tint command_line_option;\n \tint default_option;\n@@ -1200,7 +1202,7 @@ static int get_fetch_recurse_config(const struct submodule *submodule,\n \n \t\tint fetch_recurse = submodule->fetch_recurse;\n \t\tkey = xstrfmt(\"submodule.%s.fetchRecurseSubmodules\", submodule->name);\n-\t\tif (!repo_config_get_string_const(the_repository, key, &value)) {\n+\t\tif (!repo_config_get_string_const(spf->r, key, &value)) {\n \t\t\tfetch_recurse = parse_fetch_recurse_submodules_arg(key, value);\n \t\t}\n \t\tfree(key);\n@@ -1219,11 +1221,11 @@ static int get_next_submodule(struct child_process *cp,\n \tint ret = 0;\n \tstruct submodule_parallel_fetch *spf = data;\n \n-\tfor (; spf->count < active_nr; spf->count++) {\n+\tfor (; spf->count < spf->r->index->cache_nr; spf->count++) {\n \t\tstruct strbuf submodule_path = STRBUF_INIT;\n \t\tstruct strbuf submodule_git_dir = STRBUF_INIT;\n \t\tstruct strbuf submodule_prefix = STRBUF_INIT;\n-\t\tconst struct cache_entry *ce = active_cache[spf->count];\n+\t\tconst struct cache_entry *ce = spf->r->index->cache[spf->count];\n \t\tconst char *git_dir, *default_argv;\n \t\tconst struct submodule *submodule;\n \t\tstruct submodule default_submodule = SUBMODULE_INIT;\n@@ -1231,7 +1233,7 @@ static int get_next_submodule(struct child_process *cp,\n \t\tif (!S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n \n-\t\tsubmodule = submodule_from_path(&null_oid, ce->name);\n+\t\tsubmodule = submodule_from_cache(spf->r, &null_oid, ce->name);\n \t\tif (!submodule) {\n \t\t\tconst char *name = default_name_or_path(ce->name);\n \t\t\tif (name) {\n@@ -1257,7 +1259,7 @@ static int get_next_submodule(struct child_process *cp,\n \t\t\tcontinue;\n \t\t}\n \n-\t\tstrbuf_addf(&submodule_path, \"%s/%s\", spf->work_tree, ce->name);\n+\t\tstrbuf_repo_worktree_path(&submodule_path, spf->r, \"%s\", ce->name);\n \t\tstrbuf_addf(&submodule_git_dir, \"%s/.git\", submodule_path.buf);\n \t\tstrbuf_addf(&submodule_prefix, \"%s%s/\", spf->prefix, ce->name);\n \t\tgit_dir = read_gitfile(submodule_git_dir.buf);\n@@ -1310,7 +1312,8 @@ static int fetch_finish(int retvalue, struct strbuf *err,\n \treturn 0;\n }\n \n-int fetch_populated_submodules(const struct argv_array *options,\n+int fetch_populated_submodules(struct repository *r,\n+\t\t\t       const struct argv_array *options,\n \t\t\t       const char *prefix, int command_line_option,\n \t\t\t       int default_option,\n \t\t\t       int quiet, int max_parallel_jobs)\n@@ -1318,16 +1321,16 @@ int fetch_populated_submodules(const struct argv_array *options,\n \tint i;\n \tstruct submodule_parallel_fetch spf = SPF_INIT;\n \n-\tspf.work_tree = get_git_work_tree();\n+\tspf.r = r;\n \tspf.command_line_option = command_line_option;\n \tspf.default_option = default_option;\n \tspf.quiet = quiet;\n \tspf.prefix = prefix;\n \n-\tif (!spf.work_tree)\n+\tif (!r->worktree)\n \t\tgoto out;\n \n-\tif (read_cache() < 0)\n+\tif (repo_read_index(r) < 0)\n \t\tdie(\"index file corrupt\");\n \n \targv_array_push(&spf.args, \"fetch\");\ndiff --git a/submodule.h b/submodule.h\nindex e2a5de3d8..b9b7ef003 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -76,10 +76,12 @@ extern int should_update_submodules(void);\n  */\n extern const struct submodule *submodule_from_ce(const struct cache_entry *ce);\n extern void check_for_new_submodule_commits(struct object_id *oid);\n-extern int fetch_populated_submodules(const struct argv_array *options,\n-\t\t\t       const char *prefix, int command_line_option,\n-\t\t\t       int default_option,\n-\t\t\t       int quiet, int max_parallel_jobs);\n+extern int fetch_populated_submodules(struct repository *r,\n+\t\t\t\t      const struct argv_array *options,\n+\t\t\t\t      const char *prefix,\n+\t\t\t\t      int command_line_option,\n+\t\t\t\t      int default_option,\n+\t\t\t\t      int quiet, int max_parallel_jobs);\n extern unsigned is_submodule_modified(const char *path, int ignore_untracked);\n extern int submodule_uses_gitfile(const char *path);\n \n-- \n2.15.1.504.g5279b80103-goog\n\n"},{"id":"334705","messageId":"CAGZ79kZsTduEsrC-BWOuT6Y+QGn_SLngO4Fs78q2N8Rp+xnhCg@mail.gmail.com","threadId":"47434","inReplyTo":"20171212195352.146675-1-bmwill@google.com","subject":"Re: [PATCH 0/3] convert submodule.c to not use the index compat macros","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-12-12T20:08:59Z","receivedAt":"2017-12-12T20:09:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Dec 12, 2017 at 11:53 AM, Brandon Williams <bmwill@google.com> wrote:\n> This series removes the remaining users of the index compatibility macros and\n> ensures that future uses of the macros will result in compiler errors.\n\nThanks for converting the submodule code to avoid these old macros.\nSpecifically the submodule code will benefit once we have a repository\nobject available throughout the code base, so it is a great start.\n\nI reviewed the series and would call out patch 2 to be a bugfix that could\ngo independently, but the whole series is fine as-is with me.\n\nThanks,\nStefan\n"},{"id":"334707","messageId":"CAPig+cTtFB8ZyqF994TLbEj0Km930W3C758niCXNH5v3=hW5Cw@mail.gmail.com","threadId":"47434","inReplyTo":"20171212195352.146675-3-bmwill@google.com","subject":"Re: [PATCH 2/3] submodule: used correct index in is_staging_gitmodules_ok","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-12-12T20:17:37Z","receivedAt":"2017-12-12T20:17:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Dec 12, 2017 at 2:53 PM, Brandon Williams <bmwill@google.com> wrote:\n> Commit 883e248b8 (fsmonitor: teach git to optionally utilize a file\n> system monitor to speed up detecting new or changed files., 2017-09-22)\n> introduced a call to 'ce_match_stat()' in 'is_staging_gitmodules_ok()'\n> which implicitly relys on the the global 'the_index' instead of the\n\ns/relys/relies/\n\n> passed in 'struct index_state'.  Fix this by changing the call to\n> 'ie_match_stat()' and using the passed in index_state struct.\n>\n> Signed-off-by: Brandon Williams <bmwill@google.com>\n"},{"id":"334708","messageId":"xmqq7etr1y1y.fsf@gitster.mtv.corp.google.com","threadId":"47434","inReplyTo":"20171212195352.146675-1-bmwill@google.com","subject":"Re: [PATCH 0/3] convert submodule.c to not use the index compat macros","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-12T20:38:49Z","receivedAt":"2017-12-12T20:38:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams <bmwill@google.com> writes:\n\n> This series removes the remaining users of the index compatibility macros and\n> ensures that future uses of the macros will result in compiler errors.\n\nNice.  Will queue.\n\nThanks.\n\n"},{"id":"334709","messageId":"xmqq374f1xsv.fsf@gitster.mtv.corp.google.com","threadId":"47434","inReplyTo":"CAGZ79kZsTduEsrC-BWOuT6Y+QGn_SLngO4Fs78q2N8Rp+xnhCg@mail.gmail.com","subject":"Re: [PATCH 0/3] convert submodule.c to not use the index compat macros","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-12T20:44:16Z","receivedAt":"2017-12-12T20:44:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> ... would call out patch 2 to be a bugfix that could\n> go independently, but the whole series is fine as-is with me.\n\nGood eyes.  \n\nI agree that it makes sense to treat 2/3 as a follow-up fix for an\nalready graduated topic, and make the other two depend on a result\nof applying that first.  In practice it should not matter all that\nmuch in this case, because the breakage is only in 'master' and is\nnot going to be merged down to 'maint', but it nevertheless was a\ngood point to raise.\n\nThanks for carefully reading the patches through.\n\n"}]}