Hello all, I was lurking around 'gitgitgadget/git' when I saw this potential BUG added by Phillipe Blaine (reported by Javier Mora): https://github.com/gitgitgadget/git/issues/750 Link to the original mail by Javier: https://lore.kernel.org/git/ea91c2ea29064079914f6a522db5115a@xxxxxxxxxxxxxxxxxxxx/ In brief, 'git rm' does not stage the changed '.gitmodules' file when we use the '--cached' option. Technically speaking, Git used to behave this way only and hence this is not an unknown case. The test 45 of 't3600-rm.sh' already is prepared for this scenario and checks for exactly the scenario as Javier describes. So, my question is, do we need to fix this to make sure that the changed '.gitmodules' is staged? I feel that we should because: since the SM becomes irrelevant after executing 'git rm --cached', it's entry in '.gitmodules' is a plain burden and is of no practical use. The fault is in this section of 'builtin/rm.c': https://github.com/git/git/blob/v2.30.0/builtin/rm.c#L378-L402 This part: const char *path = list.entry[i].name; if (list.entry[i].is_submodule) { strbuf_reset(&buf); strbuf_addstr(&buf, path); if (remove_dir_recursively(&buf, 0)) die(_("could not remove '%s'"), path); removed = 1; if (!remove_path_from_gitmodules(path)) gitmodules_modified = 1; continue; } Needs to be executed irrespective of whether '--cached' is passed to the command or not. In particular, the following if-statement is of utmost importance: if (!remove_path_from_gitmodules(path)) gitmodules_modified = 1; Since the variable 'gitmodules_modified' is 0 when we pass 'cached', it is not staged later here: if (gitmodules_modified) stage_updated_gitmodules(&the_index); And its entry is not removed from the file. What should be done about this? I would appreciate your opinions. Regards, Shourya Shukla
