[PATCH] cvsimport move over to using git for each ref to read refs

[PATCH] cvsimport move over to using git for each ref to read refs

[Andy Whitcroft]

cvsimport: move over to using git-for-each-ref to read refs

cvsimport opens all of the files in $GIT_DIR/refs/heads and reads
out the sha1's in order to work out what time the last commit on
that branch was made (in CVS) thus allowing incremental updates.
However, this takes no account of hierachical refs naming producing
the following error for each directory in $GIT_DIR/refs:

  Use of uninitialized value in chomp at /usr/bin/git-cvsimport line 503.
  Use of uninitialized value in concatenation (.) or string at
					/usr/bin/git-cvsimport line 505.
  usage: git-cat-file [-t|-s|-e|-p|<type>] <sha1>

Take advantage of the new packed refs work to use the new
for-each-ref iterator to get this information.  Use the format
specifier to ensure we are neutral to changes in default.

[Junio: although although for-each-ref offers a --perl quoting mode
this patch does not use it as it seems only to make parsing the
output harder in perl.  If there is a neat trick for handling this
'perl' form please educate me.  Here, rely on sha1's to contain
no spaces.]

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index e5a00a1..1a62afa 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -495,13 +495,14 @@ unless(-d $git_dir) {
 	$tip_at_start = `git-rev-parse --verify HEAD`;
 
 	# Get the last import timestamps
-	opendir(D,"$git_dir/refs/heads");
-	while(defined(my $head = readdir(D))) {
-		next if $head =~ /^\./;
-		open(F,"$git_dir/refs/heads/$head")
-			or die "Bad head branch: $head: $!\n";
-		chomp(my $ftag = <F>);
-		close(F);
+	open(H, "git-for-each-ref --format='%(objectname) %(refname)'|") or
+		die "Cannot run git-for-each-ref: $!\n";
+	while(defined(my $entry = <H>)) {
+		chomp($entry);
+		my ($ftag, $name) = split(' ', $entry, 2);
+		next if ($name !~ m@^refs/heads/(.*)$@);
+		my ($head) = ($1);
+
 		open(F,"git-cat-file commit $ftag |");
 		while(<F>) {
 			next unless /^author\s.*\s(\d+)\s[-+]\d{4}$/;
@@ -510,7 +511,7 @@ unless(-d $git_dir) {
 		}
 		close(F);
 	}
-	closedir(D);
+	close(H);
 }
 
 -d $git_dir

Re: [PATCH] cvsimport move over to using git for each ref to read refs

[Jakub Narebski]

Andy Whitcroft wrote:

> +       open(H, "git-for-each-ref --format='%(objectname) %(refname)'|") or

By the way, this is equivalent to using "git-show-ref" introduced by Linus.
But if you want commit timestamp

> cvsimport opens all of the files in $GIT_DIR/refs/heads and reads
> out the sha1's in order to work out what time the last commit on
> that branch was made (in CVS) thus allowing incremental updates.

you can use it in --format as well.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] cvsimport move over to using git for each ref to read refs

[Andy Whitcroft]

Jakub Narebski wrote:
> Andy Whitcroft wrote:
> 
>> +       open(H, "git-for-each-ref --format='%(objectname) %(refname)'|") or
> 
> By the way, this is equivalent to using "git-show-ref" introduced by Linus.
> But if you want commit timestamp
> 
>> cvsimport opens all of the files in $GIT_DIR/refs/heads and reads
>> out the sha1's in order to work out what time the last commit on
>> that branch was made (in CVS) thus allowing incremental updates.
> 
> you can use it in --format as well.

Unfortuantly, for-each-ref only offers us the textual version of this
information not the numeric offset from the epoch which is what
cvsimport is after.

I guess we could teach for-each-ref to output this as well?  Perhaps
something like authorstamp?

-apw

[PATCH 1/2] for each ref add a raw timestamp field type

[Andy Whitcroft]

for-each-ref: add a raw timestamp field type

cvsimport is interested in the raw time stamps (in seconds since
the epoch) to do its time comparisons.  Export the raw timestamp under
{author,committer,tagger}stamp.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 698618b..9d6c4f0 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -51,14 +51,17 @@ static struct {
 	{ "authorname" },
 	{ "authoremail" },
 	{ "authordate", FIELD_TIME },
+	{ "authorstamp", FIELD_TIME },
 	{ "committer" },
 	{ "committername" },
 	{ "committeremail" },
 	{ "committerdate", FIELD_TIME },
+	{ "committerstamp", FIELD_TIME },
 	{ "tagger" },
 	{ "taggername" },
 	{ "taggeremail" },
 	{ "taggerdate", FIELD_TIME },
+	{ "taggerstamp", FIELD_TIME },
 	{ "subject" },
 	{ "body" },
 	{ "contents" },
@@ -344,9 +347,10 @@ static char *copy_email(const char *buf)
 	return line;
 }
 
-static void grab_date(const char *buf, struct atom_value *v)
+static void grab_date(const char *buf, struct atom_value *v, int raw)
 {
 	const char *eoemail = strstr(buf, "> ");
+	const char *eol = strchr(buf, '\n');
 	char *zone;
 	unsigned long timestamp;
 	long tz;
@@ -359,7 +363,15 @@ static void grab_date(const char *buf, s
 	tz = strtol(zone, NULL, 10);
 	if ((tz == LONG_MIN || tz == LONG_MAX) && errno == ERANGE)
 		goto bad;
-	v->s = xstrdup(show_date(timestamp, tz, 0));
+	if (raw) {
+		int len = (eol - eoemail - 2);
+		char *stamp = xmalloc(len + 1);
+
+		memcpy(stamp, eoemail + 2, len);
+		stamp[len] = 0;
+		v->s = stamp;
+	} else
+		v->s = xstrdup(show_date(timestamp, tz, 0));
 	v->ul = timestamp;
 	return;
  bad:
@@ -386,7 +398,8 @@ static void grab_person(const char *who,
 		if (name[wholen] != 0 &&
 		    strcmp(name + wholen, "name") &&
 		    strcmp(name + wholen, "email") &&
-		    strcmp(name + wholen, "date"))
+		    strcmp(name + wholen, "date") &&
+		    strcmp(name + wholen, "stamp"))
 			continue;
 		if (!wholine)
 			wholine = find_wholine(who, wholen, buf, sz);
@@ -399,7 +412,9 @@ static void grab_person(const char *who,
 		else if (!strcmp(name + wholen, "email"))
 			v->s = copy_email(wholine);
 		else if (!strcmp(name + wholen, "date"))
-			grab_date(wholine, v);
+			grab_date(wholine, v, 0);
+		else if (!strcmp(name + wholen, "stamp"))
+			grab_date(wholine, v, 1);
 	}
 }
 

[PATCH 2/2] cvsimport move over to using git for each ref to read refs V2

[Andy Whitcroft]

cvsimport: move over to using git-for-each-ref to read refs V2

cvsimport opens all of the files in $GIT_DIR/refs/heads and reads
out the sha1's in order to work out what time the last commit on
that branch was made (in CVS) thus allowing incremental updates.
However, this takes no account of hierachical refs naming producing
the following error for each directory in $GIT_DIR/refs:

  Use of uninitialized value in chomp at /usr/bin/git-cvsimport line 503.
  Use of uninitialized value in concatenation (.) or string at
					/usr/bin/git-cvsimport line 505.
  usage: git-cat-file [-t|-s|-e|-p|<type>] <sha1>

Take advantage of the new packed refs work to use the new
for-each-ref iterator to get this information.  Use the format
specifier to ensure we are neutral to changes in default.

[Junio: although although for-each-ref offers a --perl quoting mode
this patch does not use it as it seems only to make parsing the
output harder in perl.  If there is a neat trick for handling this
'perl' form please educate me.  Here, rely on sha1's and refnames
to contain no spaces.]

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index e5a00a1..5b13c23 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -495,22 +495,17 @@ unless(-d $git_dir) {
 	$tip_at_start = `git-rev-parse --verify HEAD`;
 
 	# Get the last import timestamps
-	opendir(D,"$git_dir/refs/heads");
-	while(defined(my $head = readdir(D))) {
-		next if $head =~ /^\./;
-		open(F,"$git_dir/refs/heads/$head")
-			or die "Bad head branch: $head: $!\n";
-		chomp(my $ftag = <F>);
-		close(F);
-		open(F,"git-cat-file commit $ftag |");
-		while(<F>) {
-			next unless /^author\s.*\s(\d+)\s[-+]\d{4}$/;
-			$branch_date{$head} = $1;
-			last;
-		}
-		close(F);
+	open(H, "git-for-each-ref --format='%(objectname) %(refname) %(authorstamp)'|") or
+		die "Cannot run git-for-each-ref: $!\n";
+	while(defined(my $entry = <H>)) {
+		chomp($entry);
+		my ($ftag, $name, $stamp, $zone) = split(/ /, $entry, 4);
+		next if ($name !~ m@^refs/heads/(.*)$@);
+		my ($head) = ($1);
+
+		$branch_date{$head} = $stamp;
 	}
-	closedir(D);
+	close(H);
 }
 
 -d $git_dir

Re: [PATCH] cvsimport move over to using git for each ref to read refs

[Jakub Narebski]

Andy Whitcroft wrote:
> Jakub Narebski wrote:
> > Andy Whitcroft wrote:
> > 
> >> +       open(H, "git-for-each-ref --format='%(objectname) %(refname)'|") or
> > 
> > By the way, this is equivalent to using "git-show-ref" introduced by Linus.
> > But if you want commit timestamp
> > 
> >> cvsimport opens all of the files in $GIT_DIR/refs/heads and reads
> >> out the sha1's in order to work out what time the last commit on
> >> that branch was made (in CVS) thus allowing incremental updates.
> > 
> > you can use it in --format as well.
> 
> Unfortuantly, for-each-ref only offers us the textual version of this
> information not the numeric offset from the epoch which is what
> cvsimport is after.
> 
> I guess we could teach for-each-ref to output this as well?  Perhaps
> something like authorstamp?

Or authorepoch. 

I thought about following rpm idea of format modifiers, like 
authorepoch:date for strftime(3) "%c" format or 
commiterepoch:day for strftime(3) "%a %b %d %Y" format.

Feel free to provide patches ;-)
-- 
Jakub Narebski
Poland

Re: [PATCH] cvsimport move over to using git for each ref to read refs

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

> I guess we could teach for-each-ref to output this as well?  Perhaps
> something like authorstamp?

I think you can work with "author" or "committer" to grab the
whole raw line.

About the quoting and parsing, language specific quoting mode is
meant for git-for-each-ref to produce a string that can be eval'ed
in the host language.  Think of the command as a tool to write a
short program for you.

The original is like this:

	# Get the last import timestamps
	opendir(D,"$git_dir/refs/heads");
	while(defined(my $head = readdir(D))) {
		next if $head =~ /^\./;
		open(F,"$git_dir/refs/heads/$head")
			or die "Bad head branch: $head: $!\n";
		chomp(my $ftag = <F>);
		close(F);
		open(F,"git-cat-file commit $ftag |");
		while(<F>) {
			next unless /^author\s.*\s(\d+)\s[-+]\d{4}$/;
			$branch_date{$head} = $1;
			last;
		}
		close(F);
	}
	closedir(D);

The purpose of the loop is to grab all branch heads, and grab
author timestamp for all of them, _and_ stash that in
%branch_date hash indexed by head name.  You would want to have
something like this executed for each branch:

	$refname = %(refname);
	$authorline = %(author);
	$authorline =~ /^author\s.*\s(\d+)\s[-+]\d{4}$/;
        $branch_date{$refname} = $1;

So, you have the tool to write such a program for you, by doing
something like this:

	my $template = '
                $refname = %(refname);
                $authorline = %(author);
                $authorline =~ /^author\s.*\s(\d+)\s[-+]\d{4}$/;
                $branch_date{$refname} = $1;
	';
        my @cmd = ('git-for-each-ref', '--perl', "--format=$template");

	open I, '-|', @cmd, 'refs/heads';
        my $script = join('',<I>);
        close I;

	my ($refname, $authorline);
        eval "$script";

The language specific quoting flag --perl affects how %() are
interpolated into the generated program text as literals.
That's why there is no quote around %(refname) or %(author)
in the example above when defining the $template.

Re: [PATCH] cvsimport move over to using git for each ref to read refs

[Andy Whitcroft]

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>> I guess we could teach for-each-ref to output this as well?  Perhaps
>> something like authorstamp?
> 
> I think you can work with "author" or "committer" to grab the
> whole raw line.
> 
> About the quoting and parsing, language specific quoting mode is
> meant for git-for-each-ref to produce a string that can be eval'ed
> in the host language.  Think of the command as a tool to write a
> short program for you.

Thanks for the education.  Very simple, and very powerful.  I knew there
was a reason for it out there.  Will respin a V3 patch in a bit.

-apw

[PATCH] cvsimport move over to using git for each ref to read refs V3

[Andy Whitcroft]

cvsimport: move over to using git-for-each-ref to read refs V3

cvsimport opens all of the files in $GIT_DIR/refs/heads and reads
out the sha1's in order to work out what time the last commit on
that branch was made (in CVS) thus allowing incremental updates.
However, this takes no account of hierachical refs naming producing
the following error for each directory in $GIT_DIR/refs:

  Use of uninitialized value in chomp at /usr/bin/git-cvsimport line 503.
  Use of uninitialized value in concatenation (.) or string at
					/usr/bin/git-cvsimport line 505.
  usage: git-cat-file [-t|-s|-e|-p|<type>] <sha1>

Take advantage of the new packed refs work to use the new
for-each-ref iterator to get this information.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index e5a00a1..92d14c3 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -495,22 +495,19 @@ unless(-d $git_dir) {
 	$tip_at_start = `git-rev-parse --verify HEAD`;
 
 	# Get the last import timestamps
-	opendir(D,"$git_dir/refs/heads");
-	while(defined(my $head = readdir(D))) {
-		next if $head =~ /^\./;
-		open(F,"$git_dir/refs/heads/$head")
-			or die "Bad head branch: $head: $!\n";
-		chomp(my $ftag = <F>);
-		close(F);
-		open(F,"git-cat-file commit $ftag |");
-		while(<F>) {
-			next unless /^author\s.*\s(\d+)\s[-+]\d{4}$/;
-			$branch_date{$head} = $1;
-			last;
-		}
-		close(F);
+	my $fmt = '($ref, $author) = (%(refname), %(author));';
+	open(H, "git-for-each-ref --perl --format='$fmt'|") or
+		die "Cannot run git-for-each-ref: $!\n";
+	while(defined(my $entry = <H>)) {
+		my ($ref, $author);
+		eval($entry) || die "cannot eval refs list: $@";
+
+		next if ($ref !~ m@^refs/heads/(.*)$@);
+		my ($head) = ($1);
+		$author =~ /^.*\s(\d+)\s[-+]\d{4}$/;
+		$branch_date{$head} = $1;
 	}
-	closedir(D);
+	close(H);
 }
 
 -d $git_dir

Re: [PATCH] cvsimport move over to using git for each ref to read refs

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

>> About the quoting and parsing, language specific quoting mode is
>> meant for git-for-each-ref to produce a string that can be eval'ed
>> in the host language.  Think of the command as a tool to write a
>> short program for you.
>
> Thanks for the education.  Very simple, and very powerful.  I knew there
> was a reason for it out there.  Will respin a V3 patch in a bit.

This probably showed that my initial description and example for
the feature found in Documentation/git-for-each-ref was lacking.

I would appreciate a separate patch to enhance it if you are so
inclined.

Re: [PATCH] cvsimport move over to using git for each ref to read refs V3

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

> +	my $fmt = '($ref, $author) = (%(refname), %(author));';
> +	open(H, "git-for-each-ref --perl --format='$fmt'|") or
> +		die "Cannot run git-for-each-ref: $!\n";
> +	while(defined(my $entry = <H>)) {
> +		my ($ref, $author);
> +		eval($entry) || die "cannot eval refs list: $@";
> +
> +		next if ($ref !~ m@^refs/heads/(.*)$@);
> +		my ($head) = ($1);
> +		$author =~ /^.*\s(\d+)\s[-+]\d{4}$/;
> +		$branch_date{$head} = $1;
>  	}
> -	closedir(D);
> +	close(H);
>  }

for-each-ref let's you limit the refs by leading path, so I do
not think "next if" inside the loop is needed if you say

	for-each-ref --perl --format=$fmt refs/heads

Any reason you did not like my version that allows you to get
rid of the while() loop altogether?  (hint: replace eval in my
example with "print" and see what you are evaling)

Re: [PATCH] cvsimport move over to using git for each ref to read refs V3

[Andy Whitcroft]

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>> +	my $fmt = '($ref, $author) = (%(refname), %(author));';
>> +	open(H, "git-for-each-ref --perl --format='$fmt'|") or
>> +		die "Cannot run git-for-each-ref: $!\n";
>> +	while(defined(my $entry = <H>)) {
>> +		my ($ref, $author);
>> +		eval($entry) || die "cannot eval refs list: $@";
>> +
>> +		next if ($ref !~ m@^refs/heads/(.*)$@);
>> +		my ($head) = ($1);
>> +		$author =~ /^.*\s(\d+)\s[-+]\d{4}$/;
>> +		$branch_date{$head} = $1;
>>  	}
>> -	closedir(D);
>> +	close(H);
>>  }
> 
> for-each-ref let's you limit the refs by leading path, so I do
> not think "next if" inside the loop is needed if you say
> 
> 	for-each-ref --perl --format=$fmt refs/heads

Stupid boy :)

