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

[Shourya Shukla <periperidip@xxxxxxxxx>]

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.

Reported-by: Javier Mora <javier.moradesambricio@xxxxxxx>
Signed-off-by: Shourya Shukla <periperidip@xxxxxxxxx>
---
 builtin/rm.c | 48 +++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 4858631e0f..0b74f50bfe 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -254,7 +254,7 @@ static struct option builtin_rm_options[] = {
 int cmd_rm(int argc, const char **argv, const char *prefix)
 {
 	struct lock_file lock_file = LOCK_INIT;
-	int i;
+	int i, removed = 0, gitmodules_modified = 0;
 	struct pathspec pathspec;
 	char *seen;
 
@@ -365,30 +365,32 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	if (show_only)
 		return 0;
 
-	/*
-	 * Then, unless we used "--cached", remove the filenames from
-	 * the workspace. If we fail to remove the first one, we
-	 * abort the "git rm" (but once we've successfully removed
-	 * any file at all, we'll go ahead and commit to it all:
-	 * by then we've already committed ourselves and can't fail
-	 * in the middle)
-	 */
-	if (!index_only) {
-		int removed = 0, gitmodules_modified = 0;
-		struct strbuf buf = STRBUF_INIT;
-		for (i = 0; i < list.nr; i++) {
-			const char *path = list.entry[i].name;
-			if (list.entry[i].is_submodule) {
+	for (i = 0; i < list.nr; i++) {
+		const char *path = list.entry[i].name;
+		if (list.entry[i].is_submodule) {
+			/*
+			 * Then, unless we used "--cached", remove the filenames from
+			 * the workspace. If we fail to remove the first one, we
+			 * abort the "git rm" (but once we've successfully removed
+			 * any file at all, we'll go ahead and commit to it all:
+			 * by then we've already committed ourselves and can't fail
+			 * in the middle)
+			 */
+			if (!index_only) {
+				struct strbuf buf = STRBUF_INIT;
 				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;
+				strbuf_release(&buf);
 			}
+			if (!remove_path_from_gitmodules(path))
+				gitmodules_modified = 1;
+			continue;
+		}
+		if (!index_only) {
 			if (!remove_path(path)) {
 				removed = 1;
 				continue;
@@ -396,11 +398,15 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			if (!removed)
 				die_errno("git rm: '%s'", path);
 		}
-		strbuf_release(&buf);
-		if (gitmodules_modified)
-			stage_updated_gitmodules(&the_index);
 	}
 
+	/*
+	 * Remove the entry of the submodule from the ".gitmodules" irrespective
+	 * whether "--cached" was passed or not.
+	 */
+	if (gitmodules_modified)
+		stage_updated_gitmodules(&the_index);
+
 	if (write_locked_index(&the_index, &lock_file,
 			       COMMIT_LOCK | SKIP_IF_UNCHANGED))
 		die(_("Unable to write new index file"));
-- 
2.25.1