git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: sb/submodule-move-nested breaks t7411 under GIT_FSMONITOR_TEST

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Sep 6, 2018, 12:31 UTC
Message-ID
<87tvn2remn.fsf@evledraar.gmail.com>
In-Reply-To
<CAGZ79kae4k=uLx-oX5emxas4KrqObzQhzgir0coOSBzzpO8APw@mail.gmail.com>
On Fri, May 25 2018, Stefan Beller wrote:
Show 126 quoted lines
> On Fri, May 25, 2018 at 5:28 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> On Thu, May 17 2018, Junio C Hamano wrote:
>>
>>> * sb/submodule-move-nested (2018-03-29) 6 commits
>>>   (merged to 'next' on 2018-04-25 at 86b177433a)
>>>  + submodule: fixup nested submodules after moving the submodule
>>>  + submodule-config: remove submodule_from_cache
>>>  + submodule-config: add repository argument to submodule_from_{name, path}
>>>  + submodule-config: allow submodule_free to handle arbitrary repositories
>>>  + grep: remove "repo" arg from non-supporting funcs
>>>  + submodule.h: drop declaration of connect_work_tree_and_git_dir
>>>
>>>  Moving a submodule that itself has submodule in it with "git mv"
>>>  forgot to make necessary adjustment to the nested sub-submodules;
>>>  now the codepath learned to recurse into the submodules.
>>
>> I didn't spot this earlier because I don't test this a lot, but I've
>> bisected the following breakage down to da62f786d2 ("submodule: fixup
>> nested submodules after moving the submodule", 2018-03-28) (and manually
>> confirmed by reverting). On Linux both Debian & CentOS I get tests 3 and
>> 4 failing with:
>>
>>      GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all ./t7411-submodule-config.sh
>>
>> -v -x output follows:
>>
>> expecting success:
>>         mkdir submodule &&
>>         (cd submodule &&
>>                 git init &&
>>                 echo a >a &&
>>                 git add . &&
>>                 git commit -ma
>>         ) &&
>>         mkdir super &&
>>         (cd super &&
>>                 git init &&
>>                 git submodule add ../submodule &&
>>                 git submodule add ../submodule a &&
>>                 git commit -m "add as submodule and as a" &&
>>                 git mv a b &&
>>                 git commit -m "move a to b"
>>         )
>
> when you add a test_pause here and dump the
> state of the setup, then it can be observed that when the fsmonitor is active
> the last commit is different; without fsmonitor the moved gitlink and the change
> to the .gitmodules file is part of the commit, i.e.
>
> $ git -C super show
>         commit d3d90b70a01bd17d026f75a803c8b65f5903a7c0 (HEAD -> master)
>         Author: A U Thor <author@example.com>
>         Date:   Fri May 25 19:21:58 2018 +0000
>
>             move a to b
>
>         diff --git a/.gitmodules b/.gitmodules
>         index 3f4d474..6149210 100644
>         --- a/.gitmodules
>         +++ b/.gitmodules
>         @@ -2,5 +2,5 @@
>           path = submodule
>           url = ../submodule
>          [submodule "a"]
>         - path = a
>         + path = b
>           url = ../submodule
>         diff --git a/a b/b
>         similarity index 100%
>         rename from a
>         rename to b
> When running with the fsmonitor:
>
> $ git -C super show
>         commit 57022a92acf46f303498c045440ec099cbc35a2d (HEAD -> master)
>         Author: A U Thor <author@example.com>
>         Date:   Fri May 25 19:22:52 2018 +0000
>
>             move a to b
>
>         diff --git a/a b/b
>         similarity index 100%
>         rename from a
>         rename to b
> $ git -C super diff
>         diff --git a/.gitmodules b/.gitmodules
>         index 3f4d474..6149210 100644
>         --- a/.gitmodules
>         +++ b/.gitmodules
>         @@ -2,5 +2,5 @@
>           path = submodule
>           url = ../submodule
>          [submodule "a"]
>         - path = a
>         + path = b
>           url = ../submodule
>
> This hints at a problem with git commit;
>
> I tried adding test_tick, to unconfuse the fsmonitor, but that doesn't help,
> digging further, the problem is in the git mv command, which fails to
> add the change in
> .gitmodules to the index.
>
> Adding the verbose flag to stage_updated_gitmodules() that is called by
> git-mv very late in the game, such that
>
> void stage_updated_gitmodules(struct index_state *istate)
> {
>     trace_printf("staging .gitmodules files");
>     if (add_file_to_index(istate, GITMODULES_FILE, ADD_CACHE_VERBOSE))
>         die(_("staging updated .gitmodules failed"));
> }
>
> We would get a message if the .gitmodules file is staged correctly, as
> add_file_to_index() that calls add_to_index that would print
>
>     if (verbose && !was_same)
>         printf("add '%s'\n", path);
>
> I could not see that message, so I suspect, that there is something
> racy.
>
> Will debug further.

