Re: [PATCH] git-cvsimport: allow author-specific timezones

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Chris Rorvick <crorvick@xxxxxxxxxx> writes:

> From: Chris Rorvick <chris@xxxxxxxxxxx>
>
> CVS patchsets are imported with timestamps having an offset of +0000
> (UTC).  The cvs-authors file is already used to translate the CVS
> username to full name and email in the corresponding commit.  Extend
> this file to support an optional timezone for calculating a user-
> specific timestamp offset.
>
> Signed-off-by: Chris Rorvick <chris@xxxxxxxxxxx>
> ---
>
> This supercedes the patches submitted for using the local timezone in
> commits.
>
>  Documentation/git-cvsimport.txt    |   5 +-
>  git-cvsimport.perl                 |  22 ++-
>  t/t9604-cvsimport-timestamps.sh    |  92 +++++++++++++
>  t/t9604/cvsroot/.gitattributes     |   1 +
>  t/t9604/cvsroot/CVSROOT/.gitignore |   2 +
>  t/t9604/cvsroot/module/a,v         | 265 +++++++++++++++++++++++++++++++++++++
>  6 files changed, 381 insertions(+), 6 deletions(-)
>  create mode 100775 t/t9604-cvsimport-timestamps.sh

OK, a new test script has its executable bit set (correct).

>  create mode 100664 t/t9604/cvsroot/.gitattributes
>  create mode 100664 t/t9604/cvsroot/CVSROOT/.gitignore
>  create mode 100664 t/t9604/cvsroot/module/a,v
>
> diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
> index 6695ab3..35dc636 100644
> --- a/Documentation/git-cvsimport.txt
> +++ b/Documentation/git-cvsimport.txt
> @@ -141,13 +141,14 @@ This option can be used several times to provide several detection regexes.
>  +
>  ---------
>  	exon=Andreas Ericsson <ae@xxxxxx>
> -	spawn=Simon Pawn <spawn@xxxxxxxxxxxxx>
> +	spawn=Simon Pawn <spawn@xxxxxxxxxxxxx> America/Chicago
>  
>  ---------
>  +
>  'git cvsimport' will make it appear as those authors had
>  their GIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL set properly
> -all along.
> +all along.  If a timezone is specified, GIT_AUTHOR_DATE will
> +have the corresponding offset applied.

The description above the context reads:

    -A <author-conv-file>::
            CVS by default uses the Unix username when writing its
            commit logs. Using this option and an author-conv-file
            in this format

which probably need to be updated to describe what "this format" is
a bit better.

	... an author-conv-file that maps the name recorded in CVS
	to author name, author e-mail and an optional timezone for
	the author

or something.

>  For convenience, this data is saved to `$GIT_DIR/cvs-authors`
>  each time the '-A' option is provided and read from that same

> @@ -844,7 +854,9 @@ sub commit {
>  		}
>  	}
>  
> -	my $commit_date = strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date));
> +	$ENV{'TZ'}=$author_tz;
> +	my $commit_date = strftime("%s %z", localtime($date));

With this, because it updates $commit_date, the specified timezone
applies to both AUTHOR and COMMITTER dates (which is correct---I am
just pointing it out).

> diff --git a/t/t9604-cvsimport-timestamps.sh b/t/t9604-cvsimport-timestamps.sh
> new file mode 100644

Huh?  What happened to the executable bit we saw earlier?

