{"thread":{"id":"18471","subject":"[PATCH 1/4] test-suite: adding a test for fast-export with tag variants","startedAt":"2009-03-22T21:50:17Z","lastAt":"2009-03-23T03:41:26Z","messageCount":9,"participants":["Erik Faye-Lund","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"108929","messageId":"1237758620-6116-1-git-send-email-kusmabite@gmail.com","threadId":"18471","inReplyTo":null,"subject":"[PATCH 1/4] test-suite: adding a test for fast-export with tag variants","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2009-03-22T21:50:17Z","receivedAt":"2009-03-22T21:50:17Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Signed-off-by: Erik Faye-Lund \n---\n t/t9302-fast-export-tags.sh | 25 +++++++++++++++++++++++++\n 1 files changed, 25 insertions(+), 0 deletions(-)\n create mode 100644 t/t9302-fast-export-tags.sh\n\ndiff --git a/t/t9302-fast-export-tags.sh b/t/t9302-fast-export-tags.sh\nnew file mode 100644\nindex 0000000..2ecac32\n--- /dev/null\n+++ b/t/t9302-fast-export-tags.sh\n@@ -0,0 +1,25 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2009 Erik Faye-Lund\n+#\n+\n+test_description='git fast-export tag variants'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttouch dummy &&\n+\tgit add dummy &&\n+\tgit commit -m \"initial commit\" &&\n+\tHEAD_TREE=`git show -s --pretty=raw HEAD | grep tree | sed \"s/tree //\"` &&\n+\tgit tag tree_tag -m \"tagging a tree\" $HEAD_TREE &&\n+\tgit tag -a tree_tag-obj -m \"tagging a tree\" $HEAD_TREE &&\n+\tgit tag tag-obj_tag -m \"tagging a tag\" tree_tag-obj &&\n+\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" tree_tag-obj\n+'\n+\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+\n+test_done\n-- \n1.6.2.1.226.gcb2dd\n"},{"id":"108930","messageId":"1237758620-6116-2-git-send-email-kusmabite@gmail.com","threadId":"18471","inReplyTo":"1237758620-6116-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH 2/4] builtin-fast-export.c: turn error into warning","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2009-03-22T21:50:18Z","receivedAt":"2009-03-22T21:50:18Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"fast-import doesn't have a syntax to support tree-objects (and some other\nobject-types), so fast-export shouldn't handle them. However, aborting the\noperation is a bit drastic. This patch turns the error into a warning instead.\n\nSigned-off-by: Erik Faye-Lund \n---\n builtin-fast-export.c | 6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex fdf4ae9..02bad1f 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -378,8 +378,10 @@ static void get_tags_and_duplicates(struct object_array *pending,\n \t\t\t}\n \t\t\tbreak;\n \t\tdefault:\n-\t\t\tdie (\"Unexpected object of type %s\",\n-\t\t\t typename(e->item->type));\n+\t\t\twarning(\"%s: Unexpected object of type %s, skipping.\",\n+\t\t\t e->name,\n+\t\t\t typename(e->item->type));\n+\t\t\tcontinue;\n \t\t}\n \t\tif (commit->util)\n \t\t\t/* more than one name for the same object */\n-- \n1.6.2.1.226.gcb2dd\n"},{"id":"108932","messageId":"1237758620-6116-3-git-send-email-kusmabite@gmail.com","threadId":"18471","inReplyTo":"1237758620-6116-2-git-send-email-kusmabite@gmail.com","subject":"[PATCH 3/4] builtin-fast-export.c: fix crash on tagged trees","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2009-03-22T21:50:19Z","receivedAt":"2009-03-22T21:50:19Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"If a tag object points to a tree (or another unhandled type), the commit-\npointer is left uninitialized and later dereferenced. This patch adds a default\ncase to the switch that issues a warning and skips the object.\n\nSigned-off-by: Erik Faye-Lund \n---\n builtin-fast-export.c | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex 02bad1f..c3ce320 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -375,6 +375,10 @@ static void get_tags_and_duplicates(struct object_array *pending,\n \t\t\tcase OBJ_BLOB:\n \t\t\t\thandle_object(tag->object.sha1);\n \t\t\t\tcontinue;\n+\t\t\tdefault: /* OBJ_TAG (nested tags) is already handled */\n+\t\t\t\twarning(\"Tag points to object of unexpected type %s, skipping.\",\n+\t\t\t\t typename(tag->object.type));\n+\t\t\t\tcontinue;\n \t\t\t}\n \t\t\tbreak;\n \t\tdefault:\n-- \n1.6.2.1.226.gcb2dd\n"},{"id":"108931","messageId":"1237758620-6116-4-git-send-email-kusmabite@gmail.com","threadId":"18471","inReplyTo":"1237758620-6116-3-git-send-email-kusmabite@gmail.com","subject":"[PATCH 4/4] builtin-fast-export.c: handle nested tags","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2009-03-22T21:50:20Z","receivedAt":"2009-03-22T21:50:20Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"When tags that points to tags are passed to fast-export, an error saying\n\"Tag [TAGNAME] points nowhere?\". This fix calls parse_object() on the object\nbefore referencing it's tag, to ensure the tag-info is fully initialized. In\naddition, it inserts a comment to point out where nested tags are handled. This\nis consistent with the comment for signed tags.\n\nSigned-off-by: Erik Faye-Lund \n---\n builtin-fast-export.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex c3ce320..8083c5f 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -362,7 +362,10 @@ static void get_tags_and_duplicates(struct object_array *pending,\n \t\t\tbreak;\n \t\tcase OBJ_TAG:\n \t\t\ttag = (struct tag *)e->item;\n+\n+\t\t\t/* handle nested tags */\n \t\t\twhile (tag && tag->object.type == OBJ_TAG) {\n+\t\t\t\tparse_object(tag->object.sha1);\n \t\t\t\tstring_list_append(full_name, extra_refs)->util = tag;\n \t\t\t\ttag = (struct tag *)tag->tagged;\n \t\t\t}\n-- \n1.6.2.1.226.gcb2dd\n"},{"id":"108972","messageId":"7vfxh59in9.fsf@gitster.siamese.dyndns.org","threadId":"18471","inReplyTo":"1237758620-6116-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH 1/4] test-suite: adding a test for fast-export with tag variants","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-23T00:39:54Z","receivedAt":"2009-03-23T00:39:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund writes:\n\n> Signed-off-by: Erik Faye-Lund \n> ---\n> t/t9302-fast-export-tags.sh | 25 +++++++++++++++++++++++++\n> 1 files changed, 25 insertions(+), 0 deletions(-)\n> create mode 100644 t/t9302-fast-export-tags.sh\n>\n> diff --git a/t/t9302-fast-export-tags.sh b/t/t9302-fast-export-tags.sh\n> new file mode 100644\n\nMake it executable if you need to add a new script, but shouldn't these\nsmall tests be done as an addition to existing t9301, not as a brand new\nscript?\n\n> index 0000000..2ecac32\n> --- /dev/null\n> +++ b/t/t9302-fast-export-tags.sh\n> @@ -0,0 +1,25 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2009 Erik Faye-Lund\n> +#\n> +\n> +test_description='git fast-export tag variants'\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> +\ttouch dummy &&\n> +\tgit add dummy &&\n> +\tgit commit -m \"initial commit\" &&\n> +\tHEAD_TREE=`git show -s --pretty=raw HEAD | grep tree | sed \"s/tree //\"` &&\n> +\tgit tag tree_tag -m \"tagging a tree\" $HEAD_TREE &&\n> +\tgit tag -a tree_tag-obj -m \"tagging a tree\" $HEAD_TREE &&\n> +\tgit tag tag-obj_tag -m \"tagging a tag\" tree_tag-obj &&\n> +\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" tree_tag-obj\n> +'\n> +\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> +\n> +test_done\n\nThe purpose of the first patch that adds tests is to expose existing\nproblems, and it is better to say test_expect_failure in them. Later\npatch to fix these issues will contain code change and also change to flip\nsome of the expect_failure to expect_success, and that way we can see what\nissue is fixed with which patch more easily.\n\nThese tests seem to only care about fast-export not dying, but don't we\nalso want to check if they produce correct results?\n"},{"id":"108971","messageId":"7vab7d9in4.fsf@gitster.siamese.dyndns.org","threadId":"18471","inReplyTo":"1237758620-6116-3-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH 3/4] builtin-fast-export.c: fix crash on tagged trees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-23T00:39:59Z","receivedAt":"2009-03-23T00:39:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund writes:\n\n> If a tag object points to a tree (or another unhandled type), the commit-\n> pointer is left uninitialized and later dereferenced. This patch adds a default\n> case to the switch that issues a warning and skips the object.\n>\n> Signed-off-by: Erik Faye-Lund \n> ---\n> builtin-fast-export.c | 4 ++++\n> 1 files changed, 4 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-fast-export.c b/builtin-fast-export.c\n> index 02bad1f..c3ce320 100644\n> --- a/builtin-fast-export.c\n> +++ b/builtin-fast-export.c\n> @@ -375,6 +375,10 @@ static void get_tags_and_duplicates(struct object_array *pending,\n> \t\t\tcase OBJ_BLOB:\n> \t\t\t\thandle_object(tag->object.sha1);\n> \t\t\t\tcontinue;\n> +\t\t\tdefault: /* OBJ_TAG (nested tags) is already handled */\n\nThe comment is good but only if you make this the last patch that comes\nafter the one that actually fixes the tag-to-tag (currently 4/4), no?\n\n> +\t\t\t\twarning(\"Tag points to object of unexpected type %s, skipping.\",\n> +\t\t\t\t typename(tag->object.type));\n> +\t\t\t\tcontinue;\n> \t\t\t}\n> \t\t\tbreak;\n> \t\tdefault:\n> -- \n> 1.6.2.1.226.gcb2dd\n"},{"id":"108973","messageId":"40aa078e0903221755n23457ba6m689aad34c0ec8214@mail.gmail.com","threadId":"18471","inReplyTo":"7vfxh59in9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/4] test-suite: adding a test for fast-export with tag variants","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-03-23T00:55:15Z","receivedAt":"2009-03-23T00:55:15Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Mar 23, 2009 at 1:39 AM, Junio C Hamano wrote:\n>> diff --git a/t/t9302-fast-export-tags.sh b/t/t9302-fast-export-tags.sh\n>> new file mode 100644\n>\n> Make it executable if you need to add a new script, but shouldn't these\n> small tests be done as an addition to existing t9301, not as a brand new\n> script?\n\nSure, will do. I just have to figure out how to make it executable in windows ;)\n\nAs this is my first test, I was wondering a bit about how much I could\ndepend on the state of the repo, so I was a bit reluctant to add it to\nthe same test. But I guess it's easier for everyone in the long term\nto put it into the same test, so I'll give it a go.\n\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>> +\n>> +test_done\n>\n> The purpose of the first patch that adds tests is to expose existing\n> problems, and it is better to say test_expect_failure in them.  Later\n> patch to fix these issues will contain code change and also change to flip\n> some of the expect_failure to expect_success, and that way we can see what\n> issue is fixed with which patch more easily.\n\nSure, will do. I was a bit lazy, but updating it as we go will make\nstuff easier in the future.\n\n> These tests seem to only care about fast-export not dying, but don't we\n> also want to check if they produce correct results?\n\nWell, yeah. But I was working mainly on fixing a crash-bug here, and I\ndon't think I know enough about the correct output of fast-export to\npull this off. Perhaps tighting up the test is something someone else\nwould care to do?\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"108978","messageId":"40aa078e0903221801o1ffd9fa2l3f7939d702440c3a@mail.gmail.com","threadId":"18471","inReplyTo":"7vab7d9in4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/4] builtin-fast-export.c: fix crash on tagged trees","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-03-23T01:01:03Z","receivedAt":"2009-03-23T01:01:03Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"> The comment is good but only if you make this the last patch that comes\n> after the one that actually fixes the tag-to-tag (currently 4/4), no?\n\nAh, good catch! I'll reorder the commits. The order of the last two\npatches isn't really important, so if it makes the comment more\nsenseful, I'm all for it ;)\n\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"108996","messageId":"7vy6uw9a8p.fsf@gitster.siamese.dyndns.org","threadId":"18471","inReplyTo":"40aa078e0903221755n23457ba6m689aad34c0ec8214@mail.gmail.com","subject":"Re: [PATCH 1/4] test-suite: adding a test for fast-export with tag variants","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-23T03:41:26Z","receivedAt":"2009-03-23T03:41:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund writes:\n\n> On Mon, Mar 23, 2009 at 1:39 AM, Junio C Hamano wrote:\n> ...\n>> These tests seem to only care about fast-export not dying, but don't we\n>> also want to check if they produce correct results?\n>\n> Well, yeah. But I was working mainly on fixing a crash-bug here, and I\n> don't think I know enough about the correct output of fast-export to\n> pull this off. Perhaps tighting up the test is something someone else\n> would care to do?\n\nSurely; for a starter, saying something like this:\n\n+# NEEDSWORK: not just check return status, but validate the output\n+test_expect_failure 'tree_tag'        'git fast-export tree_tag'\n+test_expect_failure 'tree_tag-obj'    'git fast-export tree_tag-obj'\n+test_expect_failure 'tag-obj_tag'     'git fast-export tag-obj_tag'\n+test_expect_failure 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj'\n\nwould do, and will invite others to fill the gap when they have time.\n\nThanks.\n"}]}