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

Re: [PATCH 1/2] rm: changes in the '.gitmodules' are staged after using '--cached'

From
SSShourya Shukla <periperidip@gmail.com>
Date
Feb 19, 2021, 15:19 UTC
Message-ID
<20210219151913.GA6254@konoha>
In-Reply-To
<0577f84b-f594-6b8a-76a2-29fb9453ee25@gmail.com>
On 18/02 03:14, Philippe Blain wrote:
Show 14 quoted lines
> Hello Shourya,
> 
> Le 2021-02-18 à 13:49, Shourya Shukla a écrit :
> > Earlier, on doing a 'git rm --cached <submodule>' did not modify the
> > '.gitmodules' entry of the submodule in question hence the file was not
> > staged. Change this behaviour to remove the entry of the submodule from
> > the '.gitmodules', something which might be more expected of the
> > command.
> 
> We prefer using the imperative mood for the commit message title,
> the present tense for describing the actual state of the code,
> and finally the imperative mood again to give order to the code base
> to change its behaviour [1]. So something like the following would fit more
> into the project's conventions:
I have no idea how I missed that one. Apologies, will make the change.
Show 11 quoted lines
>     rm: stage submodule removal from '.gitmodules' when using '--cached'
> 
>     Currently, using 'git rm --cached <submodule>' removes submodule <submodule> from the index
>     and leaves the submodule working tree intact in the superproject working tree,
>     but does not stage any changes to the '.gitmodules' file, in contrast to
>     'git rm <submodule>', which removes both the submodule and its configuration
>     in '.gitmodules' from the worktree and index.
>     Fix this inconsistency by also staging the removal of the configuration of the
>     submodule from the '.gitmodules' file, leaving the worktree copy intact, a behaviour
>     which is more in line with what might be expected when using '--cached'.
> 
Okay. I will use the above message.
Show 38 quoted lines
> However, this is *not* what you patch does; it also removes the relevant
> section from the '.gitmodules' file *in the worktree*, which is not acceptable
> because it is exactly contrary to what '--cached' means.
> 
> This was verified by running Javier's demonstration script that I included in the
> Gitgitgadget issue [2], which I copy here:
> 
> 
> ~~~
> rm -rf some_submodule top_repo
> 
> mkdir some_submodule
> cd some_submodule
> git init
> echo hello > hello.txt
> git add hello.txt
> git commit -m 'First commit of submodule'
> cd ..
> mkdir top_repo
> cd top_repo
> git init
> echo world > world.txt
> git add world.txt
> git commit -m 'First commit of top repo'
> git submodule add ../some_submodule
> git status  # both some_submodule and .gitmodules staged
> git commit -m 'Added submodule'
> git rm --cached some_submodule
> git status  # only some_submodule staged
> ~~~
> 
> With your changes, at the end '.gitmodules' is modified in both the
> worktree and the index, whereas we would want it to be modified
> *only* in the index.
> 
> And we would want it to be staged for deletion (and only deleting the config
> entry and keeping an empty ".gitmodules' file in the index)
> if the user is removing the only submodule in the superproject.
Correct.
Show 10 quoted lines
> >   builtin/rm.c | 48 +++++++++++++++++++++++++++---------------------
> >   1 file changed, 27 insertions(+), 21 deletions(-)
> > 
> 
> Once implemeted correctly (leaving the worktree version of '.gitmodules'
> intact), that patch should also change the documentation to stay up-to-date,
> since the "Submodules" section of Documentation/git-rm.txt states [3]:
> 
>     If it exists the submodule.<name> section in the gitmodules[5] file will
>     also be removed and that file will be staged (unless --cached or -n are used).

Understood. I have to let the working tree '.gitmodules' be left as-is and only change the copy in the index.

Previous: Philippe BlainNext: Junio C Hamano
Message 5 of 20 in “rm: changes in the '.gitmodules' are staged after using '--cached'”
  1. Shourya ShuklaFeb 18, 2021
  2. 1/2 rm: changes in the '.gitmodules' are staged after using '--cached'Shourya Shukla, Feb 18, 2021
  3. Philippe BlainFeb 18, 2021
  4. Philippe BlainFeb 18, 2021
  5. Shourya ShuklaFeb 19, 2021
  6. Junio C HamanoFeb 18, 2021
  7. Shourya ShuklaFeb 19, 2021
  8. Junio C HamanoFeb 20, 2021
  9. 2/2 t3600: amend test 46 to check for '.gitmodules' modificationShourya Shukla, Feb 18, 2021
  10. Philippe BlainFeb 18, 2021
  11. 0/1 rm: stage submodule removal from '.gitmodules'Shourya Shukla, Feb 22, 2021
  12. 1/1 rm: stage submodule removal from '.gitmodules' when using '--cached'Shourya Shukla, Feb 22, 2021
  13. Junio C HamanoFeb 22, 2021
  14. Shourya ShuklaMar 5, 2021
  15. Junio C HamanoMar 5, 2021
  16. Junio C HamanoFeb 22, 2021
  17. Shourya ShuklaMar 7, 2021
  18. Junio C HamanoMar 7, 2021
  19. Shourya ShuklaMar 9, 2021
  20. Junio C HamanoMar 9, 2021

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.