> index 0000000..fb7459c
> --- /dev/null
> +++ b/t/t9604-cvsimport-timestamps.sh
> @@ -0,0 +1,92 @@
> +#!/bin/sh
> +
> +test_description='git cvsimport timestamps'
> +. ./lib-cvs.sh
> +
> +setup_cvs_test_repository t9604
> +
> +test_expect_success 'check timestamps are UTC (TZ=America/Chicago)' '
> +
> +    TZ=America/Chicago git cvsimport -p"-x" -C module-1 module &&
> +    git cvsimport -p"-x" -C module-1 module &&
> +    (cd module-1 &&
> +        git log --pretty="format:%s %ai" -- >../actual-1 &&
> +        echo "" >>../actual-1
> +    ) &&
> +    echo "Rev 16 2011-11-06 07:00:01 +0000
> +Rev 15 2011-11-06 06:59:59 +0000
> +Rev 14 2011-03-13 08:00:01 +0000
> +Rev 13 2011-03-13 07:59:59 +0000
> +Rev 12 2010-12-01 00:00:00 +0000
> +Rev 11 2010-11-01 00:00:00 +0000
> +Rev 10 2010-10-01 00:00:00 +0000
> +Rev  9 2010-09-01 00:00:00 +0000
> +Rev  8 2010-08-01 00:00:00 +0000
> +Rev  7 2010-07-01 00:00:00 +0000
> +Rev  6 2010-06-01 00:00:00 +0000
> +Rev  5 2010-05-01 00:00:00 +0000
> +Rev  4 2010-04-01 00:00:00 +0000
> +Rev  3 2010-03-01 00:00:00 +0000
> +Rev  2 2010-02-01 00:00:00 +0000
> +Rev  1 2010-01-01 00:00:00 +0000" >expect-1 &&
> +    test_cmp actual-1 expect-1
> +'

A handful of issues.

    - (style) Please use one tab for one level of indent;
    - (style) Learn to use <<-\HERE document to make list easier to
      read; 
    - You shouldn't have to choose --pretty=format which places LF
      in between records and add another LF yourself.  Instead, you
      can use --pretty=tformat that uses LF as record terminator, or
      its short-hand form --format="...";
    - I am not sure what you are gaining out of the trailing "--".

See below for a suggested way to lay this out better.

> +test_expect_success 'check timestamps are UTC (TZ=Australia/Sydney)' '
> +
> +    TZ=America/Chicago git cvsimport -p"-x" -C module-2 module &&

This look identical to the first one, but even with a trivial change
to use Australia/Sydney instead of Chicago, I am not sure what the
test buys us.

> +test_expect_success 'check timestamps with author-specific timezones' '
> +
> +    echo "user1=User One <user1@xxxxxxxxxx>
> +user2=User Two <user2@xxxxxxxxxx> America/Chicago
> +user3=User Three <user3@xxxxxxxxxx> Australia/Sydney
> +user4=User Four <user4@xxxxxxxxxx> Asia/Shanghai" >cvs-authors &&
> +    git cvsimport -p"-x" -A cvs-authors -C module-3 module &&
> +    (cd module-3 &&
> +        git log --pretty="format:%s %ai" -- >../actual-3 &&
> +        echo "" >>../actual-3
> +    ) &&
> +    echo "Rev 16 2011-11-06 01:00:01 -0600
> +Rev 15 2011-11-06 01:59:59 -0500
> +Rev 14 2011-03-13 03:00:01 -0500
> +Rev 13 2011-03-13 01:59:59 -0600
> +...
> +Rev  2 2010-01-31 18:00:00 -0600
> +Rev  1 2010-01-01 00:00:00 +0000" >expect-3 &&
> +    test_cmp actual-3 expect-3
> +'

In addition to the same comments as above, this one would benefit
having %an in its output to illustrate that the code is handling
locations with daylight saving time correctly.  It would make it
stand out that Rev 16 is done during the standard time while Rev 15
is done during the DST.  It would look more like this (with the
style fix):

test_expect_success 'check timestamps with author-specific timezones' '
	cat >cvs-authors <<-EOF &&
	user1=User One <user1@xxxxxxxxxx>
	user2=User Two <user2@xxxxxxxxxx> America/Chicago
	user3=User Three <user3@xxxxxxxxxx> Australia/Sydney
	user4=User Four <user4@xxxxxxxxxx> Asia/Shanghai
	EOF
	git cvsimport -p"-x" -A cvs-authors -C module-3 module &&
	(
		cd module-3 &&
		git log --format="%s %ai %an"
	) >actual-3 &&
	cat >expect-3 <<-\EOF
	Rev 16 2011-11-06 01:00:01 -0600 User Two
	Rev 15 2011-11-06 01:59:59 -0500 User Two
	Rev 14 2011-03-13 03:00:01 -0500 User Two
	...
	Rev  2 2010-01-31 18:00:00 -0600 User Two
	Rev  1 2010-01-01 00:00:00 +0000 User One
	EOF
	test_cmp actual-3 expect-3
'
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]