Skip to content

Commit 64b1abe

Browse files
newrengitster
newren
authored and
gitster
committed
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>
1 parent 79c4759 commit 64b1abe

File tree

7 files changed

+77
-24
lines changed

7 files changed

+77
-24
lines changed

‎merge-recursive.c‎

Lines changed: 66 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -337,32 +337,37 @@ static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
337337
init_tree_desc(desc, tree->buffer, tree->size);
338338
}
339339

340-
static int git_merge_trees(int index_only,
340+
static int git_merge_trees(struct merge_options *o,
341341
struct tree *common,
342342
struct tree *head,
343343
struct tree *merge)
344344
{
345345
int rc;
346346
struct tree_desc t[3];
347-
struct unpack_trees_options opts;
348347

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;
352351
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");
360359

361360
init_tree_desc_from_tree(t+0, common);
362361
init_tree_desc_from_tree(t+1, head);
363362
init_tree_desc_from_tree(t+2, merge);
364363

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;
366371
cache_tree_free(&active_cache_tree);
367372
return rc;
368373
}
@@ -795,6 +800,20 @@ static int would_lose_untracked(const char *path)
795800
return !was_tracked(path) && file_exists(path);
796801
}
797802

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+
798817
static int make_room_for_path(struct merge_options *o, const char *path)
799818
{
800819
int status, i;
@@ -2687,6 +2706,7 @@ static int handle_modify_delete(struct merge_options *o,
26872706

26882707
static int merge_content(struct merge_options *o,
26892708
const char *path,
2709+
int file_in_way,
26902710
struct object_id *o_oid, int o_mode,
26912711
struct object_id *a_oid, int a_mode,
26922712
struct object_id *b_oid, int b_mode,
@@ -2761,7 +2781,7 @@ static int merge_content(struct merge_options *o,
27612781
return -1;
27622782
}
27632783

2764-
if (df_conflict_remains) {
2784+
if (df_conflict_remains || file_in_way) {
27652785
char *new_path;
27662786
if (o->call_depth) {
27672787
remove_file_from_cache(path);
@@ -2795,6 +2815,30 @@ static int merge_content(struct merge_options *o,
27952815
return mfi.clean;
27962816
}
27972817

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+
27982842
/* Per entry merge function */
27992843
static int process_entry(struct merge_options *o,
28002844
const char *path, struct stage_data *entry)
@@ -2814,9 +2858,12 @@ static int process_entry(struct merge_options *o,
28142858
switch (conflict_info->rename_type) {
28152859
case RENAME_NORMAL:
28162860
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);
28202867
break;
28212868
case RENAME_DIR:
28222869
clean_merge = 1;
@@ -2912,7 +2959,7 @@ static int process_entry(struct merge_options *o,
29122959
} else if (a_oid && b_oid) {
29132960
/* Case C: Added in both (check for same permissions) and */
29142961
/* 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 */,
29162963
o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
29172964
NULL);
29182965
} else if (!o_oid && !a_oid && !b_oid) {
@@ -2953,7 +3000,7 @@ int merge_trees(struct merge_options *o,
29533000
return 1;
29543001
}
29553002

2956-
code = git_merge_trees(o->call_depth, common, head, merge);
3003+
code = git_merge_trees(o, common, head, merge);
29573004

29583005
if (code != 0) {
29593006
if (show(o, 4) || o->call_depth)

‎merge-recursive.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#ifndef MERGE_RECURSIVE_H
22
#define MERGE_RECURSIVE_H
33

4+
#include "unpack-trees.h"
45
#include "string-list.h"
56

67
struct merge_options {
@@ -27,6 +28,7 @@ struct merge_options {
2728
struct strbuf obuf;
2829
struct hashmap current_file_dir_set;
2930
struct string_list df_conflict_file_set;
31+
struct unpack_trees_options unpack_opts;
3032
};
3133

3234
/*

‎t/t3501-revert-cherry-pick.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick "-" works with arguments' '
141141
test_cmp expect actual
142142
'
143143

144-
test_expect_failure 'cherry-pick works with dirty renamed file' '
144+
test_expect_success 'cherry-pick works with dirty renamed file' '
145145
test_commit to-rename &&
146146
git checkout -b unrelated &&
147147
test_commit unrelated &&

‎t/t6043-merge-rename-directories.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3298,7 +3298,7 @@ test_expect_success '11a-setup: Avoid losing dirty contents with simple rename'
32983298
)
32993299
'
33003300

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' '
33023302
(
33033303
cd 11a &&
33043304

‎t/t7607-merge-overwrite.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ test_expect_success 'will not overwrite removed file with staged changes' '
9292
test_cmp important c1.c
9393
'
9494

95-
test_expect_failure 'will not overwrite unstaged changes in renamed file' '
95+
test_expect_success 'will not overwrite unstaged changes in renamed file' '
9696
git reset --hard c1 &&
9797
git mv c1.c other.c &&
9898
git commit -m rename &&

‎unpack-trees.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,8 +1509,8 @@ static int verify_uptodate_1(const struct cache_entry *ce,
15091509
add_rejected_path(o, error_type, ce->name);
15101510
}
15111511

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)
15141514
{
15151515
if (!o->skip_sparse_checkout && (ce->ce_flags & CE_NEW_SKIP_WORKTREE))
15161516
return 0;

‎unpack-trees.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#ifndef UNPACK_TREES_H
22
#define UNPACK_TREES_H
33

4+
#include "tree-walk.h"
45
#include "string-list.h"
56

67
#define MAX_UNPACK_TREES 8
@@ -78,6 +79,9 @@ struct unpack_trees_options {
7879
extern int unpack_trees(unsigned n, struct tree_desc *t,
7980
struct unpack_trees_options *options);
8081

82+
int verify_uptodate(const struct cache_entry *ce,
83+
struct unpack_trees_options *o);
84+
8185
int threeway_merge(const struct cache_entry * const *stages,
8286
struct unpack_trees_options *o);
8387
int twoway_merge(const struct cache_entry * const *src,

0 commit comments

Comments
 (0)