git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] git-cvsimport: support local timezone
@ 2012-10-13  4:11 Chris Rorvick
  2012-10-13  4:11 ` [PATCH v2 1/2] git-cvsimport: use localtime for converting timestamps Chris Rorvick
  2012-10-13  4:11 ` [PATCH v2 2/2] git-cvsimport: allow local timezone for commits Chris Rorvick
  0 siblings, 2 replies; 5+ messages in thread
From: Chris Rorvick @ 2012-10-13  4:11 UTC (permalink / raw)
  To: git

Changes to support local timezone offsets in imported commits.  Modified
documentation to clarify behavior of new -l option.

Also, I split the original patch into two because using
localtime()/timelocal() does not affect current functionality, but makes
for sane results if someone monkeys with the hardcoded TZ environment
setting.  Also, the problem solved by the second patch could also be
addressed by simply removing the hardcoded TZ setting in favor of having
the user specify in their environment.  Both of these solutions build on
the changes made in the first patch, though.

Chris Rorvick (2):
  git-cvsimport: use localtime for converting timestamps
  git-cvsimport: allow local timezone for commits

 Documentation/git-cvsimport.txt |   13 ++++++++++---
 git-cvsimport.perl              |   13 +++++++------
 2 files changed, 17 insertions(+), 9 deletions(-)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] git-cvsimport: use localtime for converting timestamps
  2012-10-13  4:11 [PATCH v2 0/2] git-cvsimport: support local timezone Chris Rorvick
@ 2012-10-13  4:11 ` Chris Rorvick
  2012-10-13  4:11 ` [PATCH v2 2/2] git-cvsimport: allow local timezone for commits Chris Rorvick
  1 sibling, 0 replies; 5+ messages in thread
From: Chris Rorvick @ 2012-10-13  4:11 UTC (permalink / raw)
  To: git

cvsps formats timestamps for the local timezone in its output.
Using timegm() to convert to epoch-relative only works because
cvsimport overrides TZ to "UTC".  Using timelocal() does not change
the behavior of the script as is, but it does ensure cvsimport
behaves sanely if run with another TZ value.

Also, use localtime() for generating the commit timestamp instead of
gmtime().  Again, this has no affect on the script as is since TZ
is hard-wired to "UTC".  But using localtime() would allow someone
to change the value of TZ with what is likely the desired effect
(i.e., timestamps are written to the Git commit with local timezone
offset.)

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 git-cvsimport.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 8032f23..2f5da9e 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -582,7 +582,7 @@ sub pdate($) {
 	m#(\d{2,4})/(\d\d)/(\d\d)\s(\d\d):(\d\d)(?::(\d\d))?#
 		or die "Unparseable date: $d\n";
 	my $y=$1; $y-=1900 if $y>1900;
-	return timegm($6||0,$5,$4,$3,$2-1,$y);
+	return timelocal($6||0,$5,$4,$3,$2-1,$y);
 }
 
 sub pmode($) {
@@ -844,7 +844,7 @@ sub commit {
 		}
 	}
 
