git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com
Cc: git@vger.kernel.org, David.Turner@twosigma.com,
	bmwill@google.com, Stefan Beller <sbeller@google.com>
Subject: [PATCH 6/6] rm: add absorb a submodules git dir before deletion
Date: Mon, 12 Dec 2016 17:40:55 -0800	[thread overview]
Message-ID: <20161213014055.14268-7-sbeller@google.com> (raw)
In-Reply-To: <20161213014055.14268-1-sbeller@google.com>

When deleting a submodule we need to keep the actual git directory around,
such that we do not lose local changes in there and at a later checkout
of the submodule we don't need to clone it again.

Implement `depopulate_submodule`, that migrates the git directory before
deletion of a submodule and afterwards the equivalent of "rm -rf", which
is already found in entry.c, so expose that and for clarity add a suffix
"_or_dir" to it.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/rm.c  | 18 ++++++------------
 cache.h       |  2 ++
 entry.c       |  5 +++++
 submodule.c   | 31 +++++++++++++++++++++++++++++++
 submodule.h   |  6 ++++++
 t/t3600-rm.sh | 41 ++++++++++++++++-------------------------
 6 files changed, 66 insertions(+), 37 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index fdd7183f61..f8c5e9b6c6 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -400,18 +400,12 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 						continue;
 					}
 				} else {
-					strbuf_reset(&buf);
-					strbuf_addstr(&buf, path);
-					if (!remove_dir_recursively(&buf, 0)) {
-						removed = 1;
-						if (!remove_path_from_gitmodules(path))
-							gitmodules_modified = 1;
-						strbuf_release(&buf);
-						continue;
-					} else if (!file_exists(path))
-						/* Submodule was removed by user */
-						if (!remove_path_from_gitmodules(path))
-							gitmodules_modified = 1;
+					if (file_exists(path))
+						depopulate_submodule(path);
+					removed = 1;
+					if (!remove_path_from_gitmodules(path))
+						gitmodules_modified = 1;
+					continue;
 					/* Fallthrough and let remove_path() fail. */
 				}
 			}
diff --git a/cache.h b/cache.h
index a50a61a197..b645ca2f9a 100644
--- a/cache.h
+++ b/cache.h
@@ -2018,4 +2018,6 @@ void sleep_millisec(int millisec);
  */
 void safe_create_dir(const char *dir, int share);
 
+extern void remove_directory_or_die(struct strbuf *path);
+
 #endif /* CACHE_H */
diff --git a/entry.c b/entry.c
index c6eea240b6..02c4ac9f22 100644
--- a/entry.c
+++ b/entry.c
@@ -73,6 +73,11 @@ static void remove_subtree(struct strbuf *path)
 		die_errno("cannot rmdir '%s'", path->buf);
 }
 
+void remove_directory_or_die(struct strbuf *path)
+{
+	remove_subtree(path);
+}
+
 static int create_file(const char *path, unsigned int mode)
 {
 	mode = (mode & 0100) ? 0777 : 0666;
diff --git a/submodule.c b/submodule.c
index e42efa2337..3770ecb7b9 100644
--- a/submodule.c
+++ b/submodule.c
@@ -308,6 +308,37 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
 	strbuf_release(&sb);
 }
 
