Commit e47f859db9a52f38d9ca94d725a178ac4ba93836

Edward Thomson 2013-08-08T16:46:49

Don't overwrite ~ files checking out conflicts If a D/F conflict or rename 2->1 conflict occurs, we write the file sides as filename~branchname. If a file with that name already exists in the working directory, write as filename~branchname_0 instead. (Incrementing 0 until a unique filename is found.)

diff --git a/src/checkout_conflicts.c b/src/checkout_conflicts.c
index 54e0403..dcf2714 100644
--- a/src/checkout_conflicts.c
+++ b/src/checkout_conflicts.c
@@ -328,7 +328,7 @@ static int checkout_conflicts_mark_directoryfile(
 
 		if ((error = git_index_find(&j, data->index, path)) < 0) {
 			if (error == GIT_ENOTFOUND)
-				giterr_set(GITERR_MERGE,
+				giterr_set(GITERR_INDEX,
 					"Index inconsistency, could not find entry for expected conflict '%s'", path);
 
 			goto done;
@@ -336,7 +336,7 @@ static int checkout_conflicts_mark_directoryfile(
 
 		for (; j < len; j++) {
 			if ((entry = git_index_get_byindex(data->index, j)) == NULL) {
-				giterr_set(GITERR_MERGE,
+				giterr_set(GITERR_INDEX,
 					"Index inconsistency, truncated index while loading expected conflict '%s'", path);
 				error = -1;
 				goto done;
@@ -371,16 +371,33 @@ static int conflict_entry_name(
 	return 0;
 }
 
-static int conflict_path_suffixed(
-	git_buf *out,
-	const char *path,
-	const char *side_name)
+static int checkout_path_suffixed(git_buf *path, const char *suffix)
 {
-	if (git_buf_puts(out, path) < 0 ||
-		git_buf_putc(out, '~') < 0 ||
-		git_buf_puts(out, side_name) < 0)
+	size_t path_len;
+	int i = 0, error = 0;
+
+	if ((error = git_buf_putc(path, '~')) < 0 || (error = git_buf_puts(path, suffix)) < 0)
 		return -1;
 
+	path_len = git_buf_len(path);
+
+	while (git_path_exists(git_buf_cstr(path)) && i < INT_MAX) {
+		git_buf_truncate(path, path_len);
+
+		if ((error = git_buf_putc(path, '_')) < 0 ||
+			(error = git_buf_printf(path, "%d", i)) < 0)
+			return error;
+
+		i++;
+	}
+
+	if (i == INT_MAX) {
+		git_buf_truncate(path, path_len);
+
+		giterr_set(GITERR_CHECKOUT, "Could not write '%s': working directory file exists", path);
+		return GIT_EEXISTS;
+	}
+
 	return 0;
 }
 
@@ -389,7 +406,7 @@ static int checkout_write_entry(
 	checkout_conflictdata *conflict,
 	const git_index_entry *side)
 {
-	const char *hint_path = NULL, *side_label;
+	const char *hint_path = NULL, *suffix;
 	struct stat st;
 
 	assert (side == conflict->ours ||
@@ -404,14 +421,13 @@ static int checkout_write_entry(
 		(data->strategy & GIT_CHECKOUT_USE_THEIRS) == 0) {
 
 		if (side == conflict->ours)
-			side_label = data->opts.our_label ? data->opts.our_label :
+			suffix = data->opts.our_label ? data->opts.our_label :
 				"ours";
 		else if (side == conflict->theirs)
-			side_label = data->opts.their_label ? data->opts.their_label :
+			suffix = data->opts.their_label ? data->opts.their_label :
 				"theirs";
 
-		if (git_buf_putc(&data->path, '~') < 0 ||
-			git_buf_puts(&data->path, side_label) < 0)
+		if (checkout_path_suffixed(&data->path, suffix) < 0)
 			return -1;
 
 		hint_path = side->path;
@@ -433,6 +449,33 @@ static int checkout_write_entries(
 	return error;
 }
 
+static int checkout_merge_path(
+	git_buf *out,
+	checkout_data *data,
+	checkout_conflictdata *conflict,
+	git_merge_file_result *result)
+{
+	const char *our_label_raw, *their_label_raw, *suffix;
+	int i = 0, error = 0;
+
+	if ((error = git_buf_joinpath(out, git_repository_workdir(data->repo), result->path)) < 0)
+		return error;
+
+	/* Most conflicts simply use the filename in the index */
+	if (!conflict->name_collision)
+		return 0;
+
+	/* Rename 2->1 conflicts need the branch name appended */
+	our_label_raw = data->opts.our_label ? data->opts.our_label : "ours";
+	their_label_raw = data->opts.their_label ? data->opts.their_label : "theirs";
+	suffix = strcmp(result->path, conflict->ours->path) == 0 ? our_label_raw : their_label_raw;
+
+	if ((error = checkout_path_suffixed(out, suffix)) < 0)
+		return error;
+
+	return 0;
+}
+
 static int checkout_write_merge(
 	checkout_data *data,
 	checkout_conflictdata *conflict)
@@ -444,7 +487,6 @@ static int checkout_write_merge(
 		theirs = GIT_MERGE_FILE_INPUT_INIT;
 	git_merge_file_result result = GIT_MERGE_FILE_RESULT_INIT;
 	git_filebuf output = GIT_FILEBUF_INIT;
-	const char *our_label_raw, *their_label_raw, *path;
 	int error = 0;
 
 	if ((conflict->ancestor &&
@@ -457,8 +499,8 @@ static int checkout_write_merge(
 		goto done;
 
 	ancestor.label = NULL;
-	ours.label = our_label_raw = data->opts.our_label ? data->opts.our_label : "ours";
-	theirs.label = their_label_raw = data->opts.their_label ? data->opts.their_label : "theirs";
+	ours.label = data->opts.our_label ? data->opts.our_label : "ours";
+	theirs.label = data->opts.their_label ? data->opts.their_label : "theirs";
 
 	/* If all the paths are identical, decorate the diff3 file with the branch
 	 * names.  Otherwise, append branch_name:path.
@@ -485,19 +527,8 @@ static int checkout_write_merge(
 		goto done;
 	}
 
-	/* Rename 2->1 conflicts need the branch name appended */
-	if (conflict->name_collision) {
-		if ((error = conflict_path_suffixed(&path_suffixed, result.path,
-			(strcmp(result.path, conflict->ours->path) == 0 ?
-			our_label_raw : their_label_raw))) < 0)
-			goto done;
-		
-		path = git_buf_cstr(&path_suffixed);
-	} else
-		path = result.path;
-
-	if ((error = git_buf_joinpath(&path_workdir, git_repository_workdir(data->repo), path)) < 0 ||
-		(error = git_futils_mkpath2file(path_workdir.ptr, 0755) < 0) ||
+	if ((error = checkout_merge_path(&path_workdir, data, conflict, &result)) < 0 ||
+		(error = git_futils_mkpath2file(path_workdir.ptr, 0755)) < 0 ||
 		(error = git_filebuf_open(&output, path_workdir.ptr, GIT_FILEBUF_DO_NOT_BUFFER)) < 0 ||
 		(error = git_filebuf_write(&output, result.data, result.len)) < 0 ||
 		(error = git_filebuf_commit(&output, result.mode)) < 0)
diff --git a/tests-clar/checkout/conflict.c b/tests-clar/checkout/conflict.c
index da4a20f..8b6dd4c 100644
--- a/tests-clar/checkout/conflict.c
+++ b/tests-clar/checkout/conflict.c
@@ -834,3 +834,143 @@ void test_checkout_conflict__rename_keep_ours(void)
 	ensure_workdir("7-both-renamed.txt",
 				   0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11");
 }
+
+void test_checkout_conflict__name_mangled_file_exists_in_workdir(void)
+{
+	git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT;
+
+	struct checkout_index_entry checkout_index_entries[] = {
+		{ 0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11", 1, "test-one-side-one.txt" },
+		{ 0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11", 3, "test-one-side-one.txt" },
+		{ 0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07", 1, "test-one-side-two.txt" },
+		{ 0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07", 2, "test-one-side-two.txt" },
+		{ 0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11", 2, "test-one.txt" },
+		{ 0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07", 3, "test-one.txt" },
+
+		{ 0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11", 1, "test-two-side-one.txt" },
+		{ 0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11", 3, "test-two-side-one.txt" },
+		{ 0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07", 1, "test-two-side-two.txt" },
+		{ 0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07", 2, "test-two-side-two.txt" },
+		{ 0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11", 2, "test-two.txt" },
+		{ 0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07", 3, "test-two.txt" },
+
+		{ 0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11", 1, "test-three-side-one.txt" },
+		{ 0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11", 3, "test-three-side-one.txt" },
+		{ 0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07", 1, "test-three-side-two.txt" },
+		{ 0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07", 2, "test-three-side-two.txt" },
+		{ 0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11", 2, "test-three.txt" },
+		{ 0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07", 3, "test-three.txt" },
+
+		{ 0100644, CONFLICTING_ANCESTOR_OID, 1, "directory_file-one" },
+		{ 0100644, CONFLICTING_OURS_OID, 2, "directory_file-one" },
+		{ 0100644, CONFLICTING_THEIRS_OID, 0, "directory_file-one/file" },
+
+		{ 0100644, CONFLICTING_ANCESTOR_OID, 1, "directory_file-two" },
+		{ 0100644, CONFLICTING_OURS_OID, 0, "directory_file-two/file" },
+		{ 0100644, CONFLICTING_THEIRS_OID, 3, "directory_file-two" },
+	};
+
+	struct checkout_name_entry checkout_name_entries[] = {
+		{
+			"test-one-side-one.txt",
+			"test-one.txt",
+			"test-one-side-one.txt"
+		},
+		{
+			"test-one-side-two.txt",
+			"test-one-side-two.txt",
+			"test-one.txt"
+		},
+
+		{
+			"test-two-side-one.txt",
+			"test-two.txt",
+			"test-two-side-one.txt"
+		},
+		{
+			"test-two-side-two.txt",
+			"test-two-side-two.txt",
+			"test-two.txt"
+		},
+
+		{
+			"test-three-side-one.txt",
+			"test-three.txt",
+			"test-three-side-one.txt"
+		},
+		{
+			"test-three-side-two.txt",
+			"test-three-side-two.txt",
+			"test-three.txt"
+		}
+	};
+
+	opts.checkout_strategy |= GIT_CHECKOUT_SAFE;
+
+	create_index(checkout_index_entries, 24);
+	create_index_names(checkout_name_entries, 6);
+	git_index_write(g_index);
+
+	/* Add some files on disk that conflict with the names that would be chosen
+	 * for the files written for each side. */
+
+	cl_git_rewritefile("merge-resolve/test-one.txt~ours",
+		"Expect index contents to be written to ~ours_0");
+	cl_git_rewritefile("merge-resolve/test-one.txt~theirs",
+		"Expect index contents to be written to ~theirs_0");
+
+	cl_git_rewritefile("merge-resolve/test-two.txt~ours",
+		"Expect index contents to be written to ~ours_3");
+	cl_git_rewritefile("merge-resolve/test-two.txt~theirs",
+		"Expect index contents to be written to ~theirs_3");
+	cl_git_rewritefile("merge-resolve/test-two.txt~ours_0",
+		"Expect index contents to be written to ~ours_3");
+	cl_git_rewritefile("merge-resolve/test-two.txt~theirs_0",
+		"Expect index contents to be written to ~theirs_3");
+	cl_git_rewritefile("merge-resolve/test-two.txt~ours_1",
+		"Expect index contents to be written to ~ours_3");
+	cl_git_rewritefile("merge-resolve/test-two.txt~theirs_1",
+		"Expect index contents to be written to ~theirs_3");
+	cl_git_rewritefile("merge-resolve/test-two.txt~ours_2",
+		"Expect index contents to be written to ~ours_3");
+	cl_git_rewritefile("merge-resolve/test-two.txt~theirs_2",
+		"Expect index contents to be written to ~theirs_3");
+
+	cl_git_rewritefile("merge-resolve/test-three.txt~Ours",
+		"Expect case insensitive filesystems to create ~ours_0");
+	cl_git_rewritefile("merge-resolve/test-three.txt~THEIRS",
+		"Expect case insensitive filesystems to create ~theirs_0");
+
+	cl_git_rewritefile("merge-resolve/directory_file-one~ours",
+		"Index contents written to ~ours_0 in this D/F conflict");
+	cl_git_rewritefile("merge-resolve/directory_file-two~theirs",
+		"Index contents written to ~theirs_0 in this D/F conflict");
+
+	cl_git_pass(git_checkout_index(g_repo, g_index, &opts));
+
+	ensure_workdir("test-one.txt~ours_0",
+		0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11");
+	ensure_workdir("test-one.txt~theirs_0",
+		0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07");
+
+	ensure_workdir("test-two.txt~ours_3",
+		0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11");
+	ensure_workdir("test-two.txt~theirs_3",
+		0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07");
+
+	/* Name is mangled on case insensitive only */
+#if defined(GIT_WIN32) || defined(__APPLE__)
+	ensure_workdir("test-three.txt~ours_0",
+		0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11");
+	ensure_workdir("test-three.txt~theirs_0",
+		0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07");
+#else
+	ensure_workdir("test-three.txt~ours",
+		0100644, "b42712cfe99a1a500b2a51fe984e0b8a7702ba11");
+	ensure_workdir("test-three.txt~theirs",
+		0100644, "b69fe837e4cecfd4c9a40cdca7c138468687df07");
+#endif
+
+	ensure_workdir("directory_file-one~ours_0", 0100644, CONFLICTING_OURS_OID);
+	ensure_workdir("directory_file-two~theirs_0", 0100644, CONFLICTING_THEIRS_OID);
+}