git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v7 27/31] merge-recursive: fix overwriting dirty files involved in renames

From
Elijah Newren <newren@gmail.com>
Date
Jan 30, 2018, 23:25 UTC
Message-ID
<20180130232533.25846-28-newren@gmail.com>
In-Reply-To
<20180130232533.25846-1-newren@gmail.com>

This fixes an issue that existed before my directory rename detection patches that affects both normal renames and renames implied by directory rename detection. Additional codepaths that only affect overwriting of directy files that are involved in directory rename detection will be added in a subsequent commit.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-recursive.c                   | 85 ++++++++++++++++++++++++++++---------
 merge-recursive.h                   |  2 +
 t/t3501-revert-cherry-pick.sh       |  2 +-
 t/t6043-merge-rename-directories.sh |  2 +-
 t/t7607-merge-overwrite.sh          |  2 +-
 unpack-trees.c                      |  4 +-
 unpack-trees.h                      |  4 ++
 7 files changed, 77 insertions(+), 24 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 39e161e094..fba1a0d207 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -334,32 +334,37 @@ static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
 	init_tree_desc(desc, tree->buffer, tree->size);
 }
 
-static int git_merge_trees(int index_only,
+static int git_merge_trees(struct merge_options *o,
 			   struct tree *common,
 			   struct tree *head,
 			   struct tree *merge)
 {
 	int rc;
 	struct tree_desc t[3];
-	struct unpack_trees_options opts;
 
-	memset(&opts, 0, sizeof(opts));
-	if (index_only)
-		opts.index_only = 1;
+	memset(&o->unpack_opts, 0, sizeof(o->unpack_opts));
+	if (o->call_depth)
+		o->unpack_opts.index_only = 1;
 	else
-		opts.update = 1;
-	opts.merge = 1;
-	opts.head_idx = 2;
-	opts.fn = threeway_merge;
-	opts.src_index = &the_index;
-	opts.dst_index = &the_index;
-	setup_unpack_trees_porcelain(&opts, "merge");
+		o->unpack_opts.update = 1;
+	o->unpack_opts.merge = 1;
+	o->unpack_opts.head_idx = 2;
+	o->unpack_opts.fn = threeway_merge;
+	o->unpack_opts.src_index = &the_index;
+	o->unpack_opts.dst_index = &the_index;
+	setup_unpack_trees_porcelain(&o->unpack_opts, "merge");
 
 	init_tree_desc_from_tree(t+0, common);
 	init_tree_desc_from_tree(t+1, head);
 	init_tree_desc_from_tree(t+2, merge);
 
-	rc = unpack_trees(3, t, &opts);
+	rc = unpack_trees(3, t, &o->unpack_opts);
+	/*
+	 * unpack_trees NULLifies src_index, but it's used in verify_uptodate,
+	 * so set to the new index which will usually have modification
+	 * timestamp info copied over.
+	 */
+	o->unpack_opts.src_index = &the_index;
 	cache_tree_free(&active_cache_tree);
 	return rc;
 }
@@ -792,6 +797,20 @@ static int would_lose_untracked(const char *path)
 	return !was_tracked(path) && file_exists(path);
 }
 