I spotted this again after testing the split index (see https://public-inbox.org/git/87va7ireuu.fsf@evledraar.gmail.com/) and was testing the fsmonitor test mode as well.

So gentle *poke*: Did you get anywhere with debugging this? It's still failing on "master" now.

Previous: Stefan BellerNext: Stefan Beller
Message 87 of 95 in “What's cooking in git.git (May 2018, #02; Thu, 17)”
  1. Junio C HamanoMay 17, 2018
  2. jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))Kaartic Sivaraam, May 17, 2018
  3. Ævar Arnfjörð BjarmasonMay 17, 2018
  4. Kaartic SivaraamMay 17, 2018
  5. Ævar Arnfjörð BjarmasonMay 17, 2018
  6. Jeff KingMay 17, 2018
  7. jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))Kaartic Sivaraam, May 24, 2018
  8. Jeff KingMay 24, 2018
  9. branch: issue "-l" deprecation warning after pager startsJeff King, May 24, 2018
  10. Junio C HamanoMay 25, 2018
  11. Jeff KingMay 25, 2018
  12. Junio C HamanoMay 25, 2018
  13. Junio C HamanoMay 25, 2018
  14. Jeff KingMay 25, 2018
  15. Junio C HamanoMay 26, 2018
  16. 0/3 usage: prefix all lines in `vreportf()`, not just the firstMartin Ågren, May 25, 2018
  17. 1/3 usage: extract `prefix_suffix_lines()` from `advise()`Martin Ågren, May 25, 2018
  18. Junio C HamanoMay 28, 2018
  19. Duy NguyenMay 28, 2018
  20. Jeff KingMay 29, 2018
  21. Jeff KingMay 29, 2018
  22. Junio C HamanoMay 30, 2018
  23. Junio C HamanoMay 30, 2018
  24. Martin ÅgrenMay 30, 2018
  25. Jeff KingMay 31, 2018
  26. 2/3 usage: prefix all lines in `vreportf()`, not just the firstMartin Ågren, May 25, 2018
  27. Junio C HamanoMay 28, 2018
  28. Duy NguyenMay 28, 2018
  29. Junio C HamanoMay 28, 2018
  30. Martin ÅgrenMay 29, 2018
  31. Junio C HamanoMay 29, 2018
  32. Martin ÅgrenMay 29, 2018
  33. Junio C HamanoMay 29, 2018
  34. Duy NguyenMay 29, 2018
  35. Martin ÅgrenMay 30, 2018
  36. Jeff KingMay 29, 2018
  37. Martin ÅgrenMay 30, 2018
  38. 3/3 usage: translate the "error: "-prefix and othersMartin Ågren, May 25, 2018
  39. Junio C HamanoMay 26, 2018
  40. Junio C HamanoMay 26, 2018
  41. Jeff KingMay 29, 2018
  42. Jeff KingMay 29, 2018
  43. Junio C HamanoMay 30, 2018
  44. Jeff KingMay 31, 2018
  45. Kaartic SivaraamMay 26, 2018
  46. Duy NguyenJun 2, 2018
  47. Jeff KingJun 2, 2018
  48. Kaartic SivaraamMay 26, 2018
  49. Jeff KingMay 29, 2018
  50. Junio C HamanoMay 30, 2018
  51. Jeff KingMay 31, 2018
  52. Junio C HamanoJun 1, 2018
  53. Kaartic.SivaraamMay 31, 2018
  54. Derrick StoleeMay 17, 2018
  55. Stefan BellerMay 17, 2018
  56. 0/2 Reroll 2 last commits of sb/object-store-replaceStefan Beller, May 17, 2018
  57. 1/2 object.c: free replace map in raw_object_store_clearStefan Beller, May 17, 2018
  58. 2/2 replace-object.c: remove the_repository from prepare_replace_objectStefan Beller, May 17, 2018
  59. merge-recursive: give notice when submodule commit gets fast-forwardedStefan Beller, May 17, 2018
  60. 0/1 rebased: inform about auto submodule ffLeif Middelschulte, May 18, 2018
  61. 0/1 rebased: inform about auto submodule ffLeif Middelschulte, May 18, 2018
  62. 1/1 Inform about fast-forwarding of submodules during mergeLeif Middelschulte, May 18, 2018
  63. Elijah NewrenMay 18, 2018
  64. Junio C HamanoMay 21, 2018
  65. 0/8 Reroll of sb/diff-color-move-moreStefan Beller, May 17, 2018
  66. 1/8 xdiff/xdiff.h: remove unused flagsStefan Beller, May 17, 2018
  67. 2/8 xdiff/xdiffi.c: remove unneeded function declarationsStefan Beller, May 17, 2018
  68. 4/8 diff.c: adjust hash function signature to match hashmap expectationStefan Beller, May 17, 2018
  69. 5/8 diff.c: add a blocks mode for moved code detectionStefan Beller, May 17, 2018
  70. 7/8 diff.c: add --color-moved-ignore-space-delta optionStefan Beller, May 17, 2018
  71. 8/8 diff: color-moved white space handling options imply color-movedStefan Beller, May 17, 2018
  72. 3/8 diff.c: do not pass diff options as keydata to hashmapStefan Beller, May 17, 2018
  73. 6/8 diff.c: decouple white space treatment from move detection algorithmStefan Beller, May 17, 2018
  74. Simon RuderichMay 18, 2018
  75. Stefan BellerMay 18, 2018
  76. Jonathan TanMay 17, 2018
  77. Jacob KellerJun 7, 2018
  78. Junio C HamanoMay 17, 2018
  79. Stefan BellerMay 17, 2018
  80. Junio C HamanoMay 17, 2018
  81. Stefan BellerMay 17, 2018
  82. brian m. carlsonMay 21, 2018
  83. Stefan BellerMay 21, 2018
  84. sb/submodule-move-nested breaks t7411 under GIT_FSMONITOR_TESTÆvar Arnfjörð Bjarmason, May 25, 2018
  85. Stefan BellerMay 25, 2018
  86. Stefan BellerMay 25, 2018
  87. Ævar Arnfjörð BjarmasonSep 6, 2018
  88. Stefan BellerSep 6, 2018
  89. Ben PeartSep 6, 2018
  90. Stefan BellerSep 6, 2018
  91. git-mv: allow submodules and fsmonitor to work togetherStefan Beller, Sep 6, 2018
  92. Ben PeartSep 10, 2018
  93. git-mv: allow submodules and fsmonitor to work togetherBen Peart, Sep 10, 2018
  94. Stefan BellerSep 10, 2018
  95. Ben PeartSep 10, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.