-	my $commit_date = strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date));
+	my $commit_date = strftime("%z %Y-%m-%d %H:%M:%S",localtime($date));
 	$ENV{GIT_AUTHOR_NAME} = $author_name;
 	$ENV{GIT_AUTHOR_EMAIL} = $author_email;
 	$ENV{GIT_AUTHOR_DATE} = $commit_date;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] git-cvsimport: allow local timezone for commits
  2012-10-13  4:11 [PATCH v2 0/2] git-cvsimport: support local timezone Chris Rorvick
  2012-10-13  4:11 ` [PATCH v2 1/2] git-cvsimport: use localtime for converting timestamps Chris Rorvick
@ 2012-10-13  4:11 ` Chris Rorvick
  2012-10-14  6:04   ` Junio C Hamano
  1 sibling, 1 reply; 5+ messages in thread
From: Chris Rorvick @ 2012-10-13  4:11 UTC (permalink / raw)
  To: git

CVS patches are imported with the timezone offset of +0000 (UTC).
Allow timezone offsets to be calculated from the the local timezone by
adding -l to the command line or specifying cvsimport.l in the config.

This could be made the default behavior, as setting TZ=UTC in the
environment before doing the import is equivalent to the current
behavior.  But since a new default may be an unwelcome surprise to
some, make this new behavior available as an option.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 Documentation/git-cvsimport.txt |   13 ++++++++++---
 git-cvsimport.perl              |    9 +++++----
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
index 6695ab3..9059ad1 100644
--- a/Documentation/git-cvsimport.txt
+++ b/Documentation/git-cvsimport.txt
@@ -11,9 +11,9 @@ SYNOPSIS
 [verse]
 'git cvsimport' [-o <branch-for-HEAD>] [-h] [-v] [-d <CVSROOT>]
 	      [-A <author-conv-file>] [-p <options-for-cvsps>] [-P <file>]
-	      [-C <git_repository>] [-z <fuzz>] [-i] [-k] [-u] [-s <subst>]
-	      [-a] [-m] [-M <regex>] [-S <regex>] [-L <commitlimit>]
-	      [-r <remote>] [-R] [<CVS_module>]
+	      [-C <git_repository>] [-z <fuzz>] [-i] [-k] [-l] [-u]
+	      [-s <subst>] [-a] [-m] [-M <regex>] [-S <regex>]
+	      [-L <commitlimit>] [-r <remote>] [-R] [<CVS_module>]
 
 
 DESCRIPTION
@@ -89,6 +89,13 @@ the old cvs2git tool.
 	to avoid noisy changesets. Highly recommended, but off by default
 	to preserve compatibility with early imported trees.
 
+-l::
+	Use the local timezone for computing the timezone offset of commit
+	timestamps instead of the default of +0000 (UTC).  The `TZ`
+	environment variable can be used to override the default local
+	timezone, possibly useful if you are importing from a non-local
+	repository.
+
 -u::
 	Convert underscores in tag and branch names to dots.
 
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 2f5da9e..927d75c 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -28,9 +28,8 @@ use POSIX qw(strftime dup2 ENOENT);
 use IPC::Open2;
 
 $SIG{'PIPE'}="IGNORE";
-$ENV{'TZ'}="UTC";
 
-our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r, $opt_R);
+our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_l,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r, $opt_R);
 my (%conv_author_name, %conv_author_email);
 
 sub usage(;$) {
@@ -40,7 +39,7 @@ sub usage(;$) {
 Usage: git cvsimport     # fetch/update GIT from CVS
        [-o branch-for-HEAD] [-h] [-v] [-d CVSROOT] [-A author-conv-file]
        [-p opts-for-cvsps] [-P file] [-C GIT_repository] [-z fuzz] [-i] [-k]
-       [-u] [-s subst] [-a] [-m] [-M regex] [-S regex] [-L commitlimit]
+       [-l] [-u] [-s subst] [-a] [-m] [-M regex] [-S regex] [-L commitlimit]
        [-r remote] [-R] [CVS_module]
 END
 	exit(1);
@@ -128,7 +127,7 @@ sub read_repo_config {
 	}
 }
 
-my $opts = "haivmkuo:d:p:r:C:z:s:M:P:A:S:L:R";
+my $opts = "haivmkulo:d:p:r:C:z:s:M:P:A:S:L:R";
 read_repo_config($opts);
 Getopt::Long::Configure( 'no_ignore_case', 'bundling' );
 
@@ -138,6 +137,8 @@ GetOptions( map { s/:/=s/; /M/ ? "$_\@" : $_ } split( /(?!:)/, $opts ) )
     or usage();
 usage if $opt_h;
 
+$ENV{'TZ'}="UTC" unless $opt_l;
+
 if (@ARGV == 0) {
 		chomp(my $module = `git config --get cvsimport.module`);
 		push(@ARGV, $module) if $? == 0;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] git-cvsimport: allow local timezone for commits
  2012-10-13  4:11 ` [PATCH v2 2/2] git-cvsimport: allow local timezone for commits Chris Rorvick
