threads / discuss / 16784

git-fast-export and tags without a tagger

Subject: git-fast-export and tags without a tagger

## tl;dr

11 messages between Dec 18, 2008 and Dec 20, 2008.

replies: 10people: 4as markdown or json

Miklos Vajna· Dec 18, 2008, 16:46 UTC · lore
Hi,

Tags created with ancient versions of git have no tagger. The udev repo has such tags, for example:

$ git cat-file tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce object face198a5f21027fefe796dc01e19e349a2d36ce type commit tag 062

fast-export will fail on these repos. From IRC:

20:05 < Keybuk> vmiklos: exactly the same error with 1.6.0.5 20:07 < Keybuk> $ git fast-export --signed-tag=strip --all 20:07 < Keybuk> fatal: No tagger for tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce

I think it would be nice to handle the situation better than just die().

What about a --force option that would fake the tagger in case the tag points to a commmit and use the commiter from there?

I'm willing to work on this, but I thought it's better to discuss the right solution for the problem first.

Thanks.
Junio C Hamano· Dec 18, 2008, 19:15 UTC · re: Miklos Vajna · lore

Re: git-fast-export and tags without a tagger

Miklos Vajna <vmiklos@frugalware.org> writes:
Show 9 quoted lines
> Tags created with ancient versions of git have no tagger. The udev repo
> has such tags, for example:
>
> $ git cat-file tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce
> object face198a5f21027fefe796dc01e19e349a2d36ce
> type commit
> tag 062
>
> fast-export will fail on these repos. From IRC:
Is "fast-export" the only thing that chokes on these tags?

What I am worried about is if we have accidentally/stupidly/by mistake made other codepaths that check the validity of a tag object too strict, which would result things like "git show $such_a_tag" to barf.

Johannes Schindelin· Dec 18, 2008, 19:45 UTC · re: Junio C Hamano · lore

[PATCH] fast-export: deal with tag objects that do not have a tagger

When no tagger was found (old Git produced tags like this), a tagger "No Tagger <no-tagger>" with a tag date of the beginning of (Unix) time is output, so that fast-import is still able to import the result.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	On Thu, 18 Dec 2008, Junio C Hamano wrote:
	> Miklos Vajna <vmiklos@frugalware.org> writes:
	> 
	> > Tags created with ancient versions of git have no tagger. The 
	> > udev repo has such tags, for example:
	> >
	> > $ git cat-file tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce
	> > object face198a5f21027fefe796dc01e19e349a2d36ce
	> > type commit
	> > tag 062
	> >
	> > fast-export will fail on these repos. From IRC:
	> 
	> Is "fast-export" the only thing that chokes on these tags?
	I think so.  The responsible code is in fast-export.c, in any 
	case.  Of course, fast-import refuses to import a tag without a 
	tagger, so...
 builtin-fast-export.c  |   12 ++++++++----
 t/t9301-fast-export.sh |   15 +++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 7d5d57a..0af4e6f 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -297,10 +297,14 @@ static void handle_tag(const char *name, struct tag *tag)
 		message_size = strlen(message);
 	}
 	tagger = memmem(buf, message ? message - buf : size, "\ntagger ", 8);
