Merge branch 'jt/fast-export-copy-modify-fix'
authorJunio C Hamano <gitster@pobox.com>
Fri, 29 Sep 2017 02:23:42 +0000 (11:23 +0900)
committerJunio C Hamano <gitster@pobox.com>
Fri, 29 Sep 2017 02:23:42 +0000 (11:23 +0900)
"git fast-export" with -M/-C option issued "copy" instruction on a
path that is simultaneously modified, which was incorrect.

* jt/fast-export-copy-modify-fix:
fast-export: do not copy from modified file

1  2 
builtin/fast-export.c
t/t9350-fast-export.sh
diff --combined builtin/fast-export.c
index d412c0a8f354659acade8ea6271f5c60826a0b3f,6a85c25ca7806f1ec064af4a70fd675b34ff969f..da42ee5e604b72a8b3ac1edf38d8f75bd6805419
@@@ -5,7 -5,6 +5,7 @@@
   */
  #include "builtin.h"
  #include "cache.h"
 +#include "config.h"
  #include "refs.h"
  #include "commit.h"
  #include "object.h"
@@@ -93,9 -92,8 +93,9 @@@ struct anonymized_entry 
        size_t anon_len;
  };
  
 -static int anonymized_entry_cmp(const void *va, const void *vb,
 -                              const void *data)
 +static int anonymized_entry_cmp(const void *unused_cmp_data,
 +                              const void *va, const void *vb,
 +                              const void *unused_keydata)
  {
        const struct anonymized_entry *a = va, *b = vb;
        return a->orig_len != b->orig_len ||
@@@ -114,7 -112,7 +114,7 @@@ static const void *anonymize_mem(struc
        struct anonymized_entry key, *ret;
  
        if (!map->cmpfn)
 -              hashmap_init(map, anonymized_entry_cmp, 0);
 +              hashmap_init(map, anonymized_entry_cmp, NULL, 0);
  
        hashmap_entry_init(&key, memhash(orig, *len));
        key.orig = orig;
@@@ -214,7 -212,7 +214,7 @@@ static char *anonymize_blob(unsigned lo
        return strbuf_detach(&out, NULL);
  }
  
 -static void export_blob(const unsigned char *sha1)
 +static void export_blob(const struct object_id *oid)
  {
        unsigned long size;
        enum object_type type;
        if (no_data)
                return;
  
 -      if (is_null_sha1(sha1))
 +      if (is_null_oid(oid))
                return;
  
 -      object = lookup_object(sha1);
 +      object = lookup_object(oid->hash);
        if (object && object->flags & SHOWN)
                return;
  
        if (anonymize) {
                buf = anonymize_blob(&size);
 -              object = (struct object *)lookup_blob(sha1);
 +              object = (struct object *)lookup_blob(oid);
                eaten = 0;
        } else {
 -              buf = read_sha1_file(sha1, &type, &size);
 +              buf = read_sha1_file(oid->hash, &type, &size);
                if (!buf)
 -                      die ("Could not read blob %s", sha1_to_hex(sha1));
 -              if (check_sha1_signature(sha1, buf, size, typename(type)) < 0)
 -                      die("sha1 mismatch in blob %s", sha1_to_hex(sha1));
 -              object = parse_object_buffer(sha1, type, size, buf, &eaten);
 +                      die ("Could not read blob %s", oid_to_hex(oid));
 +              if (check_sha1_signature(oid->hash, buf, size, typename(type)) < 0)
 +                      die("sha1 mismatch in blob %s", oid_to_hex(oid));
 +              object = parse_object_buffer(oid, type, size, buf, &eaten);
        }
  
        if (!object)
 -              die("Could not read blob %s", sha1_to_hex(sha1));
 +              die("Could not read blob %s", oid_to_hex(oid));
  
        mark_next_object(object);
  
        printf("blob\nmark :%"PRIu32"\ndata %lu\n", last_idnum, size);
        if (size && fwrite(buf, size, 1, stdout) != 1)
 -              die_errno ("Could not write blob '%s'", sha1_to_hex(sha1));
 +              die_errno ("Could not write blob '%s'", oid_to_hex(oid));
        printf("\n");
  
        show_progress();
@@@ -325,25 -323,26 +325,26 @@@ static void print_path(const char *path
        }
  }
  
 -static void *generate_fake_sha1(const void *old, size_t *len)
 +static void *generate_fake_oid(const void *old, size_t *len)
  {
        static uint32_t counter = 1; /* avoid null sha1 */
 -      unsigned char *out = xcalloc(20, 1);
 -      put_be32(out + 16, counter++);
 +      unsigned char *out = xcalloc(GIT_SHA1_RAWSZ, 1);
 +      put_be32(out + GIT_SHA1_RAWSZ - 4, counter++);
        return out;
  }
  
 -static const unsigned char *anonymize_sha1(const unsigned char *sha1)
 +static const unsigned char *anonymize_sha1(const struct object_id *oid)
  {
        static struct hashmap sha1s;
 -      size_t len = 20;
 -      return anonymize_mem(&sha1s, generate_fake_sha1, sha1, &len);
 +      size_t len = GIT_SHA1_RAWSZ;
 +      return anonymize_mem(&sha1s, generate_fake_oid, oid, &len);
  }
  
  static void show_filemodify(struct diff_queue_struct *q,
                            struct diff_options *options, void *data)
  {
        int i;
+       struct string_list *changed = data;
  
        /*
         * Handle files below a directory first, in case they are all deleted
                case DIFF_STATUS_DELETED:
                        printf("D ");
                        print_path(spec->path);
+                       string_list_insert(changed, spec->path);
                        putchar('\n');
                        break;
  
                case DIFF_STATUS_COPIED:
                case DIFF_STATUS_RENAMED:
-                       printf("%c ", q->queue[i]->status);
-                       print_path(ospec->path);
-                       putchar(' ');
-                       print_path(spec->path);
-                       putchar('\n');
-                       if (!oidcmp(&ospec->oid, &spec->oid) &&
-                           ospec->mode == spec->mode)
-                               break;
+                       /*
+                        * If a change in the file corresponding to ospec->path
+                        * has been observed, we cannot trust its contents
+                        * because the diff is calculated based on the prior
+                        * contents, not the current contents.  So, declare a
+                        * copy or rename only if there was no change observed.
+                        */
+                       if (!string_list_has_string(changed, ospec->path)) {
+                               printf("%c ", q->queue[i]->status);
+                               print_path(ospec->path);
+                               putchar(' ');
+                               print_path(spec->path);
+                               string_list_insert(changed, spec->path);
+                               putchar('\n');
+                               if (!oidcmp(&ospec->oid, &spec->oid) &&
+                                   ospec->mode == spec->mode)
+                                       break;
+                       }
                        /* fallthrough */
  
                case DIFF_STATUS_TYPE_CHANGED:
                        if (no_data || S_ISGITLINK(spec->mode))
                                printf("M %06o %s ", spec->mode,
                                       sha1_to_hex(anonymize ?
 -                                                 anonymize_sha1(spec->oid.hash) :
 +                                                 anonymize_sha1(&spec->oid) :
                                                   spec->oid.hash));
                        else {
                                struct object *object = lookup_object(spec->oid.hash);
                                       get_object_mark(object));
                        }
                        print_path(spec->path);
+                       string_list_insert(changed, spec->path);
                        putchar('\n');
                        break;
  
@@@ -528,7 -539,8 +541,8 @@@ static void anonymize_ident_line(const 
        *end = out->buf + out->len;
  }
  
- static void handle_commit(struct commit *commit, struct rev_info *rev)
+ static void handle_commit(struct commit *commit, struct rev_info *rev,
+                         struct string_list *paths_of_changed_objects)
  {
        int saved_output_format = rev->diffopt.output_format;
        const char *commit_buffer;
            get_object_mark(&commit->parents->item->object) != 0 &&
            !full_tree) {
                parse_commit_or_die(commit->parents->item);
 -              diff_tree_sha1(commit->parents->item->tree->object.oid.hash,
 -                             commit->tree->object.oid.hash, "", &rev->diffopt);
 +              diff_tree_oid(&commit->parents->item->tree->object.oid,
 +                            &commit->tree->object.oid, "", &rev->diffopt);
        }
        else
 -              diff_root_tree_sha1(commit->tree->object.oid.hash,
 -                                  "", &rev->diffopt);
 +              diff_root_tree_oid(&commit->tree->object.oid,
 +                                 "", &rev->diffopt);
  
        /* Export the referenced blobs, and remember the marks. */
        for (i = 0; i < diff_queued_diff.nr; i++)
                if (!S_ISGITLINK(diff_queued_diff.queue[i]->two->mode))
 -                      export_blob(diff_queued_diff.queue[i]->two->oid.hash);
 +                      export_blob(&diff_queued_diff.queue[i]->two->oid);
  
        refname = commit->util;
        if (anonymize) {
        if (full_tree)
                printf("deleteall\n");
        log_tree_diff_flush(rev);
+       string_list_clear(paths_of_changed_objects, 0);
        rev->diffopt.output_format = saved_output_format;
  
        printf("\n");
@@@ -630,14 -643,15 +645,15 @@@ static void *anonymize_tag(const void *
        return strbuf_detach(&out, len);
  }
  
- static void handle_tail(struct object_array *commits, struct rev_info *revs)
+ static void handle_tail(struct object_array *commits, struct rev_info *revs,
+                       struct string_list *paths_of_changed_objects)
  {
        struct commit *commit;
        while (commits->nr) {
                commit = (struct commit *)commits->objects[commits->nr - 1].item;
                if (has_unshown_parent(commit))
                        return;
-               handle_commit(commit, revs);
+               handle_commit(commit, revs, paths_of_changed_objects);
                commits->nr--;
        }
  }
@@@ -736,7 -750,6 +752,7 @@@ static void handle_tag(const char *name
                             oid_to_hex(&tag->object.oid));
                case DROP:
                        /* Ignore this tag altogether */
 +                      free(buf);
                        return;
                case REWRITE:
                        if (tagged->type != OBJ_COMMIT) {
               (int)(tagger_end - tagger), tagger,
               tagger == tagger_end ? "" : "\n",
               (int)message_size, (int)message_size, message ? message : "");
 +      free(buf);
  }
  
  static struct commit *get_commit(struct rev_cmdline_entry *e, char *full_name)
  
                /* handle nested tags */
                while (tag && tag->object.type == OBJ_TAG) {
 -                      parse_object(tag->object.oid.hash);
 +                      parse_object(&tag->object.oid);
                        string_list_append(&extra_refs, full_name)->util = tag;
                        tag = (struct tag *)tag->tagged;
                }
@@@ -801,14 -813,14 +817,14 @@@ static void get_tags_and_duplicates(str
  
        for (i = 0; i < info->nr; i++) {
                struct rev_cmdline_entry *e = info->rev + i;
 -              unsigned char sha1[20];
 +              struct object_id oid;
                struct commit *commit;
                char *full_name;
  
                if (e->flags & UNINTERESTING)
                        continue;
  
 -              if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
 +              if (dwim_ref(e->name, strlen(e->name), oid.hash, &full_name) != 1)
                        continue;
  
                if (refspecs) {
                case OBJ_COMMIT:
                        break;
                case OBJ_BLOB:
 -                      export_blob(commit->object.oid.hash);
 +                      export_blob(&commit->object.oid);
                        continue;
                default: /* OBJ_TAG (nested tags) is already handled */
                        warning("Tag points to object of unexpected type %s, skipping.",
@@@ -909,12 -921,14 +925,12 @@@ static void export_marks(char *file
  static void import_marks(char *input_file)
  {
        char line[512];
 -      FILE *f = fopen(input_file, "r");
 -      if (!f)
 -              die_errno("cannot read '%s'", input_file);
 +      FILE *f = xfopen(input_file, "r");
  
        while (fgets(line, sizeof(line), f)) {
                uint32_t mark;
                char *line_end, *mark_end;
 -              unsigned char sha1[20];
 +              struct object_id oid;
                struct object *object;
                struct commit *commit;
                enum object_type type;
  
                mark = strtoumax(line + 1, &mark_end, 10);
                if (!mark || mark_end == line + 1
 -                      || *mark_end != ' ' || get_sha1_hex(mark_end + 1, sha1))
 +                      || *mark_end != ' ' || get_oid_hex(mark_end + 1, &oid))
                        die("corrupt mark line: %s", line);
  
                if (last_idnum < mark)
                        last_idnum = mark;
  
 -              type = sha1_object_info(sha1, NULL);
 +              type = sha1_object_info(oid.hash, NULL);
                if (type < 0)
 -                      die("object not found: %s", sha1_to_hex(sha1));
 +                      die("object not found: %s", oid_to_hex(&oid));
  
                if (type != OBJ_COMMIT)
                        /* only commits */
                        continue;
  
 -              commit = lookup_commit(sha1);
 +              commit = lookup_commit(&oid);
                if (!commit)
 -                      die("not a commit? can't happen: %s", sha1_to_hex(sha1));
 +                      die("not a commit? can't happen: %s", oid_to_hex(&oid));
  
                object = &commit->object;
  
                if (object->flags & SHOWN)
 -                      error("Object %s already has a mark", sha1_to_hex(sha1));
 +                      error("Object %s already has a mark", oid_to_hex(&oid));
  
                mark_object(object, mark);
  
@@@ -977,6 -991,7 +993,7 @@@ int cmd_fast_export(int argc, const cha
        char *export_filename = NULL, *import_filename = NULL;
        uint32_t lastimportid;
        struct string_list refspecs_list = STRING_LIST_INIT_NODUP;
+       struct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;
        struct option options[] = {
                OPT_INTEGER(0, "progress", &progress,
                            N_("show progress after <n> objects")),
        if (prepare_revision_walk(&revs))
                die("revision walk setup failed");
        revs.diffopt.format_callback = show_filemodify;
+       revs.diffopt.format_callback_data = &paths_of_changed_objects;
        DIFF_OPT_SET(&revs.diffopt, RECURSIVE);
        while ((commit = get_revision(&revs))) {
                if (has_unshown_parent(commit)) {
                        add_object_array(&commit->object, NULL, &commits);
                }
                else {
-                       handle_commit(commit, &revs);
-                       handle_tail(&commits, &revs);
+                       handle_commit(commit, &revs, &paths_of_changed_objects);
+                       handle_tail(&commits, &revs, &paths_of_changed_objects);
                }
        }
  
diff --combined t/t9350-fast-export.sh
index 8dcb05c4a5711e95bb64d8ba310e3490744fc015,419d540d35e20dbbb65879736cc8412ff504b7f2..866ddf60581e3fea1afdbe28e71e7f3137da5722
@@@ -70,7 -70,7 +70,7 @@@ test_expect_success 'iso-8859-1' 
  
        git config i18n.commitencoding ISO8859-1 &&
        # use author and committer name in ISO-8859-1 to match it.
 -      . "$TEST_DIRECTORY"/t3901-8859-1.txt &&
 +      . "$TEST_DIRECTORY"/t3901/8859-1.txt &&
        test_tick &&
        echo rosten >file &&
        git commit -s -m den file &&
@@@ -234,7 -234,7 +234,7 @@@ test_expect_success 'fast-export -C -C 
        mkdir new &&
        git --git-dir=new/.git init &&
        git fast-export -C -C --signed-tags=strip --all > output &&
-       grep "^C file6 file7\$" output &&
+       grep "^C file2 file4\$" output &&
        cat output |
        (cd new &&
         git fast-import &&
@@@ -522,4 -522,22 +522,22 @@@ test_expect_success 'delete refspec' 
        test_cmp expected actual
  '
  
+ test_expect_success 'when using -C, do not declare copy when source of copy is also modified' '
+       test_create_repo src &&
+       echo a_line >src/file.txt &&
+       git -C src add file.txt &&
+       git -C src commit -m 1st_commit &&
+       cp src/file.txt src/file2.txt &&
+       echo another_line >>src/file.txt &&
+       git -C src add file.txt file2.txt &&
+       git -C src commit -m 2nd_commit &&
+       test_create_repo dst &&
+       git -C src fast-export --all -C | git -C dst fast-import &&
+       git -C src show >expected &&
+       git -C dst show >actual &&
+       test_cmp expected actual
+ '
  test_done