git-fast-export and tags without a tagger

git-fast-export and tags without a tagger

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

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.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Junio C Hamano]

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?

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.

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

[Johannes Schindelin]

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

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

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

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.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

On Thu, Dec 18, 2008 at 11:15:54AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> 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.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Junio C Hamano]

Miklos Vajna <vmiklos@frugalware.org> writes:

> 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.

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

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

On Thu, Dec 18, 2008 at 03:20:29PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> 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!

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Johannes Schindelin]

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

[PATCH] fast-import: make tagger information optional

[Junio C Hamano]

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

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

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

On Fri, Dec 19, 2008 at 04:33:38PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> 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'?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> 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.