git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Collingbourne <peter@pcc.me.uk>
To: git@vger.kernel.org
Cc: Peter Collingbourne <peter@pcc.me.uk>
Subject: [PATCH 09/12] Add a mode parameter to the remove_path function
Date: Fri, 26 Mar 2010 15:25:37 +0000	[thread overview]
Message-ID: <1269617140-7827-10-git-send-email-peter@pcc.me.uk> (raw)
In-Reply-To: <1269617140-7827-1-git-send-email-peter@pcc.me.uk>

This patch adds a mode parameter to remove_path which determines
whether unlink or rmdir is used.  All calls to remove_path have
been modified to supply the mode parameter.

This patch also adds a test case for a bug fixed by the addition
of the mode parameter to remove_path.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 builtin/apply.c            |    2 +-
 builtin/rm.c               |    3 ++-
 dir.c                      |    4 ++--
 dir.h                      |    2 +-
 merge-recursive.c          |   27 ++++++++++++++++-----------
 t/t7405-submodule-merge.sh |   13 +++++++++++++
 6 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 65a594c..1e9e861 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3145,7 +3145,7 @@ static void remove_file(struct patch *patch, int rmdir_empty)
 	}
 	if (!cached) {
 		if (!remove_or_warn(patch->old_mode, patch->old_name) && rmdir_empty) {
-			remove_path(patch->old_name);
+			remove_path(patch->old_mode, patch->old_name);
 		}
 	}
 }