> 
> Any reason you did not like my version that allows you to get
> rid of the while() loop altogether?  (hint: replace eval in my
> example with "print" and see what you are evaling)

To my mind we avoid the 'mozilla repo' issue of 1000's of heads
busting some string length limit, or requiring some _vast_ string to
hold it (as perl is likely to cope).  Given we are sifting a small
percentage of the data out of it.

Let me know which way you want it and we'll go that way :).

-apw

Re: [PATCH] cvsimport move over to using git for each ref to read refs V3

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

> Junio C Hamano wrote:
>> 
>> for-each-ref let's you limit the refs by leading path, so I do
>> not think "next if" inside the loop is needed if you say
>> 
>> 	for-each-ref --perl --format=$fmt refs/heads
>
> Stupid boy :)
>> 
>> Any reason you did not like my version that allows you to get
>> rid of the while() loop altogether?  (hint: replace eval in my
>> example with "print" and see what you are evaling)
>
> To my mind we avoid the 'mozilla repo' issue of 1000's of heads
> busting some string length limit, or requiring some _vast_ string to
> hold it (as perl is likely to cope).  Given we are sifting a small
> percentage of the data out of it.

Quite valid reasoning.

Will apply V3 with refs/heads filtering added; no need to
resend.

Re: [PATCH] cvsimport move over to using git for each ref to read refs

[Andy Whitcroft]

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>>> About the quoting and parsing, language specific quoting mode is
>>> meant for git-for-each-ref to produce a string that can be eval'ed
>>> in the host language.  Think of the command as a tool to write a
>>> short program for you.
>> Thanks for the education.  Very simple, and very powerful.  I knew there
>> was a reason for it out there.  Will respin a V3 patch in a bit.
> 
> This probably showed that my initial description and example for
> the feature found in Documentation/git-for-each-ref was lacking.
> 
> I would appreciate a separate patch to enhance it if you are so
> inclined.

Actually the description in that manual page is pretty good.  But I
thought it was undocumented as the standard install does not install the
manual pages and I was getting the ones from my stable install to fool
me into thinking they were installed.  New developer error, doh.

That said I've taken the liberty of updating and clarifying the language
to make it very obvious from the outset that the language specific
output formats are for generating eval'able snippets and added a simple
example of it to complement your complex one.

Patch to follow.

-apw

Re: [PATCH] cvsimport move over to using git for each ref to read refs

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

> ... I've taken the liberty of updating and clarifying the language
> to make it very obvious from the outset that the language specific
> output formats are for generating eval'able snippets and added a simple
> example of it to complement your complex one.
>
> Patch to follow.

Thanks.