| Message ID | 20180906203444.162213-1-sbeller@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-mv: allow submodules and fsmonitor to work together | expand |
On 9/6/2018 4:34 PM, Stefan Beller wrote: > It was reported that > > GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all ./t7411-submodule-config.sh > > breaks as the .gitmodules file is modified and staged after the fsmonitor > considers it clean. Mark the .gitmodules file to be not clean before > staging. > > Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > Inspired-by: Ben Peart <benpeart@microsoft.com> > Signed-off-by: Stefan Beller <sbeller@google.com> > --- > > I am not quite sure if this is the correct approach and handling of the > fsmonitor API, but it unbreaks the test. > >> Just naively adding mark_fsmonitor_invalid doesn't work, as then ... > > Adding it before the staging, works. > > Please double check! I took a look at this bug/patch and wondered why add_file_to_index() wasn't properly handling the .gitmodules file. On investigation, I chased it down to what looks like a faulty test in is_staging_gitmodules_ok(). I believe the following is a better patch for this bug: diff --git a/submodule.c b/submodule.c index 50cbf5f13e..1e7194af28 100644 --- a/submodule.c +++ b/submodule.c @@ -65,8 +65,7 @@ int is_staging_gitmodules_ok(struct index_state *istate) if ((pos >= 0) && (pos < istate->cache_nr)) { struct stat st; if (lstat(GITMODULES_FILE, &st) == 0 && - ie_match_stat(istate, istate->cache[pos], &st, - CE_MATCH_IGNORE_FSMONITOR) & DATA_CHANGED) + ie_match_stat(istate, istate->cache[pos], &st, 0) & DATA_CHANGED) return 0; } Please double check but I just don't understand why the .gitmodules file should force the fsmonitor data to be ignored. This flag was added to enable proper behavior in the preload_thread() logic and I don't believe it is appropriate here. Ben > > Thanks, > Stefan > > submodule.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/submodule.c b/submodule.c > index 50cbf5f13ed..56b0d5fe24e 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -22,6 +22,7 @@ > #include "worktree.h" > #include "parse-options.h" > #include "object-store.h" > +#include "fsmonitor.h" > > static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; > static struct string_list changed_submodule_names = STRING_LIST_INIT_DUP; > @@ -149,6 +150,15 @@ int remove_path_from_gitmodules(const char *path) > > void stage_updated_gitmodules(struct index_state *istate) > { > + struct cache_entry *ce; > + int pos; > + > + pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE)); > + ce = (0 <= pos) ? istate->cache[pos] : NULL; > + > + if (ce) > + mark_fsmonitor_invalid(istate, ce); > + > if (add_file_to_index(istate, GITMODULES_FILE, 0)) > die(_("staging updated .gitmodules failed")); > } >
diff --git a/submodule.c b/submodule.c index 50cbf5f13ed..56b0d5fe24e 100644 --- a/submodule.c +++ b/submodule.c @@ -22,6 +22,7 @@ #include "worktree.h" #include "parse-options.h" #include "object-store.h" +#include "fsmonitor.h" static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; static struct string_list changed_submodule_names = STRING_LIST_INIT_DUP; @@ -149,6 +150,15 @@ int remove_path_from_gitmodules(const char *path) void stage_updated_gitmodules(struct index_state *istate) { + struct cache_entry *ce; + int pos; + + pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE)); + ce = (0 <= pos) ? istate->cache[pos] : NULL; + + if (ce) + mark_fsmonitor_invalid(istate, ce); + if (add_file_to_index(istate, GITMODULES_FILE, 0)) die(_("staging updated .gitmodules failed")); }