{"thread":{"id":"47494","subject":"[PATCH 0/2] restore fast-export full filecopy detection","startedAt":"2017-12-24T10:55:07Z","lastAt":"2018-01-07T16:56:16Z","messageCount":4,"participants":["Mark Nauwelaerts"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"335249","messageId":"1514112729-31647-1-git-send-email-mark.nauwelaerts@gmail.com","threadId":"47494","inReplyTo":null,"subject":"[PATCH 0/2] restore fast-export full filecopy detection","fromName":"Mark Nauwelaerts","fromEmail":"mark.nauwelaerts@gmail.com","sentAt":"2017-12-24T10:52:07Z","receivedAt":"2017-12-24T10:55:07Z","isPatch":true,"sender":{"key":"mark.nauwelaerts@gmail.com","avatar":null},"body":"From: Mark Nauwelaerts \n\nWhen using fast-export/fast-import to interface/bridge with another VCS\nthat explicitly tracks copy/rename as metadata, fast-export's ability\nto report filecopy/filerename is quite useful (if not essential).\n\nThere has been a fix in this area recently with as side-effect that\nin some scenarios a filecopy is no longer reported as such.\nThese few patches provide an alternative fix for the original problem\nwhile still retaining previous fast-export's filecopy reporting.\n\nSee patches for additional details and commit references.\n\n\nMark Nauwelaerts (2):\n fast-export: ensure proper order of modify, copy and rename entries\n fast-export: remove now obsolete filtering of modified files\n\n builtin/fast-export.c | 60 +++++++++++++++++++++++---------------------------\n t/t9350-fast-export.sh | 2 +-\n 2 files changed, 28 insertions(+), 34 deletions(-)\n\n-- \n2.7.4\n\n"},{"id":"335250","messageId":"1514112729-31647-3-git-send-email-mark.nauwelaerts@gmail.com","threadId":"47494","inReplyTo":"1514112729-31647-1-git-send-email-mark.nauwelaerts@gmail.com","subject":"[PATCH 2/2] fast-export: remove now obsolete filtering of modified files","fromName":"Mark Nauwelaerts","fromEmail":"mark.nauwelaerts@gmail.com","sentAt":"2017-12-24T10:52:09Z","receivedAt":"2017-12-24T10:55:12Z","isPatch":true,"sender":{"key":"mark.nauwelaerts@gmail.com","avatar":null},"body":"From: Mark Nauwelaerts \n\nRevert b3e8ca8 (\"fast-export: do not copy from modified file\", 2017-09-20)\npartially, since it is not necessary to filter out 'C' commands if the 'M'\nand 'C' commands are in correct order.\n\nThe unit test that was added is kept around though, and it still passes\nwith this new approach.\n\nSigned-off-by: Mark Nauwelaerts \n---\n builtin/fast-export.c | 46 ++++++++++++++--------------------------------\n t/t9350-fast-export.sh | 2 +-\n 2 files changed, 15 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 1b3e250..6aa4073 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -356,7 +356,6 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t struct diff_options *options, void *data)\n {\n \tint i;\n-\tstruct string_list *changed = data;\n \n \t/*\n \t * Handle files below a directory first, in case they are all deleted\n@@ -372,31 +371,20 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t\tcase DIFF_STATUS_DELETED:\n \t\t\tprintf(\"D \");\n \t\t\tprint_path(spec->path);\n-\t\t\tstring_list_insert(changed, spec->path);\n \t\t\tputchar('\\n');\n \t\t\tbreak;\n \n \t\tcase DIFF_STATUS_COPIED:\n \t\tcase DIFF_STATUS_RENAMED:\n-\t\t\t/*\n-\t\t\t * If a change in the file corresponding to ospec->path\n-\t\t\t * has been observed, we cannot trust its contents\n-\t\t\t * because the diff is calculated based on the prior\n-\t\t\t * contents, not the current contents. So, declare a\n-\t\t\t * copy or rename only if there was no change observed.\n-\t\t\t */\n-\t\t\tif (!string_list_has_string(changed, ospec->path)) {\n-\t\t\t\tprintf(\"%c \", q->queue[i]->status);\n-\t\t\t\tprint_path(ospec->path);\n-\t\t\t\tputchar(' ');\n-\t\t\t\tprint_path(spec->path);\n-\t\t\t\tstring_list_insert(changed, spec->path);\n-\t\t\t\tputchar('\\n');\n-\n-\t\t\t\tif (!oidcmp(&ospec->oid, &spec->oid) &&\n-\t\t\t\t ospec->mode == spec->mode)\n-\t\t\t\t\tbreak;\n-\t\t\t}\n+\t\t\tprintf(\"%c \", q->queue[i]->status);\n+\t\t\tprint_path(ospec->path);\n+\t\t\tputchar(' ');\n+\t\t\tprint_path(spec->path);\n+\t\t\tputchar('\\n');\n+\n+\t\t\tif (!oidcmp(&ospec->oid, &spec->oid) &&\n+\t\t\t ospec->mode == spec->mode)\n+\t\t\t\tbreak;\n \t\t\t/* fallthrough */\n \n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n@@ -417,7 +405,6 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t\t get_object_mark(object));\n \t\t\t}\n \t\t\tprint_path(spec->path);\n-\t\t\tstring_list_insert(changed, spec->path);\n \t\t\tputchar('\\n');\n \t\t\tbreak;\n \n@@ -553,8 +540,7 @@ static void anonymize_ident_line(const char **beg, const char **end)\n \t*end = out->buf + out->len;\n }\n \n-static void handle_commit(struct commit *commit, struct rev_info *rev,\n-\t\t\t struct string_list *paths_of_changed_objects)\n+static void handle_commit(struct commit *commit, struct rev_info *rev)\n {\n \tint saved_output_format = rev->diffopt.output_format;\n \tconst char *commit_buffer;\n@@ -641,7 +627,6 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \tif (full_tree)\n \t\tprintf(\"deleteall\\n\");\n \tlog_tree_diff_flush(rev);\n-\tstring_list_clear(paths_of_changed_objects, 0);\n \trev->diffopt.output_format = saved_output_format;\n \n \tprintf(\"\\n\");\n@@ -657,15 +642,14 @@ static void *anonymize_tag(const void *old, size_t *len)\n \treturn strbuf_detach(&out, len);\n }\n \n-static void handle_tail(struct object_array *commits, struct rev_info *revs,\n-\t\t\tstruct string_list *paths_of_changed_objects)\n+static void handle_tail(struct object_array *commits, struct rev_info *revs)\n {\n \tstruct commit *commit;\n \twhile (commits->nr) {\n \t\tcommit = (struct commit *)object_array_pop(commits);\n \t\tif (has_unshown_parent(commit))\n \t\t\treturn;\n-\t\thandle_commit(commit, revs, paths_of_changed_objects);\n+\t\thandle_commit(commit, revs);\n \t}\n }\n \n@@ -1004,7 +988,6 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tchar *export_filename = NULL, *import_filename = NULL;\n \tuint32_t lastimportid;\n \tstruct string_list refspecs_list = STRING_LIST_INIT_NODUP;\n-\tstruct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"progress\", &progress,\n \t\t\t N_(\"show progress after objects\")),\n@@ -1077,15 +1060,14 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\n \trevs.diffopt.format_callback = show_filemodify;\n-\trevs.diffopt.format_callback_data = &paths_of_changed_objects;\n \tDIFF_OPT_SET(&revs.diffopt, RECURSIVE);\n \twhile ((commit = get_revision(&revs))) {\n \t\tif (has_unshown_parent(commit)) {\n \t\t\tadd_object_array(&commit->object, NULL, &commits);\n \t\t}\n \t\telse {\n-\t\t\thandle_commit(commit, &revs, &paths_of_changed_objects);\n-\t\t\thandle_tail(&commits, &revs, &paths_of_changed_objects);\n+\t\t\thandle_commit(commit, &revs);\n+\t\t\thandle_tail(&commits, &revs);\n \t\t}\n \t}\n \ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 866ddf6..06c66a1 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -522,7 +522,7 @@ test_expect_success 'delete refspec' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'when using -C, do not declare copy when source of copy is also modified' '\n+test_expect_success 'when using -C, correctly order copy and modify of source of copy' '\n \ttest_create_repo src &&\n \techo a_line >src/file.txt &&\n \tgit -C src add file.txt &&\n-- \n2.7.4\n\n"},{"id":"335251","messageId":"1514112729-31647-2-git-send-email-mark.nauwelaerts@gmail.com","threadId":"47494","inReplyTo":"1514112729-31647-1-git-send-email-mark.nauwelaerts@gmail.com","subject":"[PATCH 1/2] fast-export: ensure proper order of modify, copy and rename entries","fromName":"Mark Nauwelaerts","fromEmail":"mark.nauwelaerts@gmail.com","sentAt":"2017-12-24T10:52:08Z","receivedAt":"2017-12-24T10:55:14Z","isPatch":true,"sender":{"key":"mark.nauwelaerts@gmail.com","avatar":null},"body":"From: Mark Nauwelaerts \n\nAs b3e8ca8 (\"fast-export: do not copy from modified file\", 2017-09-20)\nindicates, if a commit both modifies a file and uses it as a source for a\ncopy, then the specifications of git-fast-import require that the copy\nshould be reported first followed by the modification to the source file.\n\nThe commit above addressed the problem by never reporting the copy. However,\nthe copy can still be reported if the entries are sorted properly. That\ncan be achieved by adjusting the order of the sort that is performed anyway\nfor other reasons of consistency. This is merely an extra order condition.\n\nFurthermore, when using fast-export to export or bridge to another\nversion control system which explicitly tracks copies, then the 'C' commands\nin the output are quite useful and necessary.\n\nSigned-off-by: Mark Nauwelaerts \n---\n builtin/fast-export.c | 14 +++++++++++++-\n 1 file changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 2fb60d6..1b3e250 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -262,6 +262,14 @@ static void export_blob(const struct object_id *oid)\n \t\tfree(buf);\n }\n \n+static int order_status(const char status)\n+{\n+\tif (status == 'R')\n+\t\treturn 2;\n+\telse\n+\t\treturn status == 'M' ? 1 : 0;\n+}\n+\n static int depth_first(const void *a_, const void *b_)\n {\n \tconst struct diff_filepair *a = *((const struct diff_filepair **)a_);\n@@ -288,8 +296,12 @@ static int depth_first(const void *a_, const void *b_)\n \t * Move 'R'ename entries last so that all references of the file\n \t * appear in the output before it is renamed (e.g., when a file\n \t * was copied and renamed in the same commit).\n+\t * Moreover, 'C' needs to go before 'M' if the file was copied\n+\t * and then modified a bit, as it should be done that way as well\n+\t * at import time (also recall that 'C' is calculated on the\n+\t * original content).\n \t */\n-\treturn (a->status == 'R') - (b->status == 'R');\n+\treturn order_status(a->status) - order_status(b->status);\n }\n \n static void print_path_1(const char *path)\n-- \n2.7.4\n\n"},{"id":"336085","messageId":"5A525127.5010409@gmail.com","threadId":"47494","inReplyTo":"1514112729-31647-1-git-send-email-mark.nauwelaerts@gmail.com","subject":"Re: [PATCH 0/2] restore fast-export full filecopy detection","fromName":"Mark Nauwelaerts","fromEmail":"mark.nauwelaerts@gmail.com","sentAt":"2018-01-07T16:56:07Z","receivedAt":"2018-01-07T16:56:16Z","isPatch":true,"sender":{"key":"mark.nauwelaerts@gmail.com","avatar":null},"body":"On 24/12/17 11:52, Mark Nauwelaerts wrote:\n> From: Mark Nauwelaerts \n>\n> When using fast-export/fast-import to interface/bridge with another VCS\n> that explicitly tracks copy/rename as metadata, fast-export's ability\n> to report filecopy/filerename is quite useful (if not essential).\n>\n> There has been a fix in this area recently with as side-effect that\n> in some scenarios a filecopy is no longer reported as such.\n> These few patches provide an alternative fix for the original problem\n> while still retaining previous fast-export's filecopy reporting.\n>\n> See patches for additional details and commit references.\nAny comments or opinions on these patches? Could they be applied and included?\n\nThanks,\nMark.\n"}]}