+void depopulate_submodule(const char *path)
+{
+	struct strbuf pathbuf = STRBUF_INIT;
+	char *dot_git = xstrfmt("%s/.git", path);
+
+	/* Is it populated? */
+	if (!resolve_gitdir(dot_git))
+		goto out;
+
+	/* Does it have a .git directory? */
+	if (!submodule_uses_gitfile(path)) {
+		absorb_git_dir_into_superproject("", path,
+			ABSORB_GITDIR_RECURSE_SUBMODULES);
+
+		if (!submodule_uses_gitfile(path)) {
+			/*
+			 * We should be using a gitfile by now. Let's double
+			 * check as losing the git dir would be fatal.
+			 */
+			die("BUG: could not absorb git directory for '%s'", path);
+		}
+	}
+
+	strbuf_addstr(&pathbuf, path);
+	remove_directory_or_die(&pathbuf);
+
+out:
+	strbuf_release(&pathbuf);
+	free(dot_git);
+}
+
 /* Helper function to display the submodule header line prior to the full
  * summary output. If it can locate the submodule objects directory it will
  * attempt to lookup both the left and right commits and put them into the
diff --git a/submodule.h b/submodule.h
index 3ed3aa479a..516e377a12 100644
--- a/submodule.h
+++ b/submodule.h
@@ -53,6 +53,12 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,
 		const char *del, const char *add, const char *reset,
 		const struct diff_options *opt);
 extern void set_config_fetch_recurse_submodules(int value);
+
+/*
+ * Removes a submodule from a given path. When the submodule contains its
+ * git directory instead of a gitlink, migrate that first into the superproject.
+ */
+extern void depopulate_submodule(const char *path);
 extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
 extern int fetch_populated_submodules(const struct argv_array *options,
 			       const char *prefix, int command_line_option,
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 5e5a16c863..5aa6db584c 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -569,26 +569,22 @@ test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '
 	test_cmp expect actual
 '
 
-test_expect_success 'rm of a populated submodule with a .git directory fails even when forced' '
+test_expect_success 'rm of a populated submodule with a .git directory migrates git dir' '
 	git checkout -f master &&
 	git reset --hard &&
 	git submodule update &&
 	(cd submod &&
 		rm .git &&
 		cp -R ../.git/modules/sub .git &&
-		GIT_WORK_TREE=. git config --unset core.worktree
+		GIT_WORK_TREE=. git config --unset core.worktree &&
+		rm -r ../.git/modules/sub
 	) &&
-	test_must_fail git rm submod &&
-	test -d submod &&
-	test -d submod/.git &&
-	git status -s -uno --ignore-submodules=none >actual &&
-	! test -s actual &&
-	test_must_fail git rm -f submod &&
-	test -d submod &&
-	test -d submod/.git &&
+	git rm submod 2>output.err &&
+	! test -d submod &&
+	! test -d submod/.git &&
 	git status -s -uno --ignore-submodules=none >actual &&
-	! test -s actual &&
-	rm -rf submod
+	test -s actual &&
+	test_i18ngrep Migrating output.err
 '
 
 cat >expect.deepmodified <<EOF
@@ -667,27 +663,22 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
 	git submodule update --recursive &&
 	(cd submod/subsubmod &&
 		rm .git &&
-		cp -R ../../.git/modules/sub/modules/sub .git &&
+		mv ../../.git/modules/sub/modules/sub .git &&
 		GIT_WORK_TREE=. git config --unset core.worktree
 	) &&
-	test_must_fail git rm submod &&
-	test -d submod &&
-	test -d submod/subsubmod/.git &&
-	git status -s -uno --ignore-submodules=none >actual &&
-	! test -s actual &&
-	test_must_fail git rm -f submod &&
-	test -d submod &&
-	test -d submod/subsubmod/.git &&
+	git rm submod 2>output.err &&
+	! test -d submod &&
+	! test -d submod/subsubmod/.git &&
 	git status -s -uno --ignore-submodules=none >actual &&
-	! test -s actual &&
-	rm -rf submod
+	test -s actual &&
+	test_i18ngrep Migrating output.err
 '
 
 test_expect_success 'checking out a commit after submodule removal needs manual updates' '
-	git commit -m "submodule removal" submod &&
+	git commit -m "submodule removal" submod .gitmodules &&
 	git checkout HEAD^ &&
 	git submodule update &&
-	git checkout -q HEAD^ 2>actual &&
+	git checkout -q HEAD^ &&
 	git checkout -q master 2>actual &&
 	test_i18ngrep "^warning: unable to rmdir submod:" actual &&
 	git status -s submod >actual &&
-- 
2.11.0.rc2.35.g7af3268


  parent reply	other threads:[~2016-12-13  1:41 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-13  1:40 [PATCH 0/6] git-rm absorbs submodule git directory before deletion Stefan Beller
2016-12-13  1:40 ` [PATCH 1/6] submodule.h: add extern keyword to functions Stefan Beller
2016-12-13  1:40 ` [PATCH 2/6] submodule: modernize ok_to_remove_submodule to use argv_array Stefan Beller
2016-12-13  1:40 ` [PATCH 3/6] submodule: add flags to ok_to_remove_submodule Stefan Beller
2016-12-13  1:40 ` [PATCH 4/6] ok_to_remove_submodule: absorb the submodule git dir Stefan Beller
2016-12-13  1:40 ` [PATCH 5/6] t3600: slightly modernize style Stefan Beller
2016-12-13  1:40 ` Stefan Beller [this message]
2016-12-13  3:28   ` [PATCH 6/6] rm: add absorb a submodules git dir before deletion brian m. carlson
2016-12-13 17:51     ` Stefan Beller
2016-12-13 18:06   ` David Turner
2016-12-13  7:28 ` [PATCH 0/6] git-rm absorbs submodule git directory " Junio C Hamano
2016-12-13 17:55   ` Stefan Beller
2016-12-13 18:53     ` Junio C Hamano
2016-12-13 19:07       ` Stefan Beller
2016-12-13 19:11         ` Junio C Hamano
2016-12-13 19:13           ` Stefan Beller
2016-12-13 19:38             ` Stefan Beller
2016-12-13 19:47               ` Junio C Hamano
2016-12-13 20:09                 ` Stefan Beller
2016-12-13 20:22                   ` Stefan Beller
2016-12-13 19:54               ` 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=20161213014055.14268-7-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=David.Turner@twosigma.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).