threads / patch / 12291

patchUse user.name and user.email in import-tars.perl

Subject: [PATCH] Use user.name and user.email in import-tars.perl

## tl;dr

6 messages between Feb 24, 2008 and Sep 8, 2008. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Mike Hommey· Feb 24, 2008, 12:57 UTC · lore
Mimic what is done in git-import.sh and git-import.perl
Signed-off-by: Mike Hommey <mh@glandium.org>
---
 And that made me wonder if it wouldn't be worth, actually, to have
 git config user.name and git config user.email return the "magic" values
 gotten from guessing in ident.c when no value is in the config. That would
 allow scripts, which have no other simple means to get the user name and
 email, to have the same feature as builtins.
 contrib/fast-import/import-tars.perl |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Show changes to contrib/fast-import/import-tars.perl +4 −2
diff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl
index 23aeb25..39c091c 100755
--- a/contrib/fast-import/import-tars.perl
+++ b/contrib/fast-import/import-tars.perl
@@ -14,8 +14,10 @@ die "usage: import-tars *.tar.{gz,bz2,Z}\n" unless @ARGV;
 
 my $branch_name = 'import-tars';
 my $branch_ref = "refs/heads/$branch_name";
-my $committer_name = 'T Ar Creator';
-my $committer_email = 'tar@example.com';
+chomp(my $committer_name = `git config user.name`);
+chomp(my $committer_email = `git config user.email`);
+die 'You need to set user name and email'
+  unless $committer_name && $committer_email;
 
 open(FI, '|-', 'git', 'fast-import', '--quiet')
 	or die "Unable to start git fast-import: $!\n";
-- 
1.5.4.1.48.g0d77
Junio C Hamano· Feb 24, 2008, 18:06 UTC · re: Mike Hommey · lore

Re: [PATCH] Use user.name and user.email in import-tars.perl

Mike Hommey <mh@glandium.org> writes:
Show 10 quoted lines
> Mimic what is done in git-import.sh and git-import.perl
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>
>  And that made me wonder if it wouldn't be worth, actually, to have
>  git config user.name and git config user.email return the "magic" values
>  gotten from guessing in ident.c when no value is in the config. That would
>  allow scripts, which have no other simple means to get the user name and
>  email, to have the same feature as builtins.
Or perhaps use "git var GIT_COMMITTER_IDENT"?
Mike Hommey· Sep 7, 2008, 08:52 UTC · re: Junio C Hamano · lore

[PATCH] Use GIT_COMMITTER_IDENT instead of hardcoded values in import-tars.perl

Signed-off-by: Mike Hommey <mh@glandium.org>
---
http://marc.info/?l=git&m=120385776127178&w=2:
Show 7 quoted lines
> >  And that made me wonder if it wouldn't be worth, actually, to have
> > git config user.name and git config user.email return the "magic" values
> > gotten from guessing in ident.c when no value is in the config. That would
> > allow scripts, which have no other simple means to get the user name and
> > email, to have the same feature as builtins.
>
> Or perhaps use "git var GIT_COMMITTER_IDENT"?

The only problem I see with this approach is that lots of uses of GIT_COMMITTER_IDENT require regex'ing the output to remove the date (see git-am, old git-commit and git-tag...)

Maybe adding another variable not containing the date would be a good idea?

 contrib/fast-import/import-tars.perl |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
Show changes to contrib/fast-import/import-tars.perl +5 −4
diff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl
index 23aeb25..814fb73 100755
--- a/contrib/fast-import/import-tars.perl
+++ b/contrib/fast-import/import-tars.perl
@@ -14,8 +14,9 @@ die "usage: import-tars *.tar.{gz,bz2,Z}\n" unless @ARGV;
 
 my $branch_name = 'import-tars';
 my $branch_ref = "refs/heads/$branch_name";
-my $committer_name = 'T Ar Creator';
-my $committer_email = 'tar@example.com';
+chomp(my $committer_ident = `git var GIT_COMMITTER_IDENT`);
+die 'You need to set user name and email'
+	unless ($committer_ident =~ s/(.+ <[^>]+>).*/\1/);
 
 open(FI, '|-', 'git', 'fast-import', '--quiet')
 	or die "Unable to start git fast-import: $!\n";
@@ -100,7 +101,7 @@ foreach my $tar_file (@ARGV)
 
 	print FI <<EOF;
 commit $branch_ref
-committer $committer_name <$committer_email> $commit_time +0000
+committer $committer_ident $commit_time +0000
 data <<END_OF_COMMIT_MESSAGE
 Imported from $tar_file.
 END_OF_COMMIT_MESSAGE
@@ -119,7 +120,7 @@ EOF
 	print FI <<EOF;
 tag $tar_name
 from $branch_ref
-tagger $committer_name <$committer_email> $commit_time +0000
+tagger $committer_ident $commit_time +0000
 data <<END_OF_TAG_MESSAGE
 Package $tar_name
 END_OF_TAG_MESSAGE
-- 
1.6.0.2.g2ebc0
Junio C Hamano· Sep 7, 2008, 17:09 UTC · re: Mike Hommey · lore

Re: [PATCH] Use GIT_COMMITTER_IDENT instead of hardcoded values in import-tars.perl