@ 2012-10-14  6:04   ` Junio C Hamano
  2012-10-14 20:58     ` Christopher Rorvick
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2012-10-14  6:04 UTC (permalink / raw)
  To: Chris Rorvick; +Cc: git

Chris Rorvick <chris@rorvick.com> writes:

> CVS patches are imported with the timezone offset of +0000 (UTC).
> Allow timezone offsets to be calculated from the the local timezone by
> adding -l to the command line or specifying cvsimport.l in the config.

A single "I do not like everybody's timestamp is in GMT, so instead
use the local timezone I the importer happen to be in" sounds more
like an uninteresting hack with limited application than a useful
new feature.  Even back in CVS days, many projects and repositories
worth converting to Git were multi-people projects that span across
timezones.

I am wondering if it is sufficient to enhance existing cvs-authors
file to tie a person to a timezone to add a feature like this in a
more sensible manner.  I'd assume that in many multi-person project,
one person, even when travelling, tend to record commits in a single
timezone (i.e. his or her home timezone).  Even for a single-person
project, adding a single entry ot the file is not too much to ask to
the user.  Being able to view his human-readable name and timezone
would be good value for the amount of trouble.

> This could be made the default behavior, as setting TZ=UTC in the
> environment before doing the import is equivalent to the current
> behavior.  But since a new default may be an unwelcome surprise to
> some, make this new behavior available as an option.

This, just like use of cvs-authors, will never be the default.

When you and somebody else import the same history recorded in the
same CVS repository, you would want to see the same resulting
history, and that is even more true when these two people may
perform their import incrementally.  If you switch the default
in one version of Git, their histories will diverge at a different
place depending on when their sysadmins updated Git.

The repeatability is why the script does not use the local timezone
the importer happens to be in.