diff --git a/builtin/rm.c b/builtin/rm.c
index 61ec2cf..6ac5114 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -256,7 +256,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		int removed = 0;
 		for (i = 0; i < list.nr; i++) {
 			const char *path = list.name[i];
-			if (!remove_path(path)) {
+			unsigned int mode = list.mode[i];
+			if (!remove_path(mode, path)) {
 				removed = 1;
 				continue;
 			}
diff --git a/dir.c b/dir.c
index cb83332..2d9cd9a 100644
--- a/dir.c
+++ b/dir.c
@@ -1047,11 +1047,11 @@ void setup_standard_excludes(struct dir_struct *dir)
 		add_excludes_from_file(dir, excludes_file);
 }
 
-int remove_path(const char *name)
+int remove_path(unsigned int mode, const char *name)
 {
 	char *slash;
 
-	if (unlink(name) && errno != ENOENT)
+	if ((S_ISGITLINK(mode) ? rmdir(name) : unlink(name)) && errno != ENOENT)
 		return -1;
 
 	slash = strrchr(name, '/');
diff --git a/dir.h b/dir.h
index 3bead5f..0e48d2a 100644
--- a/dir.h
+++ b/dir.h
@@ -98,6 +98,6 @@ extern void setup_standard_excludes(struct dir_struct *dir);
 extern int remove_dir_recursively(struct strbuf *path, int flag);
 
 /* tries to remove the path with empty directories along it, ignores ENOENT */
-extern int remove_path(const char *path);
+extern int remove_path(unsigned int mode, const char *path);
 
 #endif
diff --git a/merge-recursive.c b/merge-recursive.c
index 87232b8..c5b4149 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -399,7 +399,7 @@ static int update_stages(const char *path, struct diff_filespec *o,
 }
 
 static int remove_file(struct merge_options *o, int clean,
-		       const char *path, int no_wd)
+		       unsigned int mode, const char *path, int no_wd)
 {
 	int update_cache = o->call_depth || clean;
 	int update_working_directory = !o->call_depth && !no_wd;
@@ -409,7 +409,7 @@ static int remove_file(struct merge_options *o, int clean,
 			return -1;
 	}
 	if (update_working_directory) {
-		if (remove_path(path))
+		if (remove_path(mode, path))
 			return -1;
 	}
 	return 0;
@@ -734,6 +734,8 @@ static void conflict_rename_rename(struct merge_options *o,
 {
 	char *del[2];
 	int delp = 0;
+	unsigned int ren1_mode = ren1->pair->two->mode;
+	unsigned int ren2_mode = ren2->pair->two->mode;
 	const char *ren1_dst = ren1->pair->two->path;
 	const char *ren2_dst = ren2->pair->two->path;
 	const char *dst_name1 = ren1_dst;
@@ -742,13 +744,13 @@ static void conflict_rename_rename(struct merge_options *o,
 		dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1);
 		output(o, 1, "%s is a directory in %s adding as %s instead",
 		       ren1_dst, branch2, dst_name1);
-		remove_file(o, 0, ren1_dst, 0);
+		remove_file(o, 0, ren1_mode, ren1_dst, 0);
 	}
 	if (string_list_has_string(&o->current_directory_set, ren2_dst)) {
 		dst_name2 = del[delp++] = unique_path(o, ren2_dst, branch2);
 		output(o, 1, "%s is a directory in %s adding as %s instead",
 		       ren2_dst, branch1, dst_name2);
-		remove_file(o, 0, ren2_dst, 0);
+		remove_file(o, 0, ren2_mode, ren2_dst, 0);
 	}
 	if (o->call_depth) {
 		remove_file_from_cache(dst_name1);
@@ -773,7 +775,7 @@ static void conflict_rename_dir(struct merge_options *o,
 {
 	char *new_path = unique_path(o, ren1->pair->two->path, branch1);
 	output(o, 1, "Renaming %s to %s instead", ren1->pair->one->path, new_path);
-	remove_file(o, 0, ren1->pair->two->path, 0);
+	remove_file(o, 0, ren1->pair->two->mode, ren1->pair->two->path, 0);
 	update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);
 	free(new_path);
 }
@@ -789,7 +791,7 @@ static void conflict_rename_rename_2(struct merge_options *o,
 	output(o, 1, "Renaming %s to %s and %s to %s instead",
 	       ren1->pair->one->path, new_path1,
 	       ren2->pair->one->path, new_path2);
-	remove_file(o, 0, ren1->pair->two->path, 0);
+	remove_file(o, 0, ren1->pair->two->mode, ren1->pair->two->path, 0);
 	update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path1);
 	update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, new_path2);
 	free(new_path2);
@@ -821,6 +823,7 @@ static int process_renames(struct merge_options *o,
 		struct rename *ren1 = NULL, *ren2 = NULL;
 		const char *branch1, *branch2;
 		const char *ren1_src, *ren1_dst;
+		unsigned int ren1_srcmode;
 
 		if (i >= a_renames->nr) {
 			ren2 = b_renames->items[j++].util;
@@ -863,6 +866,8 @@ static int process_renames(struct merge_options *o,
 		ren1_src = ren1->pair->one->path;
 		ren1_dst = ren1->pair->two->path;
 
+		ren1_srcmode = ren1->pair->one->mode;
+
 		if (ren2) {
 			const char *ren2_src = ren2->pair->one->path;
 			const char *ren2_dst = ren2->pair->two->path;
@@ -887,7 +892,7 @@ static int process_renames(struct merge_options *o,
 				conflict_rename_rename(o, ren1, branch1, ren2, branch2);
 			} else {
 				struct merge_file_info mfi;
-				remove_file(o, 1, ren1_src, 1);
+				remove_file(o, 1, ren1_srcmode, ren1_src, 1);
 				mfi = merge_file(o,
 						 ren1->pair->one,
 						 ren1->pair->two,
@@ -921,7 +926,7 @@ static int process_renames(struct merge_options *o,
 			struct diff_filespec src_other, dst_other;
 			int try_merge, stage = a_renames == renames1 ? 3: 2;
 
-			remove_file(o, 1, ren1_src, o->call_depth || stage == 3);
+			remove_file(o, 1, ren1_srcmode, ren1_src, o->call_depth || stage == 3);
 
 			hashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);
 			src_other.mode = ren1->src_entry->stages[stage].mode;
@@ -1077,7 +1082,7 @@ static int process_entry(struct merge_options *o,
 			if (a_sha)
 				output(o, 2, "Removing %s", path);
 			/* do not touch working file if it did not exist */
-			remove_file(o, 1, path, !a_sha);
+			remove_file(o, 1, a_mode, path, !a_sha);
 		} else {
 			/* Deleted in one and changed in the other */
 			clean_merge = 0;
@@ -1124,7 +1129,7 @@ static int process_entry(struct merge_options *o,
 			output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
 			       "Adding %s as %s",
 			       conf, path, other_branch, path, new_path);
-			remove_file(o, 0, path, 0);
+			remove_file(o, 0, mode, path, 0);
 			update_file(o, 0, sha, mode, new_path);
 		} else {
 			output(o, 2, "Adding %s", path);
@@ -1166,7 +1171,7 @@ static int process_entry(struct merge_options *o,
 		 * this entry was deleted altogether. a_mode == 0 means
 		 * we had that path and want to actively remove it.
 		 */
-		remove_file(o, 1, path, !a_mode);
+		remove_file(o, 1, a_mode, path, !a_mode);
 	} else
 		die("Fatal merge failure, shouldn't happen.");
 
diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
index 9a21f78..d87ed9e 100755
--- a/t/t7405-submodule-merge.sh
+++ b/t/t7405-submodule-merge.sh
@@ -71,4 +71,17 @@ test_expect_success 'merging with a modify/modify conflict between merge bases'
 
 '
 
+test_expect_success 'merging a submodule deletion' '
+
+	git reset --hard HEAD &&
+	git checkout -b test3 a &&
+	rm -rf sub &&
+	git update-index --remove sub &&
+	git commit -m empty &&
+	git checkout -b test4 c &&
+	test -d sub &&
+	git merge test3 &&
+	test \! -d sub
+'
+
 test_done
-- 
1.6.5

  parent reply	other threads:[~2010-03-26 15:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-26 15:25 [PATCH 00/12] Improve handling of moving and removing submodules Peter Collingbourne
2010-03-26 15:25 ` [PATCH 01/12] Generate unique ID for submodules created using "git submodule add" Peter Collingbourne
2010-03-27  9:44   ` Jonathan Nieder
2010-04-03 20:04     ` Peter Collingbourne
2010-04-03 20:04     ` [PATCH 1/2] Prefix submodule names with the path basename Peter Collingbourne
2010-04-03 20:04     ` [PATCH 2/2] Truncate the SHA1 part of the submodule name to 7 characters Peter Collingbourne
2010-03-26 15:25 ` [PATCH 02/12] Implement "git mv" for submodules Peter Collingbourne
2010-03-26 15:25 ` [PATCH 03/12] git rm: display a warning for every unremovable file Peter Collingbourne
2010-03-27 11:01   ` Jonathan Nieder
2010-03-26 15:25 ` [PATCH 04/12] Generalise the unlink_or_warn function Peter Collingbourne
2010-03-26 15:25 ` [PATCH 05/12] Implement the rmdir_or_warn function Peter Collingbourne
2010-03-26 15:25 ` [PATCH 06/12] Introduce remove_or_warn function Peter Collingbourne
2010-03-26 15:25 ` [PATCH 07/12] Remove a redundant errno test in a usage of remove_path Peter Collingbourne
2010-03-26 15:25 ` [PATCH 08/12] git rm: collect file modes Peter Collingbourne
2010-03-26 15:25 ` Peter Collingbourne [this message]
2010-03-26 15:25 ` [PATCH 10/12] git rm: do not abort due to an initialised submodule Peter Collingbourne
2010-03-26 15:25 ` [PATCH 11/12] git submodule: infrastructure for reading .gitmodules files in arbitrary locations Peter Collingbourne
2010-03-26 15:25 ` [PATCH 12/12] git rm: remove submodule entries from .gitmodules Peter Collingbourne

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1269617140-7827-10-git-send-email-peter@pcc.me.uk \
    --to=peter@pcc.me.uk \
    --cc=git@vger.kernel.org \
    /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.
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 NNTP newsgroup(s).