merge-recursive: fix was_tracked() to quit lying with some renamed paths
In commit aacb82de3ff8 ("merge-recursive: Split was_tracked() out of would_lose_untracked()", 2011-08-11), was_tracked() was split out of would_lose_untracked() with the intent to provide a function that could answer whether a path was tracked in the index before the merge. Sadly, it instead returned whether the path was in the working tree due to having been tracked in the index before the merge OR having been written there by unpack_trees(). The distinction is important when renames are involved, e.g. for a merge where: HEAD: modifies path b other: renames b->c In this case, c was not tracked in the index before the merge, but would have been added to the index at stage 0 and written to the working tree by unpack_trees(). would_lose_untracked() is more interested in the in-working-copy-for-either-reason behavior, while all other uses of was_tracked() want just was-it-tracked-in-index-before-merge behavior. Unsplit would_lose_untracked() and write a new was_tracked() function which answers whether a path was tracked in the index before the merge started. This will also affect was_dirty(), helping it to return better results since it can base answers off the original index rather than an index that possibly only copied over some of the stat information. However, was_dirty() will need an additional change that will be made in a subsequent patch. 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
a35edc84bdd6134d7aaa7bb0d220941154fe9e7e
2 files changed
+68
-24
merge-recursive.c
+67
-24
@@ -344,6 +344,7 @@ static int git_merge_trees(struct merge_options *o,
344
{
345
int rc;
346
struct tree_desc t[3];
347
+ struct index_state tmp_index = { NULL };
348
349
memset(&o->unpack_opts, 0, sizeof(o->unpack_opts));
350
if (o->call_depth)
@@ -354,7 +355,7 @@ static int git_merge_trees(struct merge_options *o,
355
o->unpack_opts.head_idx = 2;
356
o->unpack_opts.fn = threeway_merge;
357
o->unpack_opts.src_index = &the_index;
357
- o->unpack_opts.dst_index = &the_index;
358
+ o->unpack_opts.dst_index = &tmp_index;
359
setup_unpack_trees_porcelain(&o->unpack_opts, "merge");
360
361
init_tree_desc_from_tree(t+0, common);
@@ -362,13 +363,18 @@ static int git_merge_trees(struct merge_options *o,
363
init_tree_desc_from_tree(t+2, merge);
364
365
rc = unpack_trees(3, t, &o->unpack_opts);
366
+ cache_tree_free(&active_cache_tree);
367
+
368
/*
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
+ * Update the_index to match the new results, AFTER saving a copy
370
+ * in o->orig_index. Update src_index to point to the saved copy.
371
+ * (verify_uptodate() checks src_index, and the original index is
372
+ * the one that had the necessary modification timestamps.)
373
*/
370
- o->unpack_opts.src_index = &the_index;
371
- cache_tree_free(&active_cache_tree);
374
+ o->orig_index = the_index;
375
+ the_index = tmp_index;
376
+ o->unpack_opts.src_index = &o->orig_index;
377
+
378
return rc;
379
}
380
@@ -773,31 +779,59 @@ static int dir_in_way(const char *path, int check_working_copy, int empty_ok)
779
!(empty_ok && is_empty_dir(path));
780
}
781
776
-static int was_tracked(const char *path)
782
+/*
783
+ * Returns whether path was tracked in the index before the merge started
784
+ */
785
+static int was_tracked(struct merge_options *o, const char *path)
786
{
778
- int pos = cache_name_pos(path, strlen(path));
787
+ int pos = index_name_pos(&o->orig_index, path, strlen(path));
788
789
if (0 <= pos)
781
- /* we have been tracking this path */
790
+ /* we were tracking this path before the merge */
791
return 1;
792
784
- /*
785
- * Look for an unmerged entry for the path,
786
- * specifically stage #2, which would indicate
787
- * that "our" side before the merge started
788
- * had the path tracked (and resulted in a conflict).
789
- */
790
- for (pos = -1 - pos;
791
- pos < active_nr && !strcmp(path, active_cache[pos]->name);
792
- pos++)
793
- if (ce_stage(active_cache[pos]) == 2)
794
- return 1;
793
return 0;
794
}
795
796
static int would_lose_untracked(const char *path)
797
{
800
- return !was_tracked(path) && file_exists(path);
798
+ /*
799
+ * This may look like it can be simplified to:
800
+ * return !was_tracked(o, path) && file_exists(path)
801
+ * but it can't. This function needs to know whether path was in
802
+ * the working tree due to EITHER having been tracked in the index
803
+ * before the merge OR having been put into the working copy and
804
+ * index by unpack_trees(). Due to that either-or requirement, we
805
+ * check the current index instead of the original one.
806
+ *
807
+ * Note that we do not need to worry about merge-recursive itself
808
+ * updating the index after unpack_trees() and before calling this
809
+ * function, because we strictly require all code paths in
810
+ * merge-recursive to update the working tree first and the index
811
+ * second. Doing otherwise would break
812
+ * update_file()/would_lose_untracked(); see every comment in this
813
+ * file which mentions "update_stages".
814
+ */
815
+ int pos = cache_name_pos(path, strlen(path));
816
+
817
+ if (pos < 0)
818
+ pos = -1 - pos;
819
+ while (pos < active_nr &&
820
+ !strcmp(path, active_cache[pos]->name)) {
821
+ /*
822
+ * If stage #0, it is definitely tracked.
823
+ * If it has stage #2 then it was tracked
824
+ * before this merge started. All other
825
+ * cases the path was not tracked.
826
+ */
827
+ switch (ce_stage(active_cache[pos])) {
828
+ case 0:
829
+ case 2:
830
+ return 0;
831
+ }
832
+ pos++;
833
+ }
834
+ return file_exists(path);
835
}
836
837
static int was_dirty(struct merge_options *o, const char *path)
@@ -805,7 +839,7 @@ static int was_dirty(struct merge_options *o, const char *path)
839
struct cache_entry *ce;
840
int dirty = 1;
841
808
- if (o->call_depth || !was_tracked(path))
842
+ if (o->call_depth || !was_tracked(o, path))
843
return !dirty;
844
845
ce = cache_file_exists(path, strlen(path), ignore_case);
@@ -2419,7 +2453,7 @@ static int process_renames(struct merge_options *o,
2453
* add-source case).
2454
*/
2455
remove_file(o, 1, ren1_src,
2422
- renamed_stage == 2 || !was_tracked(ren1_src));
2456
+ renamed_stage == 2 || !was_tracked(o, ren1_src));
2457
2458
oidcpy(&src_other.oid,
2459
&ren1->src_entry->stages[other_stage].oid);
@@ -2812,7 +2846,7 @@ static int merge_content(struct merge_options *o,
2846
if (update_stages(o, path, &one, &a, &b))
2847
return -1;
2848
} else {
2815
- int file_from_stage2 = was_tracked(path);
2849
+ int file_from_stage2 = was_tracked(o, path);
2850
struct diff_filespec merged;
2851
oidcpy(&merged.oid, &mfi.oid);
2852
merged.mode = mfi.mode;
@@ -3081,6 +3115,15 @@ cleanup:
3115
else
3116
clean = 1;
3117
3118
+ /* Free the extra index left from git_merge_trees() */
3119
+ /*
3120
+ * FIXME: Need to also free data allocated by
3121
+ * setup_unpack_trees_porcelain() tucked away in o->unpack_opts.msgs,
3122
+ * but the problem is that only half of it refers to dynamically
3123
+ * allocated data, while the other half points at static strings.
3124
+ */
3125
+ discard_index(&o->orig_index);
3126
+
3127
if (o->call_depth && !(*result = write_tree_from_memory(o)))
3128
return -1;
3129
merge-recursive.h
+1
@@ -29,6 +29,7 @@ struct merge_options {
29
struct hashmap current_file_dir_set;
30
struct string_list df_conflict_file_set;
31
struct unpack_trees_options unpack_opts;
32
+ struct index_state orig_index;
33
};
34
35
/*