From: Shourya Shukla <periperidip@gmail.com>
To: Philippe Blain <levraiphilippeblain@gmail.com>
Cc: gitster@pobox.com, christian.couder@gmail.com,
levraiphilippeblain@gmail.com, git@vger.kernel.org
Subject: Re: [PATCH 1/2] rm: changes in the '.gitmodules' are staged after using '--cached'
Date: Fri, 19 Feb 2021 20:49:13 +0530 [thread overview]
Message-ID: <20210219151913.GA6254@konoha> (raw)
In-Reply-To: <0577f84b-f594-6b8a-76a2-29fb9453ee25@gmail.com>
On 18/02 03:14, Philippe Blain wrote:
> 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.
> 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.
> > 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.
next prev parent reply other threads:[~2021-02-19 15:20 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-18 18:49 [RFC][PATCH 0/2] rm: changes in the '.gitmodules' are staged after using '--cached' Shourya Shukla
2021-02-18 18:49 ` [PATCH 1/2] " Shourya Shukla
2021-02-18 20:14 ` Philippe Blain
2021-02-18 20:39 ` Philippe Blain
2021-02-19 15:19 ` Shourya Shukla [this message]
2021-02-18 22:03 ` Junio C Hamano
2021-02-19 15:24 ` Shourya Shukla
2021-02-20 3:31 ` Junio C Hamano
2021-02-18 18:49 ` [PATCH 2/2] t3600: amend test 46 to check for '.gitmodules' modification Shourya Shukla
2021-02-18 20:21 ` Philippe Blain
2021-02-22 17:26 ` [PATCH v2 0/1] rm: stage submodule removal from '.gitmodules' Shourya Shukla
2021-02-22 17:26 ` [PATCH v2 1/1] rm: stage submodule removal from '.gitmodules' when using '--cached' Shourya Shukla
2021-02-22 18:58 ` Junio C Hamano
2021-03-05 17:58 ` Shourya Shukla
2021-03-05 21:39 ` Junio C Hamano
2021-02-22 19:29 ` Junio C Hamano
2021-03-07 16:46 ` Shourya Shukla
2021-03-07 20:29 ` Junio C Hamano
2021-03-09 7:13 ` Shourya Shukla
2021-03-09 20:47 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210219151913.GA6254@konoha \
--to=periperidip@gmail.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=levraiphilippeblain@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).