-	if (!tagger)
-		die ("No tagger for tag %s", sha1_to_hex(tag->object.sha1));
-	tagger++;
-	tagger_end = strchrnul(tagger, '\n');
+	if (!tagger) {
+		warning ("No tagger for tag %s", sha1_to_hex(tag->object.sha1));
+		tagger = "tagger No Tagger <no-tagger> 0 +0000";
+		tagger_end = tagger + strlen(tagger);
+	} else {
+		tagger++;
+		tagger_end = strchrnul(tagger, '\n');
+	}
 
 	/* handle signed tags */
 	if (message) {
diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 2057435..2312d7a 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -239,4 +239,19 @@ test_expect_success 'fast-export | fast-import when master is tagged' '
 
 '
 
+cat > tag-content << EOF
+object $(git rev-parse HEAD)
+type commit
+tag rosten
+EOF
+
+test_expect_success 'cope with tagger-less tags' '
+
+	TAG=$(git hash-object -t tag -w tag-content) &&
+	git update-ref refs/tags/sonnenschein $TAG &&
+	git fast-export -C -C --signed-tags=strip --all > output &&
+	test $(grep -c "^tag " output) = 4
+
+'
+
 test_done
-- 
1.6.1.rc3.368.g63acc
Miklos Vajna· Dec 18, 2008, 21:34 UTC · re: Johannes Schindelin · lore

Re: [PATCH] fast-export: deal with tag objects that do not have a tagger

On Thu, Dec 18, 2008 at 08:45:44PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 	I think so.  The responsible code is in fast-export.c, in any 
> 	case.  Of course, fast-import refuses to import a tag without a 
> 	tagger, so...

That's why I asked. I think it's a reasonable assumption that in most cases the tagger and the committer of the tagged commit is the same. So in case the tagger info is missing and we tag a commit, we could fake that info on export.

Obviously this should not be the default, but I think such a mode would be useful in real-life.

Junio C Hamano· Dec 18, 2008, 23:20 UTC · re: Miklos Vajna · lore

Re: [PATCH] fast-export: deal with tag objects that do not have a tagger

Miklos Vajna <vmiklos@frugalware.org> writes:
Show 12 quoted lines
> On Thu, Dec 18, 2008 at 08:45:44PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> 	I think so.  The responsible code is in fast-export.c, in any 
>> 	case.  Of course, fast-import refuses to import a tag without a 
>> 	tagger, so...
>
> That's why I asked. I think it's a reasonable assumption that in most
> cases the tagger and the committer of the tagged commit is the same. So
> in case the tagger info is missing and we tag a commit, we could fake
> that info on export.
>
> Obviously this should not be the default, but I think such a mode would
> be useful in real-life.

Such a "faking" can well be done, and should be done, on the consuming end of the information. If you fake using the commit authorship, you would never be able to tell from the result which one is faked and which one is genuine.

I think you'd rather want to see "Unspecified Tagger" in the resulting tag object (or even better, a tag object without the tagger field created by the fast-import process), and leave the interpretation of missing tagger information to the consumers.

Miklos Vajna· Dec 18, 2008, 23:38 UTC · re: Junio C Hamano · lore

Re: [PATCH] fast-export: deal with tag objects that do not have a tagger

On Thu, Dec 18, 2008 at 03:20:29PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Such a "faking" can well be done, and should be done, on the consuming end
> of the information.  If you fake using the commit authorship, you would
> never be able to tell from the result which one is faked and which one is
> genuine.
> 
> I think you'd rather want to see "Unspecified Tagger" in the resulting tag
> object (or even better, a tag object without the tagger field created by
> the fast-import process), and leave the interpretation of missing tagger
> information to the consumers.

Aah, I missed that Dscho's patch will not just not die(), but it _will_ output a stream that fast-import will import. So just forget my complain. :-)

And Dscho, thanks!
Johannes Schindelin· Dec 20, 2008, 00:00 UTC · re: Junio C Hamano · lore

[PATCH v2] fast-export: deal with tag objects that do not have a tagger

When no tagger was found (old Git produced tags like this), no "tagger" line is printed (but this is incompatible with the current git fast-import).

Alternatively, you can pass the option --fake-missing-tagger, forcing fast-export to fake a tagger

	Unspecified Tagger <no-tagger>

with a tag date of the beginning of (Unix) time in the case of a missing tagger, so that fast-import is still able to import the result.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-fast-export.txt |    6 ++++++
 builtin-fast-export.c             |   21 ++++++++++++++++-----
 t/t9301-fast-export.sh            |   20 ++++++++++++++++++++
 3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 99a1c31..0c9eb56 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -65,6 +65,12 @@ If the backend uses a similar \--import-marks file, this allows for
 incremental bidirectional exporting of the repository by keeping the
 marks the same across runs.
 
+--fake-missing-tagger::
+	Some old repositories have tags without a tagger.  The
+	fast-import protocol was pretty strict about that, and did not
+	allow that.  So fake a tagger to be able to fast-import the
+	output.
+
 
 EXAMPLES
 --------
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 7d5d57a..8386338 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -24,6 +24,7 @@ static const char *fast_export_usage[] = {
 
 static int progress;
 static enum { VERBATIM, WARN, STRIP, ABORT } signed_tag_mode = ABORT;
+static int fake_missing_tagger;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -297,10 +298,17 @@ static void handle_tag(const char *name, struct tag *tag)
 		message_size = strlen(message);
 	}
 	tagger = memmem(buf, message ? message - buf : size, "\ntagger ", 8);
-	if (!tagger)
-		die ("No tagger for tag %s", sha1_to_hex(tag->object.sha1));
-	tagger++;
-	tagger_end = strchrnul(tagger, '\n');
+	if (!tagger) {
+		if (fake_missing_tagger)
+			tagger = "tagger Unspecified Tagger "
+				"<unspecified-tagger> 0 +0000";
+		else
+			tagger = "";
+		tagger_end = tagger + strlen(tagger);
+	} else {
+		tagger++;
+		tagger_end = strchrnul(tagger, '\n');
+	}
 
 	/* handle signed tags */
 	if (message) {
@@ -326,9 +334,10 @@ static void handle_tag(const char *name, struct tag *tag)
 
 	if (!prefixcmp(name, "refs/tags/"))
 		name += 10;
-	printf("tag %s\nfrom :%d\n%.*s\ndata %d\n%.*s\n",
+	printf("tag %s\nfrom :%d\n%.*s%sdata %d\n%.*s\n",
 	       name, get_object_mark(tag->tagged),
 	       (int)(tagger_end - tagger), tagger,
+	       tagger == tagger_end ? "" : "\n",
 	       (int)message_size, (int)message_size, message ? message : "");
 }
 
@@ -483,6 +492,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			     "Dump marks to this file"),
 		OPT_STRING(0, "import-marks", &import_filename, "FILE",
 			     "Import marks from this file"),
+		OPT_BOOLEAN(0, "fake-missing-tagger", &fake_missing_tagger,
+			     "Fake a tagger when tags lack one"),
 		OPT_END()
 	};
 
diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 2057435..9985721 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -239,4 +239,24 @@ test_expect_success 'fast-export | fast-import when master is tagged' '
 
 '
 
+cat > tag-content << EOF
+object $(git rev-parse HEAD)
+type commit
+tag rosten
+EOF
+
+test_expect_success 'cope with tagger-less tags' '
+
+	TAG=$(git hash-object -t tag -w tag-content) &&
+	git update-ref refs/tags/sonnenschein $TAG &&
+	git fast-export -C -C --signed-tags=strip --all > output &&
+	test $(grep -c "^tag " output) = 4 &&
+	! grep "Unspecified Tagger" output &&
+	git fast-export -C -C --signed-tags=strip --all \
+		--fake-missing-tagger > output &&
+	test $(grep -c "^tag " output) = 4 &&
+	grep "Unspecified Tagger" output
+
+'
+
 test_done
-- 
1.6.1.rc3.412.ga72b
Junio C Hamano· Dec 20, 2008, 00:33 UTC · re: Johannes Schindelin · lore

[PATCH] fast-import: make tagger information optional

Even though newer Porcelain tools always record the tagger information when creating new tags, export/import pair should be able to faithfully reproduce ancient tag objects that lack tagger information.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * ... and corresponding patch to fast-import may look like this.
 fast-import.c          |   24 ++++++++++++++----------
 t/t9300-fast-import.sh |   18 ++++++++++++++++++
 2 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 201d4ff..d107896 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2265,23 +2265,27 @@ static void parse_new_tag(void)
 	read_next_command();
 
 	/* tagger ... */
