merge-recursive: fix overwriting dirty files involved in renames

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 dirty files that are involved in directory rename detection will be added in a subsequent commit. Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Apr 19, 2018 at 10:58 UTC 64b1abe962b44e6bad84b980e8ea2811302e71c7
7 files changed +77 -24
merge-recursive.c
+66 -19
@@ -337,32 +337,37 @@ static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
337 init_tree_desc(desc, tree->buffer, tree->size);
338 }
339
340 -static int git_merge_trees(int index_only,
340 +static int git_merge_trees(struct merge_options *o,
341 struct tree *common,
342 struct tree *head,
343 struct tree *merge)
344 {
345 int rc;
346 struct tree_desc t[3];
347 - struct unpack_trees_options opts;
347
349 - memset(&opts, 0, sizeof(opts));
350 - if (index_only)
351 - opts.index_only = 1;
348 + memset(&o->unpack_opts, 0, sizeof(o->unpack_opts));
349 + if (o->call_depth)
350 + o->unpack_opts.index_only = 1;
351 else
353 - opts.update = 1;
354 - opts.merge = 1;
355 - opts.head_idx = 2;
356 - opts.fn = threeway_merge;
357 - opts.src_index = &the_index;
358 - opts.dst_index = &the_index;
359 - setup_unpack_trees_porcelain(&opts, "merge");
352 + o->unpack_opts.update = 1;
353 + o->unpack_opts.merge = 1;
354 + o->unpack_opts.head_idx = 2;
355 + o->unpack_opts.fn = threeway_merge;
356 + o->unpack_opts.src_index = &the_index;
357 + o->unpack_opts.dst_index = &the_index;
358 + setup_unpack_trees_porcelain(&o->unpack_opts, "merge");
359
360 init_tree_desc_from_tree(t+0, common);
361 init_tree_desc_from_tree(t+1, head);
362 init_tree_desc_from_tree(t+2, merge);
363
365 - rc = unpack_trees(3, t, &opts);
364 + rc = unpack_trees(3, t, &o->unpack_opts);
365 + /*
366 + * unpack_trees NULLifies src_index, but it's used in verify_uptodate,
367 + * so set to the new index which will usually have modification
368 + * timestamp info copied over.
369 + */
370 + o->unpack_opts.src_index = &the_index;
371 cache_tree_free(&active_cache_tree);
372 return rc;
373 }
@@ -795,6 +800,20 @@ static int would_lose_untracked(const char *path)
800 return !was_tracked(path) && file_exists(path);
801 }
802
803 +static int was_dirty(struct merge_options *o, const char *path)
804 +{
805 + struct cache_entry *ce;
806 + int dirty = 1;
807 +
808 + if (o->call_depth || !was_tracked(path))
809 + return !dirty;
810 +
811 + ce = cache_file_exists(path, strlen(path), ignore_case);
812 + dirty = (ce->ce_stat_data.sd_mtime.sec > 0 &&
813 + verify_uptodate(ce, &o->unpack_opts) != 0);
814 + return dirty;
815 +}
816 +
817 static int make_room_for_path(struct merge_options *o, const char *path)
818 {
819 int status, i;
@@ -2687,6 +2706,7 @@ static int handle_modify_delete(struct merge_options *o,
2706
2707 static int merge_content(struct merge_options *o,
2708 const char *path,
2709 + int file_in_way,
2710 struct object_id *o_oid, int o_mode,
2711 struct object_id *a_oid, int a_mode,
2712 struct object_id *b_oid, int b_mode,
@@ -2761,7 +2781,7 @@ static int merge_content(struct merge_options *o,
2781 return -1;
2782 }
2783
2764 - if (df_conflict_remains) {
2784 + if (df_conflict_remains || file_in_way) {
2785 char *new_path;
2786 if (o->call_depth) {
2787 remove_file_from_cache(path);
@@ -2795,6 +2815,30 @@ static int merge_content(struct merge_options *o,
2815 return mfi.clean;
2816 }
2817
2818 +static int conflict_rename_normal(struct merge_options *o,
2819 + const char *path,
2820 + struct object_id *o_oid, unsigned int o_mode,
2821 + struct object_id *a_oid, unsigned int a_mode,
2822 + struct object_id *b_oid, unsigned int b_mode,
2823 + struct rename_conflict_info *ci)
2824 +{
2825 + int clean_merge;
2826 + int file_in_the_way = 0;
2827 +
2828 + if (was_dirty(o, path)) {
2829 + file_in_the_way = 1;
2830 + output(o, 1, _("Refusing to lose dirty file at %s"), path);
2831 + }
2832 +
2833 + /* Merge the content and write it out */
2834 + clean_merge = merge_content(o, path, file_in_the_way,
2835 + o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
2836 + ci);
2837 + if (clean_merge > 0 && file_in_the_way)
2838 + clean_merge = 0;
2839 + return clean_merge;
2840 +}
2841 +
2842 /* Per entry merge function */
2843 static int process_entry(struct merge_options *o,
2844 const char *path, struct stage_data *entry)
@@ -2814,9 +2858,12 @@ static int process_entry(struct merge_options *o,
2858 switch (conflict_info->rename_type) {
2859 case RENAME_NORMAL:
2860 case RENAME_ONE_FILE_TO_ONE:
2817 - clean_merge = merge_content(o, path,
2818 - o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
2819 - conflict_info);
2861 + clean_merge = conflict_rename_normal(o,
2862 + path,
2863 + o_oid, o_mode,
2864 + a_oid, a_mode,
2865 + b_oid, b_mode,
2866 + conflict_info);
2867 break;
2868 case RENAME_DIR:
2869 clean_merge = 1;
@@ -2912,7 +2959,7 @@ static int process_entry(struct merge_options *o,
2959 } else if (a_oid && b_oid) {
2960 /* Case C: Added in both (check for same permissions) and */
2961 /* case D: Modified in both, but differently. */
2915 - clean_merge = merge_content(o, path,
2962 + clean_merge = merge_content(o, path, 0 /* file_in_way */,
2963 o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
2964 NULL);
2965 } else if (!o_oid && !a_oid && !b_oid) {
@@ -2953,7 +3000,7 @@ int merge_trees(struct merge_options *o,
3000 return 1;
3001 }
3002
2956 - code = git_merge_trees(o->call_depth, common, head, merge);
3003 + code = git_merge_trees(o, common, head, merge);
3004
3005 if (code != 0) {
3006 if (show(o, 4) || o->call_depth)
merge-recursive.h
+2
@@ -1,6 +1,7 @@
1 #ifndef MERGE_RECURSIVE_H
2 #define MERGE_RECURSIVE_H
3
4 +#include "unpack-trees.h"
5 #include "string-list.h"
6
7 struct merge_options {
@@ -27,6 +28,7 @@ struct merge_options {
28 struct strbuf obuf;
29 struct hashmap current_file_dir_set;
30 struct string_list df_conflict_file_set;
31 + struct unpack_trees_options unpack_opts;
32 };
33
34 /*
t/t3501-revert-cherry-pick.sh
+1 -1
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick "-" works with arguments' '
141 test_cmp expect actual
142 '
143
144 -test_expect_failure 'cherry-pick works with dirty renamed file' '
144 +test_expect_success 'cherry-pick works with dirty renamed file' '
145 test_commit to-rename &&
146 git checkout -b unrelated &&
147 test_commit unrelated &&
t/t6043-merge-rename-directories.sh
+1 -1
@@ -3298,7 +3298,7 @@ test_expect_success '11a-setup: Avoid losing dirty contents with simple rename'
3298 )
3299 '
3300
3301 -test_expect_failure '11a-check: Avoid losing dirty contents with simple rename' '
3301 +test_expect_success '11a-check: Avoid losing dirty contents with simple rename' '
3302 (
3303 cd 11a &&
3304
t/t7607-merge-overwrite.sh
+1 -1
@@ -92,7 +92,7 @@ test_expect_success 'will not overwrite removed file with staged changes' '
92 test_cmp important c1.c
93 '
94
95 -test_expect_failure 'will not overwrite unstaged changes in renamed file' '
95 +test_expect_success 'will not overwrite unstaged changes in renamed file' '
96 git reset --hard c1 &&
97 git mv c1.c other.c &&
98 git commit -m rename &&
unpack-trees.c
+2 -2
@@ -1509,8 +1509,8 @@ static int verify_uptodate_1(const struct cache_entry *ce,
1509 add_rejected_path(o, error_type, ce->name);
1510 }
1511
1512 -static int verify_uptodate(const struct cache_entry *ce,
1513 - struct unpack_trees_options *o)
1512 +int verify_uptodate(const struct cache_entry *ce,
1513 + struct unpack_trees_options *o)
1514 {
1515 if (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))
1516 return 0;
unpack-trees.h
+4
@@ -1,6 +1,7 @@
1 #ifndef UNPACK_TREES_H
2 #define UNPACK_TREES_H
3
4 +#include "tree-walk.h"
5 #include "string-list.h"
6
7 #define MAX_UNPACK_TREES 8
@@ -78,6 +79,9 @@ struct unpack_trees_options {
79 extern int unpack_trees(unsigned n, struct tree_desc *t,
80 struct unpack_trees_options *options);
81
82 +int verify_uptodate(const struct cache_entry *ce,
83 + struct unpack_trees_options *o);
84 +
85 int threeway_merge(const struct cache_entry * const *stages,
86 struct unpack_trees_options *o);
87 int twoway_merge(const struct cache_entry * const *src,