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

On 18/02 03:14, Philippe Blain wrote:

Hello Shourya,

Le 2021-02-18 à 13:49, Shourya Shukla a écrit :

quoted

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.

    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.

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.

quoted

  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.