> Signed-off-by: Chris Rorvick <chris@rorvick.com>
> ---
>  Documentation/git-cvsimport.txt |   13 ++++++++++---
>  git-cvsimport.perl              |    9 +++++----
>  2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
> index 6695ab3..9059ad1 100644
> --- a/Documentation/git-cvsimport.txt
> +++ b/Documentation/git-cvsimport.txt
> @@ -11,9 +11,9 @@ SYNOPSIS
>  [verse]
>  'git cvsimport' [-o <branch-for-HEAD>] [-h] [-v] [-d <CVSROOT>]
>  	      [-A <author-conv-file>] [-p <options-for-cvsps>] [-P <file>]
> -	      [-C <git_repository>] [-z <fuzz>] [-i] [-k] [-u] [-s <subst>]
> -	      [-a] [-m] [-M <regex>] [-S <regex>] [-L <commitlimit>]
> -	      [-r <remote>] [-R] [<CVS_module>]
> +	      [-C <git_repository>] [-z <fuzz>] [-i] [-k] [-l] [-u]
> +	      [-s <subst>] [-a] [-m] [-M <regex>] [-S <regex>]
> +	      [-L <commitlimit>] [-r <remote>] [-R] [<CVS_module>]
>  
>  
>  DESCRIPTION
> @@ -89,6 +89,13 @@ the old cvs2git tool.
>  	to avoid noisy changesets. Highly recommended, but off by default
>  	to preserve compatibility with early imported trees.
>  
> +-l::
> +	Use the local timezone for computing the timezone offset of commit
> +	timestamps instead of the default of +0000 (UTC).  The `TZ`
> +	environment variable can be used to override the default local
> +	timezone, possibly useful if you are importing from a non-local
> +	repository.
> +
>  -u::
>  	Convert underscores in tag and branch names to dots.
>  
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index 2f5da9e..927d75c 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -28,9 +28,8 @@ use POSIX qw(strftime dup2 ENOENT);
>  use IPC::Open2;
>  
>  $SIG{'PIPE'}="IGNORE";
> -$ENV{'TZ'}="UTC";
>  
> -our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r, $opt_R);
> +our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_l,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r, $opt_R);
>  my (%conv_author_name, %conv_author_email);
>  
>  sub usage(;$) {
> @@ -40,7 +39,7 @@ sub usage(;$) {
>  Usage: git cvsimport     # fetch/update GIT from CVS
>         [-o branch-for-HEAD] [-h] [-v] [-d CVSROOT] [-A author-conv-file]
>         [-p opts-for-cvsps] [-P file] [-C GIT_repository] [-z fuzz] [-i] [-k]
> -       [-u] [-s subst] [-a] [-m] [-M regex] [-S regex] [-L commitlimit]
> +       [-l] [-u] [-s subst] [-a] [-m] [-M regex] [-S regex] [-L commitlimit]
>         [-r remote] [-R] [CVS_module]
>  END
>  	exit(1);
> @@ -128,7 +127,7 @@ sub read_repo_config {
>  	}
>  }
>  
> -my $opts = "haivmkuo:d:p:r:C:z:s:M:P:A:S:L:R";
> +my $opts = "haivmkulo:d:p:r:C:z:s:M:P:A:S:L:R";
>  read_repo_config($opts);
>  Getopt::Long::Configure( 'no_ignore_case', 'bundling' );
>  
> @@ -138,6 +137,8 @@ GetOptions( map { s/:/=s/; /M/ ? "$_\@" : $_ } split( /(?!:)/, $opts ) )
>      or usage();
>  usage if $opt_h;
>  
> +$ENV{'TZ'}="UTC" unless $opt_l;
> +
>  if (@ARGV == 0) {
>  		chomp(my $module = `git config --get cvsimport.module`);
>  		push(@ARGV, $module) if $? == 0;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] git-cvsimport: allow local timezone for commits
  2012-10-14  6:04   ` Junio C Hamano
@ 2012-10-14 20:58     ` Christopher Rorvick
  0 siblings, 0 replies; 5+ messages in thread
From: Christopher Rorvick @ 2012-10-14 20:58 UTC (permalink / raw)
  Cc: git

On Sun, Oct 14, 2012 at 1:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Chris Rorvick <chris@rorvick.com> writes:
>
>> CVS patches are imported with the timezone offset of +0000 (UTC).
>> Allow timezone offsets to be calculated from the the local timezone by
>> adding -l to the command line or specifying cvsimport.l in the config.
>
> A single "I do not like everybody's timestamp is in GMT, so instead
> use the local timezone I the importer happen to be in" sounds more
> like an uninteresting hack with limited application than a useful
> new feature.  Even back in CVS days, many projects and repositories
> worth converting to Git were multi-people projects that span across
> timezones.
>
> I am wondering if it is sufficient to enhance existing cvs-authors
> file to tie a person to a timezone to add a feature like this in a
> more sensible manner.  I'd assume that in many multi-person project,
> one person, even when travelling, tend to record commits in a single
> timezone (i.e. his or her home timezone).  Even for a single-person
> project, adding a single entry ot the file is not too much to ask to
> the user.  Being able to view his human-readable name and timezone
> would be good value for the amount of trouble.

This sounds pretty straight forward.  It had crossed my mind that
using the cvs-authors file like this would be a more general solution,
but I thought that what I proposed was at least a step in the right
direction.  But since anyone that cares about this is almost certainly
putting together a cvs-authors file anyway, I agree that being able to
set an alternate default timezone probably isn't a very useful
addition.

I'll resubmit when I have something working.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-10-14 20:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-13  4:11 [PATCH v2 0/2] git-cvsimport: support local timezone Chris Rorvick
2012-10-13  4:11 ` [PATCH v2 1/2] git-cvsimport: use localtime for converting timestamps Chris Rorvick
2012-10-13  4:11 ` [PATCH v2 2/2] git-cvsimport: allow local timezone for commits Chris Rorvick
2012-10-14  6:04   ` Junio C Hamano
2012-10-14 20:58     ` Christopher Rorvick

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).