{"thread":{"id":"19899","subject":"A few fast export fixups -- round 2","startedAt":"2009-06-22T13:06:35Z","lastAt":"2009-06-26T04:45:13Z","messageCount":11,"participants":["newren@gmail.com","Junio C Hamano","Elijah Newren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"116763","messageId":"1245676001-14734-1-git-send-email-newren@gmail.com","threadId":"19899","inReplyTo":null,"subject":"A few fast export fixups -- round 2","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-22T13:06:35Z","receivedAt":"2009-06-22T13:06:35Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This patch series fixes a few fast-export bugs I have come across,\nplus some new testcases to verify the fixes, and finally a small\naddition to the documentation.\n\nChanges since v1:\n - fixed test issues identified by Johannes Sixt and Stephen Boyd\n - squashed patch #2 into patch #1\n\nElijah Newren (6):\n fast-export: Omit tags that tag trees\n fast-export: Make sure we show actual ref names instead of \"(null)\"\n fast-export: Do parent rewriting to avoid dropping relevant commits\n fast-export: Add a --tag-of-filtered-object option for newly dangling tags\n Add new fast-export testcases\n fast-export: Document the fact that git-rev-list arguments are accepted\n\n Documentation/git-fast-export.txt | 17 +++++++++\n builtin-fast-export.c | 72 +++++++++++++++++++++++++++++++++---\n revision.c | 10 +----\n revision.h | 8 ++++\n t/t9301-fast-export.sh | 63 +++++++++++++++++++++++++++++++-\n 5 files changed, 155 insertions(+), 15 deletions(-)\n"},{"id":"116764","messageId":"1245676001-14734-2-git-send-email-newren@gmail.com","threadId":"19899","inReplyTo":"1245676001-14734-1-git-send-email-newren@gmail.com","subject":"[PATCH v2] fast-export: Omit tags that tag trees","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-22T13:06:36Z","receivedAt":"2009-06-22T13:06:36Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit c0582c53bcf4e83bba70e1ad23abbad31f96ebc8 introduced logic to just\nomit tags that point to tree objects. However, a case was missed\nresulting in a tag being output which pointed at \"mark :0\", which would\ncause fast-import to crash.\n\nSigned-off-by: Elijah Newren \n---\nIncludes changes suggested by Stephen Boyd (squashing commits together\nand moving the NEEDSWORK comment to where it is still relevant) and\nJohannes Sixt (fixing the testcase to remain in the right directory\neven if git init fails and to catch failures in git fast-export).\n\n builtin-fast-export.c | 8 +++++++-\n t/t9301-fast-export.sh | 8 +++++++-\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex 6cef810..891e2d4 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -407,9 +407,15 @@ static void handle_tags_and_duplicates(struct string_list *extra_refs)\n \tfor (i = extra_refs->nr - 1; i >= 0; i--) {\n \t\tconst char *name = extra_refs->items[i].string;\n \t\tstruct object *object = extra_refs->items[i].util;\n+\t\tstruct tag *tag;\n \t\tswitch (object->type) {\n \t\tcase OBJ_TAG:\n-\t\t\thandle_tag(name, (struct tag *)object);\n+\t\t\ttag = (struct tag *)object;\n+\t\t\tif (tag->tagged->type == OBJ_TREE) {\n+\t\t\t\t/* Ignore this tag altogether */\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t\thandle_tag(name, tag);\n \t\t\tbreak;\n \t\tcase OBJ_COMMIT:\n \t\t\t/* create refs pointing to already seen commits */\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 8c8a9e6..3f13e6b 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -271,8 +271,14 @@ test_expect_success 'set-up a few more tags for tag export tests' '\n \tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" tree_tag-obj\n '\n \n+test_expect_success 'tree_tag' '\n+\tmkdir result &&\n+\t(cd result && git init) &&\n+\tgit fast-export tree_tag > fe-stream &&\n+\t(cd result && git fast-import < ../fe-stream)\n+'\n+\n # NEEDSWORK: not just check return status, but validate the output\n-test_expect_success 'tree_tag' 'git fast-export tree_tag'\n test_expect_success 'tree_tag-obj' 'git fast-export tree_tag-obj'\n test_expect_success 'tag-obj_tag' 'git fast-export tag-obj_tag'\n test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'\n-- \n1.6.0.6\n"},{"id":"116766","messageId":"1245676001-14734-3-git-send-email-newren@gmail.com","threadId":"19899","inReplyTo":"1245676001-14734-1-git-send-email-newren@gmail.com","subject":"[PATCH v2] fast-export: Make sure we show actual ref names instead of \"(null)\"","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-22T13:06:37Z","receivedAt":"2009-06-22T13:06:37Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nUnder some cases (e.g. 'git fast-export --parents master -- COPYING' run\nin git.git), the output included the lines\n reset (null)\n commit (null)\ninstead of\n reset refs/heads/master\n commit refs/heads/master\nThis simple change fixes that.\n\nSigned-off-by: Elijah Newren \n---\n(No change to this patch since the first series.)\nAs noted above, run 'git fast-export --parents master -- COPYING' in\ngit.git to see this bug triggered.\n\n builtin-fast-export.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex 891e2d4..9aa409b 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -531,6 +531,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tget_tags_and_duplicates(&revs.pending, &extra_refs);\n \n \trevs.topo_order = 1;\n+\trevs.show_source = 1;\n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\n \trevs.diffopt.format_callback = show_filemodify;\n-- \n1.6.0.6\n"},{"id":"116765","messageId":"1245676001-14734-4-git-send-email-newren@gmail.com","threadId":"19899","inReplyTo":"1245676001-14734-1-git-send-email-newren@gmail.com","subject":"[PATCH v2] fast-export: Do parent rewriting to avoid dropping relevant commits","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-22T13:06:38Z","receivedAt":"2009-06-22T13:06:38Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen specifying paths to export, parent rewriting must be turned on for\nfast-export to output anything at all.\n\nSigned-off-by: Elijah Newren \n---\n(No change to this patch since the first series.)\nTry 'git fast-export master -- COPYING' in git.git and notice how there\nis no output at all (whereas 'git rev-list master -- COPYING' does show\nsome existing revisions). This simple patch restores the missing file\nand commit in the fast-export output.\n\n builtin-fast-export.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex 9aa409b..b60a97e 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -532,6 +532,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \n \trevs.topo_order = 1;\n \trevs.show_source = 1;\n+\trevs.rewrite_parents = 1;\n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\n \trevs.diffopt.format_callback = show_filemodify;\n-- \n1.6.0.6\n"},{"id":"116767","messageId":"1245676001-14734-5-git-send-email-newren@gmail.com","threadId":"19899","inReplyTo":"1245676001-14734-1-git-send-email-newren@gmail.com","subject":"[PATCH v2] fast-export: Add a --tag-of-filtered-object option for newly dangling tags","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-22T13:06:39Z","receivedAt":"2009-06-22T13:06:39Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen providing a list of paths to limit what is exported, the object that\na tag points to can be filtered out entirely. This new switch allows\nthe user to specify what should happen to the tag in such a case. The\ndefault action, 'abort' will exit with an error message. With 'drop', the\ntag will simply be omitted from the output. With 'rewrite', if the object\ntagged was a commit, the tag will be modified to tag an ancestor of the\nremoved commit.\n\nSigned-off-by: Elijah Newren \n---\n(No change to this patch since the first series.)\nWhen providing a list of paths to limit what is exported, the object that\na tag points to can be filtered out entirely. This new switch allows\nthe user to specify what should happen to the tag in such a case. The\ndefault action, 'abort' will exit with an error message. With 'drop', the\ntag will simply be omitted from the output. With 'rewrite', if the object\ntagged was a commit, the tag will be modified to tag an ancestor of the\nremoved commit.\n\n Documentation/git-fast-export.txt | 11 ++++++\n builtin-fast-export.c | 64 +++++++++++++++++++++++++++++++++---\n revision.c | 10 +-----\n revision.h | 8 +++++\n 4 files changed, 79 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex 0c9eb56..194abde 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -36,6 +36,17 @@ when encountering a signed tag. With 'strip', the tags will be made\n unsigned, with 'verbatim', they will be silently exported\n and with 'warn', they will be exported, but you will see a warning.\n \n+--tag-of-filtered-object=(abort|drop|rewrite)::\n+\tSpecify how to handle tags whose tagged objectis filtered out.\n+\tSince revisions and files to export can be limited by path,\n+\ttagged objects may be filtered completely.\n++\n+When asking to 'abort' (which is the default), this program will die\n+when encountering such a tag. With 'drop' it will omit such tags from\n+the output. With 'rewrite', if the tagged object is a commit, it will\n+rewrite the tag to tag an ancestor commit (via parent rewriting; see\n+linkgit:git-rev-list[1])\n+\n -M::\n -C::\n \tPerform move and/or copy detection, as described in the\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex b60a97e..917ac05 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -23,7 +23,8 @@ static const char *fast_export_usage[] = {\n };\n \n static int progress;\n-static enum { VERBATIM, WARN, STRIP, ABORT } signed_tag_mode = ABORT;\n+static enum { ABORT, VERBATIM, WARN, STRIP } signed_tag_mode = ABORT;\n+static enum { ERROR, DROP, REWRITE } tag_of_filtered_mode = ABORT;\n static int fake_missing_tagger;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n@@ -42,6 +43,20 @@ static int parse_opt_signed_tag_mode(const struct option *opt,\n \treturn 0;\n }\n \n+static int parse_opt_tag_of_filtered_mode(const struct option *opt,\n+\t\t\t\t\t const char *arg, int unset)\n+{\n+\tif (unset || !strcmp(arg, \"abort\"))\n+\t\ttag_of_filtered_mode = ABORT;\n+\telse if (!strcmp(arg, \"drop\"))\n+\t\ttag_of_filtered_mode = DROP;\n+\telse if (!strcmp(arg, \"rewrite\"))\n+\t\ttag_of_filtered_mode = REWRITE;\n+\telse\n+\t\treturn error(\"Unknown tag-of-filtered mode: %s\", arg);\n+\treturn 0;\n+}\n+\n static struct decoration idnums;\n static uint32_t last_idnum;\n \n@@ -282,13 +297,14 @@ static void handle_tail(struct object_array *commits, struct rev_info *revs)\n \t}\n }\n \n-static void handle_tag(const char *name, struct tag *tag)\n+static void handle_tag(const char *name, struct tag *tag, struct rev_info *revs)\n {\n \tunsigned long size;\n \tenum object_type type;\n \tchar *buf;\n \tconst char *tagger, *tagger_end, *message;\n \tsize_t message_size = 0;\n+\tstruct commit *commit;\n \n \tbuf = read_sha1_file(tag->object.sha1, &type, &size);\n \tif (!buf)\n@@ -333,10 +349,42 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\t\t}\n \t}\n \n+\t/* handle tag->tagged having been filtered out due to paths specified */\n+\tstruct object * tagged = tag->tagged;\n+\tint tagged_mark = get_object_mark(tagged);\n+\tif (!tagged_mark) {\n+\t\tswitch(tag_of_filtered_mode) {\n+\t\tcase ABORT:\n+\t\t\tdie (\"Tag %s tags unexported commit; use \"\n+\t\t\t \"--tag-of-filtered-object= to handle it.\",\n+\t\t\t sha1_to_hex(tag->object.sha1));\n+\t\tcase DROP:\n+\t\t\t/* Ignore this tag altogether */\n+\t\t\treturn;\n+\t\t\t\t/* fallthru */\n+\t\tcase REWRITE:\n+\t\t\tif (tagged->type != OBJ_COMMIT) {\n+\t\t\t\tdie (\"Tag %s tags unexported commit; use \"\n+\t\t\t\t \"--tag-of-filtered-object= to handle it.\",\n+\t\t\t\t sha1_to_hex(tag->object.sha1));\n+\t\t\t}\n+\t\t\tcommit = (struct commit *)tagged;\n+\t\t\tswitch (rewrite_one_commit(revs, &commit)) {\n+\t\t\tcase rewrite_one_ok:\n+\t\t\t\ttagged_mark = get_object_mark(&commit->object);\n+\t\t\t\tbreak;\n+\t\t\tcase rewrite_one_noparents:\n+\t\t\tcase rewrite_one_error:\n+\t\t\t\tdie (\"Can't find replacement commit for tag %s\\n\",\n+\t\t\t\t sha1_to_hex(tag->object.sha1));\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \tif (!prefixcmp(name, \"refs/tags/\"))\n \t\tname += 10;\n \tprintf(\"tag %s\\nfrom :%d\\n%.*s%sdata %d\\n%.*s\\n\",\n-\t name, get_object_mark(tag->tagged),\n+\t name, tagged_mark,\n \t (int)(tagger_end - tagger), tagger,\n \t tagger == tagger_end ? \"\" : \"\\n\",\n \t (int)message_size, (int)message_size, message ? message : \"\");\n@@ -399,7 +447,8 @@ static void get_tags_and_duplicates(struct object_array *pending,\n \t}\n }\n \n-static void handle_tags_and_duplicates(struct string_list *extra_refs)\n+static void handle_tags_and_duplicates(struct string_list *extra_refs,\n+\t\t\t\t struct rev_info *revs)\n {\n \tstruct commit *commit;\n \tint i;\n@@ -415,7 +464,7 @@ static void handle_tags_and_duplicates(struct string_list *extra_refs)\n \t\t\t\t/* Ignore this tag altogether */\n \t\t\t\treturn;\n \t\t\t}\n-\t\t\thandle_tag(name, tag);\n+\t\t\thandle_tag(name, tag, revs);\n \t\t\tbreak;\n \t\tcase OBJ_COMMIT:\n \t\t\t/* create refs pointing to already seen commits */\n@@ -504,6 +553,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK(0, \"signed-tags\", &signed_tag_mode, \"mode\",\n \t\t\t \"select handling of signed tags\",\n \t\t\t parse_opt_signed_tag_mode),\n+\t\tOPT_CALLBACK(0, \"tag-of-filtered-object\", &tag_of_filtered_mode, \"mode\",\n+\t\t\t \"select handling of tags that tag filtered objects\",\n+\t\t\t parse_opt_tag_of_filtered_mode),\n \t\tOPT_STRING(0, \"export-marks\", &export_filename, \"FILE\",\n \t\t\t \"Dump marks to this file\"),\n \t\tOPT_STRING(0, \"import-marks\", &import_filename, \"FILE\",\n@@ -551,7 +603,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\thandle_tags_and_duplicates(&extra_refs);\n+\thandle_tags_and_duplicates(&extra_refs, &revs);\n \n \tif (export_filename)\n \t\texport_marks(export_filename);\ndiff --git a/revision.c b/revision.c\nindex bf58448..5ed2841 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1601,13 +1601,7 @@ int prepare_revision_walk(struct rev_info *revs)\n \treturn 0;\n }\n \n-enum rewrite_result {\n-\trewrite_one_ok,\n-\trewrite_one_noparents,\n-\trewrite_one_error,\n-};\n-\n-static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp)\n+enum rewrite_result rewrite_one_commit(struct rev_info *revs, struct commit **pp)\n {\n \tstruct commit_list *cache = NULL;\n \n@@ -1633,7 +1627,7 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)\n \tstruct commit_list **pp = &commit->parents;\n \twhile (*pp) {\n \t\tstruct commit_list *parent = *pp;\n-\t\tswitch (rewrite_one(revs, &parent->item)) {\n+\t\tswitch (rewrite_one_commit(revs, &parent->item)) {\n \t\tcase rewrite_one_ok:\n \t\t\tbreak;\n \t\tcase rewrite_one_noparents:\ndiff --git a/revision.h b/revision.h\nindex 227164c..6bf5b9e 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -158,6 +158,14 @@ extern void add_pending_object(struct rev_info *revs, struct object *obj, const\n \n extern void add_head_to_pending(struct rev_info *);\n \n+enum rewrite_result {\n+\trewrite_one_ok,\n+\trewrite_one_noparents,\n+\trewrite_one_error,\n+};\n+\n+extern enum rewrite_result rewrite_one_commit(struct rev_info *revs, struct commit **pp);\n+\n enum commit_action {\n \tcommit_ignore,\n \tcommit_show,\n-- \n1.6.0.6\n"},{"id":"116768","messageId":"1245676001-14734-6-git-send-email-newren@gmail.com","threadId":"19899","inReplyTo":"1245676001-14734-1-git-send-email-newren@gmail.com","subject":"[PATCH v2] Add new fast-export testcases","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-22T13:06:40Z","receivedAt":"2009-06-22T13:06:40Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThe testcases test the new --tag-of-filtered-object option, and test the\noutput when limiting what to export by path.\n\nSigned-off-by: Elijah Newren \n---\n(No change to this patch since the first series.)\n\n t/t9301-fast-export.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 55 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 3f13e6b..d03d599 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -262,6 +262,61 @@ test_expect_success 'cope with tagger-less tags' '\n \n '\n \n+test_expect_success 'setup for limiting exports by PATH' '\n+\tmkdir limit-by-paths &&\n+\tcd limit-by-paths &&\n+\tgit init &&\n+\techo hi > there &&\n+\tgit add there &&\n+\tgit commit -m \"First file\" &&\n+\techo foo > bar &&\n+\tgit add bar &&\n+\tgit commit -m \"Second file\" &&\n+\tgit tag -a -m msg mytag &&\n+\tcd ..\n+'\n+\n+cat > limit-by-paths/expected << EOF\n+blob\n+mark :1\n+data 3\n+hi\n+\n+reset refs/tags/mytag\n+commit refs/tags/mytag\n+mark :2\n+author A U Thor 1112912713 -0700\n+committer C O Mitter 1112912713 -0700\n+data 11\n+First file\n+M 100644 :1 there\n+\n+EOF\n+\n+test_expect_success 'dropping tag of filtered out object' '\n+\tcd limit-by-paths &&\n+\tgit fast-export --tag-of-filtered-object=drop mytag -- there > output &&\n+\ttest_cmp output expected &&\n+\tcd ..\n+'\n+\n+cat >> limit-by-paths/expected << EOF\n+tag mytag\n+from :2\n+tagger C O Mitter 1112912713 -0700\n+data 4\n+msg\n+\n+EOF\n+\n+test_expect_success 'rewriting tag of filtered out object' '\n+\tcd limit-by-paths &&\n+\tgit fast-export --tag-of-filtered-object=rewrite mytag -- there > output &&\n+\ttest_cmp output expected &&\n+\tcd ..\n+'\n+\n+\n test_expect_success 'set-up a few more tags for tag export tests' '\n \tgit checkout -f master &&\n \tHEAD_TREE=`git show -s --pretty=raw HEAD | grep tree | sed \"s/tree //\"` &&\n-- \n1.6.0.6\n"},{"id":"116769","messageId":"1245676001-14734-7-git-send-email-newren@gmail.com","threadId":"19899","inReplyTo":"1245676001-14734-1-git-send-email-newren@gmail.com","subject":"[PATCH v2] fast-export: Document the fact that git-rev-list arguments are accepted","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-22T13:06:41Z","receivedAt":"2009-06-22T13:06:41Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\n\nSigned-off-by: Elijah Newren \n---\n(No change to this patch since the first series.)\n\n Documentation/git-fast-export.txt | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex 194abde..af2328d 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -82,6 +82,12 @@ marks the same across runs.\n \tallow that. So fake a tagger to be able to fast-import the\n \toutput.\n \n+[git-rev-list-args...]::\n+ A list of arguments, acceptable to 'git-rev-parse' and\n+ 'git-rev-list', that specifies the specific objects and references\n+ to export. For example, `master\\~10..master` causes the\n+ current master reference to be exported along with all objects\n+ added since its 10th ancestor commit.\n \n EXAMPLES\n --------\n-- \n1.6.0.6\n"},{"id":"116782","messageId":"7vr5xcazji.fsf@alter.siamese.dyndns.org","threadId":"19899","inReplyTo":"1245676001-14734-2-git-send-email-newren@gmail.com","subject":"Re: [PATCH v2] fast-export: Omit tags that tag trees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-22T18:33:53Z","receivedAt":"2009-06-22T18:33:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"newren@gmail.com writes:\n\n> From: Elijah Newren \n>\n> Commit c0582c53bcf4e83bba70e1ad23abbad31f96ebc8 introduced logic to just\n> omit tags that point to tree objects. However, a case was missed\n> resulting in a tag being output which pointed at \"mark :0\", which would\n> cause fast-import to crash.\n>\n> Signed-off-by: Elijah Newren \n> ---\n> Includes changes suggested by Stephen Boyd (squashing commits together\n> and moving the NEEDSWORK comment to where it is still relevant) and\n> Johannes Sixt (fixing the testcase to remain in the right directory\n> even if git init fails and to catch failures in git fast-export).\n>\n> builtin-fast-export.c | 8 +++++++-\n> t/t9301-fast-export.sh | 8 +++++++-\n> 2 files changed, 14 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin-fast-export.c b/builtin-fast-export.c\n> index 6cef810..891e2d4 100644\n> --- a/builtin-fast-export.c\n> +++ b/builtin-fast-export.c\n> @@ -407,9 +407,15 @@ static void handle_tags_and_duplicates(struct string_list *extra_refs)\n> \tfor (i = extra_refs->nr - 1; i >= 0; i--) {\n> \t\tconst char *name = extra_refs->items[i].string;\n> \t\tstruct object *object = extra_refs->items[i].util;\n> +\t\tstruct tag *tag;\n> \t\tswitch (object->type) {\n> \t\tcase OBJ_TAG:\n> -\t\t\thandle_tag(name, (struct tag *)object);\n> +\t\t\ttag = (struct tag *)object;\n> +\t\t\tif (tag->tagged->type == OBJ_TREE) {\n> +\t\t\t\t/* Ignore this tag altogether */\n> +\t\t\t\treturn;\n> +\t\t\t}\n> +\t\t\thandle_tag(name, tag);\n> \t\t\tbreak;\n\nThis seems to be the only callsite of handle_tag(), and I wonder why the\nabove special case is not in the called function instead.\n\nIf a newer version of fast-import ever supports a tag that points at\nnon-commits, a patch to add the corresponding support on the export side\nlogically should go to handle_tag(), no?\n"},{"id":"116780","messageId":"7vk534azjb.fsf@alter.siamese.dyndns.org","threadId":"19899","inReplyTo":"1245676001-14734-3-git-send-email-newren@gmail.com","subject":"Re: [PATCH v2] fast-export: Make sure we show actual ref names instead of \"(null)\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-22T18:34:00Z","receivedAt":"2009-06-22T18:34:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"newren@gmail.com writes:\n\n> From: Elijah Newren \n>\n> Under some cases (e.g. 'git fast-export --parents master -- COPYING' run\n> in git.git), the output included the lines\n> reset (null)\n> commit (null)\n> instead of\n> reset refs/heads/master\n> commit refs/heads/master\n> This simple change fixes that.\n>\n> Signed-off-by: Elijah Newren \n> ---\n> (No change to this patch since the first series.)\n> As noted above, run 'git fast-export --parents master -- COPYING' in\n> git.git to see this bug triggered.\n\nIsn't this easier to read?\n\n> Under some cases (e.g. 'git fast-export --parents master -- COPYING' run\n> in git.git), the output included the lines\n>\n> reset (null)\n> commit (null)\n>\n> instead of\n>\n> reset refs/heads/master\n> commit refs/heads/master\n>\n> This simple change fixes that.\n\nThe last line does not add much value. I would have preferred to see\nsomething like this while reviewing, so that I did not have to dig that\ninformation out from the source myself:\n\n\tThis is because the code expects commit->util to point at the ref\n\tbut that is only set by the revision traversal machinery when\n\tshow_source is asked, in a way similar to \"--source\" option of\n\t\"git log\" family of commands.\n\nBut otherwise, good catch. Thanks.\n\n> builtin-fast-export.c | 1 +\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-fast-export.c b/builtin-fast-export.c\n> index 891e2d4..9aa409b 100644\n> --- a/builtin-fast-export.c\n> +++ b/builtin-fast-export.c\n> @@ -531,6 +531,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n> \tget_tags_and_duplicates(&revs.pending, &extra_refs);\n> \n> \trevs.topo_order = 1;\n> +\trevs.show_source = 1;\n> \tif (prepare_revision_walk(&revs))\n> \t\tdie(\"revision walk setup failed\");\n> \trevs.diffopt.format_callback = show_filemodify;\n> -- \n> 1.6.0.6\n"},{"id":"116781","messageId":"7vd48wazhu.fsf@alter.siamese.dyndns.org","threadId":"19899","inReplyTo":"1245676001-14734-5-git-send-email-newren@gmail.com","subject":"Re: [PATCH v2] fast-export: Add a --tag-of-filtered-object option for newly dangling tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-22T18:34:53Z","receivedAt":"2009-06-22T18:34:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"newren@gmail.com writes:\n\n> From: Elijah Newren \n>\n> When providing a list of paths to limit what is exported, the object that\n> a tag points to can be filtered out entirely. This new switch allows\n> the user to specify what should happen to the tag in such a case. The\n> default action, 'abort' will exit with an error message. With 'drop', the\n> tag will simply be omitted from the output. With 'rewrite', if the object\n> tagged was a commit, the tag will be modified to tag an ancestor of the\n> removed commit.\n\nI had to wonder what \"an ancestor\" means in the above sentence.\n\n - It cannot be the \"direct ancestor\", as such a commit could also have\n been filtered out. it will probably find an ancestor of the tagged\n commit that is not TREESAME, i.e. changes one of the specified paths.\n is that what the code tries to do?\n\n - Assuming that the answer is true, finding an ancestor means go back in\n the ancestry graph. How should a merged history be handled?\n\n - If two branches merged, only one among which touched the paths, then\n the simplication would have linearized the history already, so it is\n a non issue;\n\n - If a merge really had changes from both branches, that merge would\n remain in the output, and the tag will be moved there.\n\n - Did I miss any other case?\n\nIf I have to wonder, many other people who read the \"git log\" output later\nwill get puzzled, too. Please clarify.\n\n> @@ -333,10 +349,42 @@ static void handle_tag(const char *name, struct tag *tag)\n> \t\t\t}\n> \t}\n> \n> +\t/* handle tag->tagged having been filtered out due to paths specified */\n> +\tstruct object * tagged = tag->tagged;\n> +\tint tagged_mark = get_object_mark(tagged);\n> +\tif (!tagged_mark) {\n> +\t\tswitch(tag_of_filtered_mode) {\n> +\t\tcase ABORT:\n> +\t\t\tdie (\"Tag %s tags unexported commit; use \"\n> +\t\t\t \"--tag-of-filtered-object= to handle it.\",\n> +\t\t\t sha1_to_hex(tag->object.sha1));\n> +\t\tcase DROP:\n> +\t\t\t/* Ignore this tag altogether */\n> +\t\t\treturn;\n> +\t\t\t\t/* fallthru */\n\nDoesn't fall through...\n\n> +\t\tcase REWRITE:\n> +\t\t\tif (tagged->type != OBJ_COMMIT) {\n> +\t\t\t\tdie (\"Tag %s tags unexported commit; use \"\n> +\t\t\t\t \"--tag-of-filtered-object= to handle it.\",\n> +\t\t\t\t sha1_to_hex(tag->object.sha1));\n> +\t\t\t}\n> +\t\t\tcommit = (struct commit *)tagged;\n> +\t\t\tswitch (rewrite_one_commit(revs, &commit)) {\n> +\t\t\tcase rewrite_one_ok:\n> +\t\t\t\ttagged_mark = get_object_mark(&commit->object);\n> +\t\t\t\tbreak;\n> +\t\t\tcase rewrite_one_noparents:\n> +\t\t\tcase rewrite_one_error:\n> +\t\t\t\tdie (\"Can't find replacement commit for tag %s\\n\",\n> +\t\t\t\t sha1_to_hex(tag->object.sha1));\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n\nThis makes me a bit worried. The rewrite_one() function is never designed\nto be called from outside while the main traversal has still work to do,\nnor called more than once on the same commit object. I do not know what\nwould happen if somebody did so.\n\nGranted, all of this processing happens after the revision traversing\nmachinery is done with all the commits, and rewriting commits further here\nwill not have a chance of breaking the subtleties in get_revision() and\neverything called from it that already has happened in the main traversal,\nbut still I would prefer not exposing this function out of revision.c to\navoid mistakes if possible.\n\nAlso, are you absolutely sure that your revs is always limited at this\npoint? Otherwise, the parents of this commit are queued in rev->list,\nexpecting somebody else to later pick them up and further process, but\nthere is nobody who does that in your codepath as far as I can see. What\nwill happen to these parent commits?\n"},{"id":"116989","messageId":"51419b2c0906252145pe174d1diddee9ed81413af74@mail.gmail.com","threadId":"19899","inReplyTo":"7vd48wazhu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] fast-export: Add a --tag-of-filtered-object option for newly dangling tags","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2009-06-26T04:45:13Z","receivedAt":"2009-06-26T04:45:13Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nThanks for the detailed feedback and suggestions. I'll send a new\npatch series shortly, which I believe addresses all your suggestions\nand requests (and opens up a new issue), but there is one item in\nparticular I wanted to respond to...\n\nOn Mon, Jun 22, 2009 at 12:34 PM, Junio C Hamano wrote:\n> Also, are you absolutely sure that your revs is always limited at this\n> point?  Otherwise, the parents of this commit are queued in rev->list,\n> expecting somebody else to later pick them up and further process, but\n> there is nobody who does that in your codepath as far as I can see.  What\n> will happen to these parent commits?\n\nrevs was not limited, but should have been. Inside setup_revisions,\nrevs->limited is set if revs->topo_order is, but unfortunately we\nweren't setting revs->topo_order until after the call to\nsetup_revisions. So, as you suggest, my code caused the parents of\nthe rewritten commit to needlessly get queued up and left around, with\nno one to process them. Oops.\n\n\nElijah\n"}]}