fast-export: do not copy from modified file
When run with the "-C" option, fast-export writes 'C' commands in its output whenever the internal diff mechanism detects a file copy, indicating that fast-import should copy the given existing file to the given new filename. However, the diff mechanism works against the prior version of the file, whereas fast-import uses whatever is current. This causes issues when a commit both modifies a file and uses it as the source for a copy. Therefore, teach fast-export to refrain from writing 'C' when it has already written a modification command for a file. An existing test in t9350-fast-export is also fixed in this patch. The existing line "C file6 file7" copies the wrong version of file6, but it has coincidentally worked because file7 was subsequently overridden. Reported-by: Juraj Oršulić <juraj.orsulic@fer.hr> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan committed
Sep 20, 2017 at 16:55 UTC
b3e8ca89cfe34712a093c37fdbac7df7eaae91e7
2 files changed
+51
-15
builtin/fast-export.c
+32
-14
@@ -342,6 +342,7 @@ static void show_filemodify(struct diff_queue_struct *q,
342
struct diff_options *options, void *data)
343
{
344
int i;
345
+ struct string_list *changed = data;
346
347
/*
348
* Handle files below a directory first, in case they are all deleted
@@ -357,20 +358,31 @@ static void show_filemodify(struct diff_queue_struct *q,
358
case DIFF_STATUS_DELETED:
359
printf("D ");
360
print_path(spec->path);
361
+ string_list_insert(changed, spec->path);
362
putchar('\n');
363
break;
364
365
case DIFF_STATUS_COPIED:
366
case DIFF_STATUS_RENAMED:
365
- printf("%c ", q->queue[i]->status);
366
- print_path(ospec->path);
367
- putchar(' ');
368
- print_path(spec->path);
369
- putchar('\n');
370
-
371
- if (!oidcmp(&ospec->oid, &spec->oid) &&
372
- ospec->mode == spec->mode)
373
- break;
367
+ /*
368
+ * If a change in the file corresponding to ospec->path
369
+ * has been observed, we cannot trust its contents
370
+ * because the diff is calculated based on the prior
371
+ * contents, not the current contents. So, declare a
372
+ * copy or rename only if there was no change observed.
373
+ */
374
+ if (!string_list_has_string(changed, ospec->path)) {
375
+ printf("%c ", q->queue[i]->status);
376
+ print_path(ospec->path);
377
+ putchar(' ');
378
+ print_path(spec->path);
379
+ string_list_insert(changed, spec->path);
380
+ putchar('\n');
381
+
382
+ if (!oidcmp(&ospec->oid, &spec->oid) &&
383
+ ospec->mode == spec->mode)
384
+ break;
385
+ }
386
/* fallthrough */
387
388
case DIFF_STATUS_TYPE_CHANGED:
@@ -391,6 +403,7 @@ static void show_filemodify(struct diff_queue_struct *q,
403
get_object_mark(object));
404
}
405
print_path(spec->path);
406
+ string_list_insert(changed, spec->path);
407
putchar('\n');
408
break;
409
@@ -526,7 +539,8 @@ static void anonymize_ident_line(const char **beg, const char **end)
539
*end = out->buf + out->len;
540
}
541
529
-static void handle_commit(struct commit *commit, struct rev_info *rev)
542
+static void handle_commit(struct commit *commit, struct rev_info *rev,
543
+ struct string_list *paths_of_changed_objects)
544
{
545
int saved_output_format = rev->diffopt.output_format;
546
const char *commit_buffer;
@@ -613,6 +627,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
627
if (full_tree)
628
printf("deleteall\n");
629
log_tree_diff_flush(rev);
630
+ string_list_clear(paths_of_changed_objects, 0);
631
rev->diffopt.output_format = saved_output_format;
632
633
printf("\n");
@@ -628,14 +643,15 @@ static void *anonymize_tag(const void *old, size_t *len)
643
return strbuf_detach(&out, len);
644
}
645
631
-static void handle_tail(struct object_array *commits, struct rev_info *revs)
646
+static void handle_tail(struct object_array *commits, struct rev_info *revs,
647
+ struct string_list *paths_of_changed_objects)
648
{
649
struct commit *commit;
650
while (commits->nr) {
651
commit = (struct commit *)commits->objects[commits->nr - 1].item;
652
if (has_unshown_parent(commit))
653
return;
638
- handle_commit(commit, revs);
654
+ handle_commit(commit, revs, paths_of_changed_objects);
655
commits->nr--;
656
}
657
}
@@ -975,6 +991,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
991
char *export_filename = NULL, *import_filename = NULL;
992
uint32_t lastimportid;
993
struct string_list refspecs_list = STRING_LIST_INIT_NODUP;
994
+ struct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;
995
struct option options[] = {
996
OPT_INTEGER(0, "progress", &progress,
997
N_("show progress after <n> objects")),
@@ -1047,14 +1064,15 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
1064
if (prepare_revision_walk(&revs))
1065
die("revision walk setup failed");
1066
revs.diffopt.format_callback = show_filemodify;
1067
+ revs.diffopt.format_callback_data = &paths_of_changed_objects;
1068
DIFF_OPT_SET(&revs.diffopt, RECURSIVE);
1069
while ((commit = get_revision(&revs))) {
1070
if (has_unshown_parent(commit)) {
1071
add_object_array(&commit->object, NULL, &commits);
1072
}
1073
else {
1056
- handle_commit(commit, &revs);
1057
- handle_tail(&commits, &revs);
1074
+ handle_commit(commit, &revs, &paths_of_changed_objects);
1075
+ handle_tail(&commits, &revs, &paths_of_changed_objects);
1076
}
1077
}
1078
t/t9350-fast-export.sh
+19
-1
@@ -234,7 +234,7 @@ test_expect_success 'fast-export -C -C | fast-import' '
234
mkdir new &&
235
git --git-dir=new/.git init &&
236
git fast-export -C -C --signed-tags=strip --all > output &&
237
- grep "^C file6 file7\$" output &&
237
+ grep "^C file2 file4\$" output &&
238
cat output |
239
(cd new &&
240
git fast-import &&
@@ -522,4 +522,22 @@ test_expect_success 'delete refspec' '
522
test_cmp expected actual
523
'
524
525
+test_expect_success 'when using -C, do not declare copy when source of copy is also modified' '
526
+ test_create_repo src &&
527
+ echo a_line >src/file.txt &&
528
+ git -C src add file.txt &&
529
+ git -C src commit -m 1st_commit &&
530
+
531
+ cp src/file.txt src/file2.txt &&
532
+ echo another_line >>src/file.txt &&
533
+ git -C src add file.txt file2.txt &&
534
+ git -C src commit -m 2nd_commit &&
535
+
536
+ test_create_repo dst &&
537
+ git -C src fast-export --all -C | git -C dst fast-import &&
538
+ git -C src show >expected &&
539
+ git -C dst show >actual &&
540
+ test_cmp expected actual
541
+'
542
+
543
test_done