{"thread":{"id":"16784","subject":"git-fast-export and tags without a tagger","startedAt":"2008-12-18T16:46:14Z","lastAt":"2008-12-20T00:59:39Z","messageCount":11,"participants":["Miklos Vajna","Junio C Hamano","Johannes Schindelin","Shawn O. Pearce"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"98262","messageId":"20081218164614.GS5691@genesis.frugalware.org","threadId":"16784","inReplyTo":null,"subject":"git-fast-export and tags without a tagger","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-12-18T16:46:14Z","receivedAt":"2008-12-18T16:46:14Z","isPatch":false,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Hi,\n\nTags created with ancient versions of git have no tagger. The udev repo\nhas such tags, for example:\n\n$ git cat-file tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce\nobject face198a5f21027fefe796dc01e19e349a2d36ce\ntype commit\ntag 062\n\nfast-export will fail on these repos. From IRC:\n\n20:05 < Keybuk> vmiklos: exactly the same error with 1.6.0.5\n20:07 < Keybuk> $ git fast-export --signed-tag=strip --all\n20:07 < Keybuk> fatal: No tagger for tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce\n\nI think it would be nice to handle the situation better than just die().\n\nWhat about a --force option that would fake the tagger in case the tag\npoints to a commmit and use the commiter from there?\n\nI'm willing to work on this, but I thought it's better to discuss the\nright solution for the problem first.\n\nThanks.\n"},{"id":"98273","messageId":"7vbpv9guqd.fsf@gitster.siamese.dyndns.org","threadId":"16784","inReplyTo":"20081218164614.GS5691@genesis.frugalware.org","subject":"Re: git-fast-export and tags without a tagger","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-18T19:15:54Z","receivedAt":"2008-12-18T19:15:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna writes:\n\n> Tags created with ancient versions of git have no tagger. The udev repo\n> has such tags, for example:\n>\n> $ git cat-file tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce\n> object face198a5f21027fefe796dc01e19e349a2d36ce\n> type commit\n> tag 062\n>\n> fast-export will fail on these repos. From IRC:\n\nIs \"fast-export\" the only thing that chokes on these tags?\n\nWhat I am worried about is if we have accidentally/stupidly/by mistake\nmade other codepaths that check the validity of a tag object too strict,\nwhich would result things like \"git show $such_a_tag\" to barf.\n"},{"id":"98276","messageId":"alpine.DEB.1.00.0812182044100.6952@intel-tinevez-2-302","threadId":"16784","inReplyTo":"7vbpv9guqd.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] fast-export: deal with tag objects that do not have a tagger","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-12-18T19:45:44Z","receivedAt":"2008-12-18T19:45:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen no tagger was found (old Git produced tags like this), a\ntagger \"No Tagger \" with a tag date of the beginning of\n(Unix) time is output, so that fast-import is\nstill able to import the result.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Thu, 18 Dec 2008, Junio C Hamano wrote:\n\n\t> Miklos Vajna writes:\n\t> \n\t> > Tags created with ancient versions of git have no tagger. The \n\t> > udev repo has such tags, for example:\n\t> >\n\t> > $ git cat-file tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce\n\t> > object face198a5f21027fefe796dc01e19e349a2d36ce\n\t> > type commit\n\t> > tag 062\n\t> >\n\t> > fast-export will fail on these repos. From IRC:\n\t> \n\t> Is \"fast-export\" the only thing that chokes on these tags?\n\n\tI think so. The responsible code is in fast-export.c, in any \n\tcase. Of course, fast-import refuses to import a tag without a \n\ttagger, so...\n\n builtin-fast-export.c | 12 ++++++++----\n t/t9301-fast-export.sh | 15 +++++++++++++++\n 2 files changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex 7d5d57a..0af4e6f 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -297,10 +297,14 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\tmessage_size = strlen(message);\n \t}\n \ttagger = memmem(buf, message ? message - buf : size, \"\\ntagger \", 8);\n-\tif (!tagger)\n-\t\tdie (\"No tagger for tag %s\", sha1_to_hex(tag->object.sha1));\n-\ttagger++;\n-\ttagger_end = strchrnul(tagger, '\\n');\n+\tif (!tagger) {\n+\t\twarning (\"No tagger for tag %s\", sha1_to_hex(tag->object.sha1));\n+\t\ttagger = \"tagger No Tagger 0 +0000\";\n+\t\ttagger_end = tagger + strlen(tagger);\n+\t} else {\n+\t\ttagger++;\n+\t\ttagger_end = strchrnul(tagger, '\\n');\n+\t}\n \n \t/* handle signed tags */\n \tif (message) {\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 2057435..2312d7a 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -239,4 +239,19 @@ test_expect_success 'fast-export | fast-import when master is tagged' '\n \n '\n \n+cat > tag-content << EOF\n+object $(git rev-parse HEAD)\n+type commit\n+tag rosten\n+EOF\n+\n+test_expect_success 'cope with tagger-less tags' '\n+\n+\tTAG=$(git hash-object -t tag -w tag-content) &&\n+\tgit update-ref refs/tags/sonnenschein $TAG &&\n+\tgit fast-export -C -C --signed-tags=strip --all > output &&\n+\ttest $(grep -c \"^tag \" output) = 4\n+\n+'\n+\n test_done\n-- \n1.6.1.rc3.368.g63acc\n"},{"id":"98287","messageId":"20081218213407.GX5691@genesis.frugalware.org","threadId":"16784","inReplyTo":"alpine.DEB.1.00.0812182044100.6952@intel-tinevez-2-302","subject":"Re: [PATCH] fast-export: deal with tag objects that do not have a tagger","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-12-18T21:34:07Z","receivedAt":"2008-12-18T21:34:07Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Thu, Dec 18, 2008 at 08:45:44PM +0100, Johannes Schindelin wrote:\n> \tI think so. The responsible code is in fast-export.c, in any \n> \tcase. Of course, fast-import refuses to import a tag without a \n> \ttagger, so...\n\nThat's why I asked. I think it's a reasonable assumption that in most\ncases the tagger and the committer of the tagged commit is the same. So\nin case the tagger info is missing and we tag a commit, we could fake\nthat info on export.\n\nObviously this should not be the default, but I think such a mode would\nbe useful in real-life.\n"},{"id":"98288","messageId":"20081218213609.GY5691@genesis.frugalware.org","threadId":"16784","inReplyTo":"7vbpv9guqd.fsf@gitster.siamese.dyndns.org","subject":"Re: git-fast-export and tags without a tagger","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-12-18T21:36:09Z","receivedAt":"2008-12-18T21:36:09Z","isPatch":false,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Thu, Dec 18, 2008 at 11:15:54AM -0800, Junio C Hamano wrote:\n> Is \"fast-export\" the only thing that chokes on these tags?\n> \n> What I am worried about is if we have accidentally/stupidly/by mistake\n> made other codepaths that check the validity of a tag object too strict,\n> which would result things like \"git show $such_a_tag\" to barf.\n\ngit show works fine on the tag. git fsck --full passes without any\nwarning/error as well, so I don't think we have to worry about that.\n"},{"id":"98298","messageId":"7viqphf4ua.fsf@gitster.siamese.dyndns.org","threadId":"16784","inReplyTo":"20081218213407.GX5691@genesis.frugalware.org","subject":"Re: [PATCH] fast-export: deal with tag objects that do not have a tagger","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-18T23:20:29Z","receivedAt":"2008-12-18T23:20:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna writes:\n\n> On Thu, Dec 18, 2008 at 08:45:44PM +0100, Johannes Schindelin wrote:\n>> \tI think so. The responsible code is in fast-export.c, in any \n>> \tcase. Of course, fast-import refuses to import a tag without a \n>> \ttagger, so...\n>\n> That's why I asked. I think it's a reasonable assumption that in most\n> cases the tagger and the committer of the tagged commit is the same. So\n> in case the tagger info is missing and we tag a commit, we could fake\n> that info on export.\n>\n> Obviously this should not be the default, but I think such a mode would\n> be useful in real-life.\n\nSuch a \"faking\" can well be done, and should be done, on the consuming end\nof the information. If you fake using the commit authorship, you would\nnever be able to tell from the result which one is faked and which one is\ngenuine.\n\nI think you'd rather want to see \"Unspecified Tagger\" in the resulting tag\nobject (or even better, a tag object without the tagger field created by\nthe fast-import process), and leave the interpretation of missing tagger\ninformation to the consumers.\n"},{"id":"98301","messageId":"20081218233843.GA21154@genesis.frugalware.org","threadId":"16784","inReplyTo":"7viqphf4ua.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fast-export: deal with tag objects that do not have a tagger","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-12-18T23:38:43Z","receivedAt":"2008-12-18T23:38:43Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Thu, Dec 18, 2008 at 03:20:29PM -0800, Junio C Hamano wrote:\n> Such a \"faking\" can well be done, and should be done, on the consuming end\n> of the information. If you fake using the commit authorship, you would\n> never be able to tell from the result which one is faked and which one is\n> genuine.\n> \n> I think you'd rather want to see \"Unspecified Tagger\" in the resulting tag\n> object (or even better, a tag object without the tagger field created by\n> the fast-import process), and leave the interpretation of missing tagger\n> information to the consumers.\n\nAah, I missed that Dscho's patch will not just not die(), but it _will_\noutput a stream that fast-import will import. So just forget my\ncomplain. :-)\n\nAnd Dscho, thanks!\n"},{"id":"98396","messageId":"alpine.DEB.1.00.0812200059480.30769@pacific.mpi-cbg.de","threadId":"16784","inReplyTo":"7viqphf4ua.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2] fast-export: deal with tag objects that do not have a tagger","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-12-20T00:00:27Z","receivedAt":"2008-12-20T00:00:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen no tagger was found (old Git produced tags like this),\nno \"tagger\" line is printed (but this is incompatible with the current\ngit fast-import).\n\nAlternatively, you can pass the option --fake-missing-tagger, forcing\nfast-export to fake a tagger\n\n\tUnspecified Tagger \n\nwith a tag date of the beginning of (Unix) time in the case of a missing\ntagger, so that fast-import is still able to import the result.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-fast-export.txt | 6 ++++++\n builtin-fast-export.c | 21 ++++++++++++++++-----\n t/t9301-fast-export.sh | 20 ++++++++++++++++++++\n 3 files changed, 42 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex 99a1c31..0c9eb56 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -65,6 +65,12 @@ If the backend uses a similar \\--import-marks file, this allows for\n incremental bidirectional exporting of the repository by keeping the\n marks the same across runs.\n \n+--fake-missing-tagger::\n+\tSome old repositories have tags without a tagger. The\n+\tfast-import protocol was pretty strict about that, and did not\n+\tallow that. So fake a tagger to be able to fast-import the\n+\toutput.\n+\n \n EXAMPLES\n --------\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex 7d5d57a..8386338 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -24,6 +24,7 @@ static const char *fast_export_usage[] = {\n \n static int progress;\n static enum { VERBATIM, WARN, STRIP, ABORT } signed_tag_mode = ABORT;\n+static int fake_missing_tagger;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n \t\t\t\t const char *arg, int unset)\n@@ -297,10 +298,17 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\tmessage_size = strlen(message);\n \t}\n \ttagger = memmem(buf, message ? message - buf : size, \"\\ntagger \", 8);\n-\tif (!tagger)\n-\t\tdie (\"No tagger for tag %s\", sha1_to_hex(tag->object.sha1));\n-\ttagger++;\n-\ttagger_end = strchrnul(tagger, '\\n');\n+\tif (!tagger) {\n+\t\tif (fake_missing_tagger)\n+\t\t\ttagger = \"tagger Unspecified Tagger \"\n+\t\t\t\t\" 0 +0000\";\n+\t\telse\n+\t\t\ttagger = \"\";\n+\t\ttagger_end = tagger + strlen(tagger);\n+\t} else {\n+\t\ttagger++;\n+\t\ttagger_end = strchrnul(tagger, '\\n');\n+\t}\n \n \t/* handle signed tags */\n \tif (message) {\n@@ -326,9 +334,10 @@ static void handle_tag(const char *name, struct tag *tag)\n \n \tif (!prefixcmp(name, \"refs/tags/\"))\n \t\tname += 10;\n-\tprintf(\"tag %s\\nfrom :%d\\n%.*s\\ndata %d\\n%.*s\\n\",\n+\tprintf(\"tag %s\\nfrom :%d\\n%.*s%sdata %d\\n%.*s\\n\",\n \t name, get_object_mark(tag->tagged),\n \t (int)(tagger_end - tagger), tagger,\n+\t tagger == tagger_end ? \"\" : \"\\n\",\n \t (int)message_size, (int)message_size, message ? message : \"\");\n }\n \n@@ -483,6 +492,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\t\t \"Dump marks to this file\"),\n \t\tOPT_STRING(0, \"import-marks\", &import_filename, \"FILE\",\n \t\t\t \"Import marks from this file\"),\n+\t\tOPT_BOOLEAN(0, \"fake-missing-tagger\", &fake_missing_tagger,\n+\t\t\t \"Fake a tagger when tags lack one\"),\n \t\tOPT_END()\n \t};\n \ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 2057435..9985721 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -239,4 +239,24 @@ test_expect_success 'fast-export | fast-import when master is tagged' '\n \n '\n \n+cat > tag-content << EOF\n+object $(git rev-parse HEAD)\n+type commit\n+tag rosten\n+EOF\n+\n+test_expect_success 'cope with tagger-less tags' '\n+\n+\tTAG=$(git hash-object -t tag -w tag-content) &&\n+\tgit update-ref refs/tags/sonnenschein $TAG &&\n+\tgit fast-export -C -C --signed-tags=strip --all > output &&\n+\ttest $(grep -c \"^tag \" output) = 4 &&\n+\t! grep \"Unspecified Tagger\" output &&\n+\tgit fast-export -C -C --signed-tags=strip --all \\\n+\t\t--fake-missing-tagger > output &&\n+\ttest $(grep -c \"^tag \" output) = 4 &&\n+\tgrep \"Unspecified Tagger\" output\n+\n+'\n+\n test_done\n-- \n1.6.1.rc3.412.ga72b\n"},{"id":"98400","messageId":"7vfxkj65y5.fsf@gitster.siamese.dyndns.org","threadId":"16784","inReplyTo":"alpine.DEB.1.00.0812200059480.30769@pacific.mpi-cbg.de","subject":"[PATCH] fast-import: make tagger information optional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-20T00:33:38Z","receivedAt":"2008-12-20T00:33:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Even though newer Porcelain tools always record the tagger information\nwhen creating new tags, export/import pair should be able to faithfully\nreproduce ancient tag objects that lack tagger information.\n\nSigned-off-by: Junio C Hamano \n---\n * ... and corresponding patch to fast-import may look like this.\n\n fast-import.c | 24 ++++++++++++++----------\n t/t9300-fast-import.sh | 18 ++++++++++++++++++\n 2 files changed, 32 insertions(+), 10 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 201d4ff..d107896 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2265,23 +2265,27 @@ static void parse_new_tag(void)\n \tread_next_command();\n \n \t/* tagger ... */\n-\tif (prefixcmp(command_buf.buf, \"tagger \"))\n-\t\tdie(\"Expected tagger command, got %s\", command_buf.buf);\n-\ttagger = parse_ident(command_buf.buf + 7);\n+\tif (!prefixcmp(command_buf.buf, \"tagger \")) {\n+\t\ttagger = parse_ident(command_buf.buf + 7);\n+\t\tread_next_command();\n+\t} else\n+\t\ttagger = NULL;\n \n \t/* tag payload/message */\n-\tread_next_command();\n \tparse_data(&msg);\n \n \t/* build the tag object */\n \tstrbuf_reset(&new_data);\n+\n \tstrbuf_addf(&new_data,\n-\t\t\"object %s\\n\"\n-\t\t\"type %s\\n\"\n-\t\t\"tag %s\\n\"\n-\t\t\"tagger %s\\n\"\n-\t\t\"\\n\",\n-\t\tsha1_to_hex(sha1), commit_type, t->name, tagger);\n+\t\t \"object %s\\n\"\n+\t\t \"type %s\\n\"\n+\t\t \"tag %s\\n\",\n+\t\t sha1_to_hex(sha1), commit_type, t->name);\n+\tif (tagger)\n+\t\tstrbuf_addf(&new_data,\n+\t\t\t \"tagger %s\\n\", tagger);\n+\tstrbuf_addch(&new_data, '\\n');\n \tstrbuf_addbuf(&new_data, &msg);\n \tfree(tagger);\n \ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 91b5ace..5a2aaf2 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -56,6 +56,12 @@ M 644 :2 file2\n M 644 :3 file3\n M 755 :4 file4\n \n+tag series-A\n+from :5\n+data <actual && test_cmp expect actual'\n \n cat >expect <actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect < wrote:\n> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\n> index 91b5ace..5a2aaf2 100755\n> --- a/t/t9300-fast-import.sh\n> +++ b/t/t9300-fast-import.sh\n> @@ -56,6 +56,12 @@ M 644 :2 file2\n> M 644 :3 file3\n> M 755 :4 file4\n> \n> +tag series-A\n> +from :5\n> +data < +An annotated tag without an tagger\n\nMinor nit: isn't this (here and later one more time) 'a tagger'?\n"},{"id":"98406","messageId":"20081220005939.GV32487@spearce.org","threadId":"16784","inReplyTo":"7vfxkj65y5.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fast-import: make tagger information optional","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-12-20T00:59:39Z","receivedAt":"2008-12-20T00:59:39Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Even though newer Porcelain tools always record the tagger information\n> when creating new tags, export/import pair should be able to faithfully\n> reproduce ancient tag objects that lack tagger information.\n> \n> Signed-off-by: Junio C Hamano \n\nThanks Junio.\n\nAcked-by: Shawn O. Pearce \n\n\n-- \nShawn.\n"}]}