{"thread":{"id":"19871","subject":"A few fast-export fixups","startedAt":"2009-06-20T04:36:56Z","lastAt":"2009-06-22T13:12:03Z","messageCount":14,"participants":["newren@gmail.com","Jeff King","Elijah Newren","Stephen Boyd","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"116652","messageId":"1245472623-28103-1-git-send-email-newren@gmail.com","threadId":"19871","inReplyTo":null,"subject":"A few fast-export fixups","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-20T04:36:56Z","receivedAt":"2009-06-20T04:36:56Z","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"},{"id":"116658","messageId":"1245472623-28103-2-git-send-email-newren@gmail.com","threadId":"19871","inReplyTo":"1245472623-28103-1-git-send-email-newren@gmail.com","subject":"[PATCH 1/7] fast-export: Omit tags that tag trees","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-20T04:36:57Z","receivedAt":"2009-06-20T04:36:57Z","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---\n builtin-fast-export.c | 8 +++++++-\n 1 files changed, 7 insertions(+), 1 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 */\n-- \n1.6.3.2.323.gfb84f\n"},{"id":"116657","messageId":"1245472623-28103-3-git-send-email-newren@gmail.com","threadId":"19871","inReplyTo":"1245472623-28103-1-git-send-email-newren@gmail.com","subject":"[PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-20T04:36:58Z","receivedAt":"2009-06-20T04:36:58Z","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---\nShould I have just squashed this with the previous patch? Or with the\nother testcase patch?\n\n t/t9301-fast-export.sh | 8 +++++++-\n 1 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 8c8a9e6..d17f0e4 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -272,7 +272,13 @@ test_expect_success 'set-up a few more tags for tag export tests' '\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' '\n+\tmkdir result &&\n+\tcd result &&\n+\tgit init &&\n+\tcd ..\n+\tgit fast-export tree_tag | (cd result && git fast-import)\n+'\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.3.2.323.gfb84f\n"},{"id":"116656","messageId":"1245472623-28103-4-git-send-email-newren@gmail.com","threadId":"19871","inReplyTo":"1245472623-28103-1-git-send-email-newren@gmail.com","subject":"[PATCH 3/7] fast-export: Make sure we show actual ref names instead of \"(null)\"","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-20T04:36:59Z","receivedAt":"2009-06-20T04:36:59Z","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---\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.3.2.323.gfb84f\n"},{"id":"116655","messageId":"1245472623-28103-5-git-send-email-newren@gmail.com","threadId":"19871","inReplyTo":"1245472623-28103-1-git-send-email-newren@gmail.com","subject":"[PATCH 4/7] fast-export: Do parent rewriting to avoid dropping relevant commits","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-20T04:37:00Z","receivedAt":"2009-06-20T04:37:00Z","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---\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.3.2.323.gfb84f\n"},{"id":"116653","messageId":"1245472623-28103-6-git-send-email-newren@gmail.com","threadId":"19871","inReplyTo":"1245472623-28103-1-git-send-email-newren@gmail.com","subject":"[PATCH 5/7] fast-export: Add a --tag-of-filtered-object option for newly dangling tags","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-20T04:37:01Z","receivedAt":"2009-06-20T04:37:01Z","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---\nI had a hard time coming up with a decent option name. If anyone has a\nbetter suggestion for the name, I'm all ears.\n\nTo see a case where this bug is triggered, run\n git fast-export --signed-tag=strip v1.6.3.2 -- COPYING\nin git.git and note that the tag that is output points to 'mark :0' (a\nnon-existent mark), which will cause fast-import to crash.\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.3.2.323.gfb84f\n"},{"id":"116654","messageId":"1245472623-28103-7-git-send-email-newren@gmail.com","threadId":"19871","inReplyTo":"1245472623-28103-1-git-send-email-newren@gmail.com","subject":"[PATCH 6/7] Add new fast-export testcases","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-20T04:37:02Z","receivedAt":"2009-06-20T04:37:02Z","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 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 d17f0e4..8b165ff 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.3.2.323.gfb84f\n"},{"id":"116659","messageId":"1245472623-28103-8-git-send-email-newren@gmail.com","threadId":"19871","inReplyTo":"1245472623-28103-1-git-send-email-newren@gmail.com","subject":"[PATCH 7/7] fast-export: Document the fact that git-rev-list arguments are accepted","fromName":"","fromEmail":"newren@gmail.com","sentAt":"2009-06-20T04:37:03Z","receivedAt":"2009-06-20T04:37:03Z","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 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.3.2.323.gfb84f\n"},{"id":"116675","messageId":"20090620173128.GE31022@coredump.intra.peff.net","threadId":"19871","inReplyTo":"1245472623-28103-2-git-send-email-newren@gmail.com","subject":"Re: [PATCH 1/7] fast-export: Omit tags that tag trees","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-20T17:31:28Z","receivedAt":"2009-06-20T17:31:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 19, 2009 at 10:36:57PM -0600, newren@gmail.com wrote:\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\nDo we really want to disallow tags pointing to trees? There is at least\none well-known case in use (kernel v2.6.11).\n\nAlso, (and I haven't investigated at all), this sounds like the same\nissue we have with tags pointing to tags. IOW, everything referenceable\nshould be given a mark, but it is not currently. I posted a \"how about\nthis\" patch for the tag case here:\n\n http://article.gmane.org/gmane.comp.version-control.git/119245\n\nbut I never got around to following it up with tests. Could you do\nsomething similar for the tree case?\n\n-Peff\n"},{"id":"116676","messageId":"51419b2c0906201101u2f8e6981q84ab7acc13dc9135@mail.gmail.com","threadId":"19871","inReplyTo":"20090620173128.GE31022@coredump.intra.peff.net","subject":"Re: [PATCH 1/7] fast-export: Omit tags that tag trees","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2009-06-20T18:01:00Z","receivedAt":"2009-06-20T18:01:00Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Sat, Jun 20, 2009 at 11:31 AM, Jeff King wrote:\n> Do we really want to disallow tags pointing to trees? There is at least\n> one well-known case in use (kernel v2.6.11).\n>\n> Also, (and I haven't investigated at all), this sounds like the same\n> issue we have with tags pointing to tags. IOW, everything referenceable\n> should be given a mark, but it is not currently. I posted a \"how about\n> this\" patch for the tag case here:\n>\n>  http://article.gmane.org/gmane.comp.version-control.git/119245\n>\n> but I never got around to following it up with tests. Could you do\n> something similar for the tree case?\n\nI don't see how we could do something similar for the tree case\nwithout making some significant change to the output/input of both\nfast-export and fast-import. Tag objects are part of the output of\nfast-export, thus we can add a mark line to give the object a name and\nthus provide us a valid mark we can make the outer tag point to.\nTrees are not output by fast-export (other than implicitly by\nincluding files in commits), so we have nothing to point such a tag\nat. If we were to do something like use the full sha1sum instead of\nmarks in such a case (which git-fast-import should currently accept),\nthen (a) we'd break exporting a repository limited by path ('git\nfast-export master -- libfoo\"), and (b) we'd break interoperation with\nother tools like bzr-fast-import (or a possible future git that uses a\ndifferent checksum).\n\nIf someone wanted to tackle modifying the output/input syntax of\nfast-export and fast-import, maybe something could be done here, but\nmy patch just tries to make things operate sanely within the given\ndocumented limitations (as found in the git-fast-export manpage --\n\"Since 'git-fast-import' cannot tag trees, you will not be able to\nexport the linux-2.6.git repository completely, as it contains a tag\nreferencing a tree instead of a commit.\").\n\n\nElijah\n"},{"id":"116678","messageId":"20090620185236.GA8847@coredump.intra.peff.net","threadId":"19871","inReplyTo":"51419b2c0906201101u2f8e6981q84ab7acc13dc9135@mail.gmail.com","subject":"Re: [PATCH 1/7] fast-export: Omit tags that tag trees","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-20T18:52:37Z","receivedAt":"2009-06-20T18:52:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 20, 2009 at 12:01:00PM -0600, Elijah Newren wrote:\n\n> I don't see how we could do something similar for the tree case\n> without making some significant change to the output/input of both\n> fast-export and fast-import. Tag objects are part of the output of\n> fast-export, thus we can add a mark line to give the object a name and\n> thus provide us a valid mark we can make the outer tag point to.\n> Trees are not output by fast-export (other than implicitly by\n> including files in commits), so we have nothing to point such a tag\n> at. If we were to do something like use the full sha1sum instead of\n\nAh, right. That falls under the \"I didn't look at this too closely\" part\nof my previous message. :)\n\nIt does seem a shame not to be able to say \"and now here is a tree\" in\nthe same way we say \"and now here is a commit\". And since that is not\npossible without add-ons that might cause compatibility issues, your fix\nseems reasonable to at least avoid generating bogus output.\n\nAnother option would be to generate a pseudo-commit with no parentage\nthat holds the tree, and then point the tag at that. It's obviously not\nexactly what the user wanted, but neither is simply omitting. You could\neven mark the commit message with a \"THIS IS REALLY A TREE\" magic\nstring, and then have fast-import recognize and convert those back to a\ntree (by just peeling the commit away).\n\n-Peff\n"},{"id":"116693","messageId":"4A3DCAD7.4030701@gmail.com","threadId":"19871","inReplyTo":"1245472623-28103-3-git-send-email-newren@gmail.com","subject":"Re: [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-06-21T05:53:27Z","receivedAt":"2009-06-21T05:53:27Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"newren@gmail.com wrote:\n> Should I have just squashed this with the previous patch? Or with the\n> other testcase patch?\n\nI think squashing it with the previous one is better as long as this is\ndirectly related to patch 1.\n\n>\n> t/t9301-fast-export.sh | 8 +++++++-\n> 1 files changed, 7 insertions(+), 1 deletions(-)\n>\n> diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\n> index 8c8a9e6..d17f0e4 100755\n> --- a/t/t9301-fast-export.sh\n> +++ b/t/t9301-fast-export.sh\n> @@ -272,7 +272,13 @@ test_expect_success 'set-up a few more tags for tag export tests' '\n> '\n> \n> # NEEDSWORK: not just check return status, but validate the output\n\nIs this comment still relevant?\n\n> -test_expect_success 'tree_tag' 'git fast-export tree_tag'\n> +test_expect_success 'tree_tag' '\n> +\tmkdir result &&\n> +\tcd result &&\n> +\tgit init &&\n> +\tcd ..\n> +\tgit fast-export tree_tag | (cd result && git fast-import)\n> +'\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\nI don't really know, but could test_create_repo() be useful here?\n"},{"id":"116694","messageId":"200906210817.51520.j6t@kdbg.org","threadId":"19871","inReplyTo":"1245472623-28103-3-git-send-email-newren@gmail.com","subject":"Re: [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-06-21T06:17:51Z","receivedAt":"2009-06-21T06:17:51Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 20. Juni 2009, newren@gmail.com wrote:\n> -test_expect_success 'tree_tag' 'git fast-export tree_tag'\n> +test_expect_success 'tree_tag' '\n> +\tmkdir result &&\n> +\tcd result &&\n> +\tgit init &&\n> +\tcd ..\n> +\tgit fast-export tree_tag | (cd result && git fast-import)\n> +'\n\nYou should make this\n\ntest_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\nin order to (1) not remain in the wrong directory if git init fails, and (2) \nto catch failures in git fast-export.\n\n-- Hannes\n"},{"id":"116770","messageId":"51419b2c0906220612l5ce0ee4fs6466bb13b001872f@mail.gmail.com","threadId":"19871","inReplyTo":"200906210817.51520.j6t@kdbg.org","subject":"Re: [PATCH 2/7] Modify fast-export testcase to check that we correctly omit tags of trees","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2009-06-22T13:12:03Z","receivedAt":"2009-06-22T13:12:03Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sun, Jun 21, 2009 at 12:17 AM, Johannes Sixt wrote:\n> On Samstag, 20. Juni 2009, newren@gmail.com wrote:\n>> -test_expect_success 'tree_tag'        'git fast-export tree_tag'\n>> +test_expect_success 'tree_tag'        '\n>> +     mkdir result &&\n>> +     cd result &&\n>> +     git init &&\n>> +     cd ..\n>> +     git fast-export tree_tag | (cd result && git fast-import)\n>> +'\n>\n> You should make this\n>\n> test_expect_success 'tree_tag' '\n>        mkdir result &&\n>        (cd result && git init) &&\n>        git fast-export tree_tag > fe-stream &&\n>        (cd result && git fast-import < fe-stream)\n> '\n>\n> in order to (1) not remain in the wrong directory if git init fails, and (2)\n> to catch failures in git fast-export.\n\nThanks for the tips; I've sent a new patch series with these changes\n(except that the second fe-stream needs a ../ in front of it), as well\nas the changes suggested by Stephen Boyd in the other email in this\nthread.\n\nElijah\n"}]}