Mike Hommey <mh@glandium.org> writes:
Show 18 quoted lines
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>
> http://marc.info/?l=git&m=120385776127178&w=2:
>> >  And that made me wonder if it wouldn't be worth, actually, to have
>> > git config user.name and git config user.email return the "magic" values
>> > gotten from guessing in ident.c when no value is in the config. That would
>> > allow scripts, which have no other simple means to get the user name and
>> > email, to have the same feature as builtins.
>>
>> Or perhaps use "git var GIT_COMMITTER_IDENT"?
>
> The only problem I see with this approach is that lots of uses of
> GIT_COMMITTER_IDENT require regex'ing the output to remove the date
> (see git-am, old git-commit and git-tag...)
>
> Maybe adding another variable not containing the date would be a
> good idea?

Looking at the two patches side-by-side makes me think your original from from February actually is much better.

Why don't we apply your original, reproduced here for a quick round of comment?

-- >8 --
From: Mike Hommey <mh@glandium.org>
Subject: [PATCH] Use user.name and user.email in import-tars.perl
Date: Sun, 24 Feb 2008 13:57:18 +0100
Mimic what is done in git-import.sh and git-import.perl
Signed-off-by: Mike Hommey <mh@glandium.org>
---
 contrib/fast-import/import-tars.perl |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Show changes to contrib/fast-import/import-tars.perl +4 −2
diff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl
index 23aeb25..39c091c 100755
--- a/contrib/fast-import/import-tars.perl
+++ b/contrib/fast-import/import-tars.perl
@@ -14,8 +14,10 @@ die "usage: import-tars *.tar.{gz,bz2,Z}\n" unless @ARGV;
 
 my $branch_name = 'import-tars';
 my $branch_ref = "refs/heads/$branch_name";
-my $committer_name = 'T Ar Creator';
-my $committer_email = 'tar@example.com';
+chomp(my $committer_name = `git config user.name`);
+chomp(my $committer_email = `git config user.email`);
+die 'You need to set user name and email'
+  unless $committer_name && $committer_email;
 
 open(FI, '|-', 'git', 'fast-import', '--quiet')
 	or die "Unable to start git fast-import: $!\n";
Johannes Schindelin· Sep 8, 2008, 14:51 UTC · re: Mike Hommey · lore

Re: [PATCH] Use GIT_COMMITTER_IDENT instead of hardcoded values in import-tars.perl

Hi,
On Sun, 7 Sep 2008, Mike Hommey wrote:
Show 5 quoted lines
> -my $committer_name = 'T Ar Creator';
> -my $committer_email = 'tar@example.com';
> +chomp(my $committer_ident = `git var GIT_COMMITTER_IDENT`);
> +die 'You need to set user name and email'
> +	unless ($committer_ident =~ s/(.+ <[^>]+>).*/\1/);
I have at least one script that will be broken by this change in behavior.

To me, the issue is just like git-cvsimport, which sets the committer not to the actual committer, so that two people can end up with identical commit names, even if they cvsimported independently. I'd like the same behavior for import-tars. I actually use it that way.

Ciao, Dscho

Junio C Hamano· Sep 8, 2008, 20:40 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Use GIT_COMMITTER_IDENT instead of hardcoded values in import-tars.perl

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 14 quoted lines
> On Sun, 7 Sep 2008, Mike Hommey wrote:
>
>> -my $committer_name = 'T Ar Creator';
>> -my $committer_email = 'tar@example.com';
>> +chomp(my $committer_ident = `git var GIT_COMMITTER_IDENT`);
>> +die 'You need to set user name and email'
>> +	unless ($committer_ident =~ s/(.+ <[^>]+>).*/\1/);
>
> I have at least one script that will be broken by this change in behavior.
>
> To me, the issue is just like git-cvsimport, which sets the committer not 
> to the actual committer, so that two people can end up with identical 
> commit names, even if they cvsimported independently.  I'd like the same 
> behavior for import-tars.  I actually use it that way.
I sense there are conflicting goals here.

cvsimport has partial information about the author (only short account name and nothing else), and by replicating them without taking them literally you can achieve reproducibility. On the other extreme is to use the authorname mapping file to sacrifice reproducibility with other people that do not have the identical author mapping file to obtain more readable resulting history with real names. You can do both.

With the hardcoded 'T Ar Creator', you do not have any choice but strict reproducibility without readable names. With Mike's original patch to make it in line with git-import.{sh,perl}, you cannot still have both, because setting GIT_COMMITTER_NAME does not affect what user.name configuration says. But with "git var GIT_COMMITTER_IDENT", you could.

This makes me wonder if it might be a better design to:
 * Make fast-import feeders to preserve as much information from the
   source material but not from the environment.  This is half-similar in
   spirit to what cvsimport does---it does not know the timezone so it
   always uses GMT, and it uses the short account name because it is the
   only thing available, but it does not use hardcoded "cvs", and the
   environment can affect it further by setting up an author mapping
   file.  Here I am saying a fast-import feeder shouldn't (and does not
   have to) take the environment into account, if it does not have good
   data in the source material.
   In the context of importing tarballs, zipfiles and an existing directory
   which is a tarball extract, there is not much authorship information in
   the source material (each entry in a tarball may have the owner
   information, but what if your tarball have more than one files, with
   different owners?).
 * Invent a fast-import stream filter that allows you to munge authorship
   and committer information selectively.  Splice that in to the pipeline
   between the feeder and the fast-import, if you want the resulting
   history more readable if desired (e.g. use author mapping file).
   Or you can choose not to use such a filter, and get a reproducible
   result.

If the "filter" turns out to be simple enough, it might even make sense to make it part of the fast-import itself, but that is an implementation detail.

← back to recent threads