-	if (prefixcmp(command_buf.buf, "tagger "))
-		die("Expected tagger command, got %s", command_buf.buf);
-	tagger = parse_ident(command_buf.buf + 7);
+	if (!prefixcmp(command_buf.buf, "tagger ")) {
+		tagger = parse_ident(command_buf.buf + 7);
+		read_next_command();
+	} else
+		tagger = NULL;
 
 	/* tag payload/message */
-	read_next_command();
 	parse_data(&msg);
 
 	/* build the tag object */
 	strbuf_reset(&new_data);
+
 	strbuf_addf(&new_data,
-		"object %s\n"
-		"type %s\n"
-		"tag %s\n"
-		"tagger %s\n"
-		"\n",
-		sha1_to_hex(sha1), commit_type, t->name, tagger);
+		    "object %s\n"
+		    "type %s\n"
+		    "tag %s\n",
+		    sha1_to_hex(sha1), commit_type, t->name);
+	if (tagger)
+		strbuf_addf(&new_data,
+			    "tagger %s\n", tagger);
+	strbuf_addch(&new_data, '\n');
 	strbuf_addbuf(&new_data, &msg);
 	free(tagger);
 
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 91b5ace..5a2aaf2 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -56,6 +56,12 @@ M 644 :2 file2
 M 644 :3 file3
 M 755 :4 file4
 
+tag series-A
+from :5
+data <<EOF
+An annotated tag without an tagger
+EOF
+
 INPUT_END
 test_expect_success \
     'A: create pack from stdin' \
@@ -102,6 +108,18 @@ test_expect_success \
 	'git cat-file blob master:file4 >actual && test_cmp expect actual'
 
 cat >expect <<EOF
+object $(git rev-parse refs/heads/master)
+type commit
+tag series-A
+
+An annotated tag without an tagger
+EOF
+test_expect_success 'A: verify tag/series-A' '
+	git cat-file tag tags/series-A >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<EOF
 :2 `git rev-parse --verify master:file2`
 :3 `git rev-parse --verify master:file3`
 :4 `git rev-parse --verify master:file4`
-- 
1.6.1.rc3.48.g2724
Miklos Vajna· Dec 20, 2008, 00:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] fast-import: make tagger information optional

On Fri, Dec 19, 2008 at 04:33:38PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index 91b5ace..5a2aaf2 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -56,6 +56,12 @@ M 644 :2 file2
>  M 644 :3 file3
>  M 755 :4 file4
>  
> +tag series-A
> +from :5
> +data <<EOF
> +An annotated tag without an tagger
Minor nit: isn't this (here and later one more time) 'a tagger'?
Shawn O. Pearce· Dec 20, 2008, 00:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] fast-import: make tagger information optional

Junio C Hamano <gitster@pobox.com> wrote:
Show 5 quoted lines
> Even though newer Porcelain tools always record the tagger information
> when creating new tags, export/import pair should be able to faithfully
> reproduce ancient tag objects that lack tagger information.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thanks Junio.
Acked-by: Shawn O. Pearce <spearce@spearce.org>
-- 
Shawn.
Miklos Vajna· Dec 18, 2008, 21:36 UTC · re: Junio C Hamano · lore

Re: git-fast-export and tags without a tagger

On Thu, Dec 18, 2008 at 11:15:54AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
Show 5 quoted lines
> Is "fast-export" the only thing that chokes on these tags?
> 
> What I am worried about is if we have accidentally/stupidly/by mistake
> made other codepaths that check the validity of a tag object too strict,
> which would result things like "git show $such_a_tag" to barf.

git show works fine on the tag. git fsck --full passes without any warning/error as well, so I don't think we have to worry about that.

← back to recent threads