+static int was_dirty(struct merge_options *o, const char *path)
+{
+	struct cache_entry *ce;
+	int dirty = 1;
+
+	if (o->call_depth || !was_tracked(path))
+		return !dirty;
+
+	ce = cache_file_exists(path, strlen(path), ignore_case);
+	dirty = (ce->ce_stat_data.sd_mtime.sec > 0 &&
+		 verify_uptodate(ce, &o->unpack_opts) != 0);
+	return dirty;
+}
+
 static int make_room_for_path(struct merge_options *o, const char *path)
 {
 	int status, i;
@@ -2654,6 +2673,7 @@ static int handle_modify_delete(struct merge_options *o,
 
 static int merge_content(struct merge_options *o,
 			 const char *path,
+			 int file_in_way,
 			 struct object_id *o_oid, int o_mode,
 			 struct object_id *a_oid, int a_mode,
 			 struct object_id *b_oid, int b_mode,
@@ -2728,7 +2748,7 @@ static int merge_content(struct merge_options *o,
 				return -1;
 	}
 
-	if (df_conflict_remains) {
+	if (df_conflict_remains || file_in_way) {
 		char *new_path;
 		if (o->call_depth) {
 			remove_file_from_cache(path);
@@ -2762,6 +2782,30 @@ static int merge_content(struct merge_options *o,
 	return mfi.clean;
 }
 
+static int conflict_rename_normal(struct merge_options *o,
+				  const char *path,
+				  struct object_id *o_oid, unsigned int o_mode,
+				  struct object_id *a_oid, unsigned int a_mode,
+				  struct object_id *b_oid, unsigned int b_mode,
+				  struct rename_conflict_info *ci)
+{
+	int clean_merge;
+	int file_in_the_way = 0;
+
+	if (was_dirty(o, path)) {
+		file_in_the_way = 1;
+		output(o, 1, _("Refusing to lose dirty file at %s"), path);
+	}
+
+	/* Merge the content and write it out */
+	clean_merge = merge_content(o, path, file_in_the_way,
+				    o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
+				    ci);
+	if (clean_merge > 0 && file_in_the_way)
+		clean_merge = 0;
+	return clean_merge;
+}
+
 /* Per entry merge function */
 static int process_entry(struct merge_options *o,
 			 const char *path, struct stage_data *entry)
@@ -2781,9 +2825,12 @@ static int process_entry(struct merge_options *o,
 		switch (conflict_info->rename_type) {
 		case RENAME_NORMAL:
 		case RENAME_ONE_FILE_TO_ONE:
-			clean_merge = merge_content(o, path,
-						    o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
-						    conflict_info);
+			clean_merge = conflict_rename_normal(o,
+							     path,
+							     o_oid, o_mode,
+							     a_oid, a_mode,
+							     b_oid, b_mode,
+							     conflict_info);
 			break;
 		case RENAME_DIR:
 			clean_merge = 1;
@@ -2879,7 +2926,7 @@ static int process_entry(struct merge_options *o,
 	} else if (a_oid && b_oid) {
 		/* Case C: Added in both (check for same permissions) and */
 		/* case D: Modified in both, but differently. */
-		clean_merge = merge_content(o, path,
+		clean_merge = merge_content(o, path, 0 /* file_in_way */,
 					    o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
 					    NULL);
 	} else if (!o_oid && !a_oid && !b_oid) {
@@ -2920,7 +2967,7 @@ int merge_trees(struct merge_options *o,
 		return 1;
 	}
 
-	code = git_merge_trees(o->call_depth, common, head, merge);
+	code = git_merge_trees(o, common, head, merge);
 
 	if (code != 0) {
 		if (show(o, 4) || o->call_depth)
diff --git a/merge-recursive.h b/merge-recursive.h
index e1be27f57c..a557201a50 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -1,6 +1,7 @@
 #ifndef MERGE_RECURSIVE_H
 #define MERGE_RECURSIVE_H
 
+#include "unpack-trees.h"
 #include "string-list.h"
 
 struct merge_options {
@@ -27,6 +28,7 @@ struct merge_options {
 	struct strbuf obuf;
 	struct hashmap current_file_dir_set;
 	struct string_list df_conflict_file_set;
+	struct unpack_trees_options unpack_opts;
 };
 
 struct dir_rename_entry {
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 783bdbf59d..0d89f6d0f6 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick "-" works with arguments' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'cherry-pick works with dirty renamed file' '
+test_expect_success 'cherry-pick works with dirty renamed file' '
 	test_commit to-rename &&
 	git checkout -b unrelated &&
 	test_commit unrelated &&
diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
index abb5b20f6b..89b2eacf38 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -3290,7 +3290,7 @@ test_expect_success '11a-setup: Avoid losing dirty contents with simple rename'
 	)
 '
 
-test_expect_failure '11a-check: Avoid losing dirty contents with simple rename' '
+test_expect_success '11a-check: Avoid losing dirty contents with simple rename' '
 	(
 		cd 11a &&
 
diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh
index 9c422bcd7c..dd8ab7ede1 100755
--- a/t/t7607-merge-overwrite.sh
+++ b/t/t7607-merge-overwrite.sh
@@ -92,7 +92,7 @@ test_expect_success 'will not overwrite removed file with staged changes' '
 	test_cmp important c1.c
 '
 
-test_expect_failure 'will not overwrite unstaged changes in renamed file' '
+test_expect_success 'will not overwrite unstaged changes in renamed file' '
 	git reset --hard c1 &&
 	git mv c1.c other.c &&
 	git commit -m rename &&
diff --git a/unpack-trees.c b/unpack-trees.c
index 96c3327f19..f99fe9b9fd 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1486,8 +1486,8 @@ static int verify_uptodate_1(const struct cache_entry *ce,
 		add_rejected_path(o, error_type, ce->name);
 }
 
-static int verify_uptodate(const struct cache_entry *ce,
-			   struct unpack_trees_options *o)
+int verify_uptodate(const struct cache_entry *ce,
+		    struct unpack_trees_options *o)
 {
 	if (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))
 		return 0;
diff --git a/unpack-trees.h b/unpack-trees.h
index 6c48117b84..41178ada94 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -1,6 +1,7 @@
 #ifndef UNPACK_TREES_H
 #define UNPACK_TREES_H
 
+#include "tree-walk.h"
 #include "string-list.h"
 
 #define MAX_UNPACK_TREES 8
@@ -78,6 +79,9 @@ struct unpack_trees_options {
 extern int unpack_trees(unsigned n, struct tree_desc *t,
 		struct unpack_trees_options *options);
 
+int verify_uptodate(const struct cache_entry *ce,
+		    struct unpack_trees_options *o);
+
 int threeway_merge(const struct cache_entry * const *stages,
 		   struct unpack_trees_options *o);
 int twoway_merge(const struct cache_entry * const *src,

-- 
2.16.1.106.gf69932adfe
Previous: Elijah NewrenNext: Stefan Beller
Message 39 of 63 in “Add directory rename detection to git”
  1. 00/31 Add directory rename detection to gitElijah Newren, Jan 30, 2018
  2. Elijah NewrenJan 30, 2018
  3. 24/31 merge-recursive: when comparing files, don't include treesElijah Newren, Jan 30, 2018
  4. 28/31 merge-recursive: fix remaining directory rename + dirty overwrite casesElijah Newren, Jan 30, 2018
  5. Stefan BellerFeb 5, 2018
  6. Elijah NewrenFeb 5, 2018
  7. 05/31 directory rename detection: files/directories in the way of some renamesElijah Newren, Jan 30, 2018
  8. 06/31 directory rename detection: testcases checking which side did the renameElijah Newren, Jan 30, 2018
  9. 10/31 directory rename detection: tests for handling overwriting untracked filesElijah Newren, Jan 30, 2018
  10. 15/31 merge-recursive: make !o->detect_rename codepath more obviousElijah Newren, Jan 30, 2018
  11. Stefan BellerFeb 2, 2018
  12. 14/31 merge-recursive: fix leaks of allocated renames and diff_filepairsElijah Newren, Jan 30, 2018
  13. Stefan BellerFeb 2, 2018
  14. 23/31 merge-recursive: check for file level conflicts then get new nameElijah Newren, Jan 30, 2018
  15. 30/31 merge-recursive: avoid spurious rename/rename conflict from dir renamesElijah Newren, Jan 30, 2018
  16. 16/31 merge-recursive: split out code for determining diff_filepairsElijah Newren, Jan 30, 2018
  17. Stefan BellerFeb 3, 2018
  18. Elijah NewrenFeb 3, 2018
  19. 11/31 directory rename detection: tests for handling overwriting dirty filesElijah Newren, Jan 30, 2018
  20. 22/31 merge-recursive: add computation of collisions due to dir rename & mergingElijah Newren, Jan 30, 2018
  21. 31/31 merge-recursive: ensure we write updates for directory-renamed fileElijah Newren, Jan 30, 2018
  22. Stefan BellerFeb 5, 2018
  23. 20/31 merge-recursive: check for directory level conflictsElijah Newren, Jan 30, 2018
  24. Stefan BellerFeb 5, 2018
  25. Elijah NewrenFeb 5, 2018
  26. 25/31 merge-recursive: apply necessary modifications for directory renamesElijah Newren, Jan 30, 2018
  27. SZEDER GáborFeb 16, 2018
  28. 17/31 merge-recursive: add a new hashmap for storing directory renamesElijah Newren, Jan 30, 2018
  29. Stefan BellerFeb 3, 2018
  30. Elijah NewrenFeb 3, 2018
  31. Johannes SixtFeb 4, 2018
  32. Elijah NewrenFeb 5, 2018
  33. Junio C HamanoFeb 5, 2018
  34. Stefan BellerFeb 5, 2018
  35. Elijah NewrenFeb 5, 2018
  36. 21/31 merge-recursive: add a new hashmap for storing file collisionsElijah Newren, Jan 30, 2018
  37. Stefan BellerFeb 5, 2018
  38. 29/31 directory rename detection: new testcases showcasing a pair of bugsElijah Newren, Jan 30, 2018
  39. 27/31 merge-recursive: fix overwriting dirty files involved in renamesElijah Newren, Jan 30, 2018
  40. Stefan BellerFeb 5, 2018
  41. Elijah NewrenFeb 5, 2018
  42. 03/31 directory rename detection: testcases to avoid taking detection too farElijah Newren, Jan 30, 2018
  43. 01/31 directory rename detection: basic testcasesElijah Newren, Jan 30, 2018
  44. 04/31 directory rename detection: partially renamed directory testcase/discussionElijah Newren, Jan 30, 2018
  45. 19/31 merge-recursive: add get_directory_renames()Elijah Newren, Jan 30, 2018
  46. Stefan BellerFeb 3, 2018
  47. Elijah NewrenFeb 3, 2018
  48. Elijah NewrenFeb 4, 2018
  49. Eric SunshineFeb 4, 2018
  50. Eric SunshineFeb 4, 2018
  51. Stefan BellerFeb 5, 2018
  52. 18/31 merge-recursive: make a helper function for cleanup for handle_renamesElijah Newren, Jan 30, 2018
  53. Stefan BellerFeb 3, 2018
  54. 02/31 directory rename detection: directory splitting testcasesElijah Newren, Jan 30, 2018
  55. 13/31 merge-recursive: introduce new functions to handle rename logicElijah Newren, Jan 30, 2018
  56. Stefan BellerFeb 2, 2018
  57. 12/31 merge-recursive: move the get_renames() functionElijah Newren, Jan 30, 2018
  58. Stefan BellerFeb 2, 2018
  59. 09/31 directory rename detection: miscellaneous testcases to complete coverageElijah Newren, Jan 30, 2018
  60. 08/31 directory rename detection: testcases exploring possibly suboptimal mergesElijah Newren, Jan 30, 2018
  61. 26/31 merge-recursive: avoid clobbering untracked files with directory renamesElijah Newren, Jan 30, 2018
  62. 07/31 directory rename detection: more involved edge/corner testcasesElijah Newren, Jan 30, 2018
  63. Stefan BellerFeb 5, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.