{"thread":{"id":"13288","subject":"[updated PATCH] Same default as cvsimport when using --use-log-author","startedAt":"2008-04-27T17:32:46Z","lastAt":"2008-05-01T03:47:32Z","messageCount":8,"participants":["Stephen R. van den Berg","Junio C Hamano","Johannes Schindelin","Eric Wong","Andy Whitcroft"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"75305","messageId":"20080427173246.10023.5687.stgit@aristoteles.cuci.nl","threadId":"13288","inReplyTo":null,"subject":"[updated PATCH] Same default as cvsimport when using --use-log-author","fromName":"Stephen R. van den Berg","fromEmail":"srb@cuci.nl","sentAt":"2008-04-27T17:32:46Z","receivedAt":"2008-04-27T17:32:46Z","isPatch":true,"sender":{"key":"srb@cuci.nl","avatar":"https://gravatar.com/avatar/f75389059e827634d38e9df2a9b6ecbd50028b5a454442efa1c7205b7ff29c6a?d=mp&s=160"},"body":"When using git-cvsimport, the author is inferred from the cvs commit,\ne.g. cvs commit logname is foobaruser, then the author field in git\nresults in:\n\nAuthor: foobaruser <foobaruser>\n\nWhich is not perfect, but perfectly acceptable given the circumstances.\n\nThe default git-svn import however, results in:\n\nAuthor: foobaruser <foobaruser@acf43c95-373e-0410-b603-e72c3f656dc1>\n\nWhen using mixes of imports, from CVS and SVN into the same git\nrepository, you'd like to harmonise the imports to the format cvsimport\nuses.\ngit-svn supports an experimental option --use-log-author which currently\nresults in:\n\nAuthor: foobaruser <unknown>\n\nThis patches harmonises the result with cvsimport, and makes\ngit-svn --use-log-author produce:\n\nAuthor: foobaruser <foobaruser>\n\nSigned-off-by: Stephen R. van den Berg <srb@cuci.nl>\n---\n\n git-svn.perl |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex b151049..846e739 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -2434,6 +2434,9 @@ sub make_log_entry {\n \t\t} else {\n \t\t\t($name, $email) = ($name_field, 'unknown');\n \t\t}\n+\t        if (!defined $email) {\n+\t\t    $email = $name;\n+\t        }\n \t}\n \tif (defined $headrev && $self->use_svm_props) {\n \t\tif ($self->rewrite_root) {\n"},{"id":"75331","messageId":"7vbq3vf2k4.fsf@gitster.siamese.dyndns.org","threadId":"13288","inReplyTo":"20080427173246.10023.5687.stgit@aristoteles.cuci.nl","subject":"Re: [updated PATCH] Same default as cvsimport when using --use-log-author","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-27T20:47:07Z","receivedAt":"2008-04-27T20:47:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Stephen R. van den Berg\" <srb@cuci.nl> writes:\n\n> git-svn supports an experimental option --use-log-author which currently\n> results in:\n>\n> Author: foobaruser <unknown>\n\nI have a question about this.  Is the \"<unknown> coming from...\n\n> This patches harmonises the result with cvsimport, and makes\n> git-svn --use-log-author produce:\n>\n> Author: foobaruser <foobaruser>\n> ...\n> diff --git a/git-svn.perl b/git-svn.perl\n> index b151049..846e739 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -2434,6 +2434,9 @@ sub make_log_entry {\n>  \t\t} else {\n>  \t\t\t($name, $email) = ($name_field, 'unknown');\n>  \t\t}\n\n... this 'unknown' we see here?\n\n> +\t        if (!defined $email) {\n> +\t\t    $email = $name;\n> +\t        }\n>  \t}\n\nI would think not -- if that is the case, the codepath you added as a fix\nwould not trigger.  Which means in some other cases, the 'unknown' we see\nabove in the context also still happens.  Is it a good thing?  Maybe we\nwould also want to make it consistently do \"somebody <somebody>\" instead,\nby doing...\n\n\t} else {\n\t\t$name = $name_field;\n\t}\n        if (!defined $email) {\n\t    $email = $name;\n        }\n"},{"id":"75365","messageId":"alpine.DEB.1.00.0804281114560.5399@eeepc-johanness","threadId":"13288","inReplyTo":"20080427173246.10023.5687.stgit@aristoteles.cuci.nl","subject":"Re: [updated PATCH] Same default as cvsimport when using --use-log-author","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-04-28T10:15:55Z","receivedAt":"2008-04-28T10:15:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\ncould we have a oneline description which is more descriptive in gitweb, \nplease?  Something like \"git-svn: use the same default for \n--use-log-author as cvsimport\"?\n\nThanks,\nDscho\n"},{"id":"75494","messageId":"20080429061823.GE24171@muzzle","threadId":"13288","inReplyTo":"7vbq3vf2k4.fsf@gitster.siamese.dyndns.org","subject":"Re: [updated PATCH] Same default as cvsimport when using --use-log-author","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2008-04-29T06:18:23Z","receivedAt":"2008-04-29T06:18:23Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> \"Stephen R. van den Berg\" <srb@cuci.nl> writes:\n> \n> > git-svn supports an experimental option --use-log-author which currently\n> > results in:\n> >\n> > Author: foobaruser <unknown>\n> \n> I have a question about this.  Is the \"<unknown> coming from...\n> \n> > This patches harmonises the result with cvsimport, and makes\n> > git-svn --use-log-author produce:\n> >\n> > Author: foobaruser <foobaruser>\n> > ...\n> > diff --git a/git-svn.perl b/git-svn.perl\n> > index b151049..846e739 100755\n> > --- a/git-svn.perl\n> > +++ b/git-svn.perl\n> > @@ -2434,6 +2434,9 @@ sub make_log_entry {\n> >  \t\t} else {\n> >  \t\t\t($name, $email) = ($name_field, 'unknown');\n> >  \t\t}\n> \n> ... this 'unknown' we see here?\n> \n> > +\t        if (!defined $email) {\n> > +\t\t    $email = $name;\n> > +\t        }\n> >  \t}\n> \n> I would think not -- if that is the case, the codepath you added as a fix\n> would not trigger.  Which means in some other cases, the 'unknown' we see\n> above in the context also still happens.  Is it a good thing?  Maybe we\n> would also want to make it consistently do \"somebody <somebody>\" instead,\n> by doing...\n> \n> \t} else {\n> \t\t$name = $name_field;\n> \t}\n>         if (!defined $email) {\n> \t    $email = $name;\n>         }\n> \n\nI don't think Stephen's patch ever gets triggered, either.\n\nThis section of code was done by Andy, so I can't tell his motivations\nfor using 'unknown' the way he did.\n\n$email does appear to get set correctly for the first two elsifs cases\nhere in the existing code:\n\n\t\tif (!defined $name_field) {\n\t\t\t#\n\t\t} elsif ($name_field =~ /(.*?)\\s+<(.*)>/) {\n\t\t\t($name, $email) = ($1, $2);\n\t\t} elsif ($name_field =~ /(.*)@/) {\n\t\t\t($name, $email) = ($1, $name_field);\n\t\t} else {\n\t\t\t($name, $email) = ($name_field, $name_field);\n\nSo I propose the following one-line change instead of Stephen's:\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex b151049..301a5b4 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -2432,7 +2432,7 @@ sub make_log_entry {\n \t\t} elsif ($name_field =~ /(.*)@/) {\n \t\t\t($name, $email) = ($1, $name_field);\n \t\t} else {\n-\t\t\t($name, $email) = ($name_field, 'unknown');\n+\t\t\t($name, $email) = ($name_field, $name_field);\n \t\t}\n \t}\n \tif (defined $headrev && $self->use_svm_props) {\n\n-- \nEric Wong\n"},{"id":"75516","messageId":"20080429095213.GY5401@shadowen.org","threadId":"13288","inReplyTo":"20080429061823.GE24171@muzzle","subject":"Re: [updated PATCH] Same default as cvsimport when using --use-log-author","fromName":"Andy Whitcroft","fromEmail":"apw@shadowen.org","sentAt":"2008-04-29T09:52:24Z","receivedAt":"2008-04-29T09:52:24Z","isPatch":true,"sender":{"key":"apw@shadowen.org","avatar":"https://gravatar.com/avatar/d3088262854661a913ef35cc40fedcc270142d4461791142bc1ea0b2a4e2e147?d=mp&s=160"},"body":"On Mon, Apr 28, 2008 at 11:18:23PM -0700, Eric Wong wrote:\n> Junio C Hamano <gitster@pobox.com> wrote:\n> > \"Stephen R. van den Berg\" <srb@cuci.nl> writes:\n> > \n> > > git-svn supports an experimental option --use-log-author which currently\n> > > results in:\n> > >\n> > > Author: foobaruser <unknown>\n> > \n> > I have a question about this.  Is the \"<unknown> coming from...\n> > \n> > > This patches harmonises the result with cvsimport, and makes\n> > > git-svn --use-log-author produce:\n> > >\n> > > Author: foobaruser <foobaruser>\n> > > ...\n> > > diff --git a/git-svn.perl b/git-svn.perl\n> > > index b151049..846e739 100755\n> > > --- a/git-svn.perl\n> > > +++ b/git-svn.perl\n> > > @@ -2434,6 +2434,9 @@ sub make_log_entry {\n> > >  \t\t} else {\n> > >  \t\t\t($name, $email) = ($name_field, 'unknown');\n> > >  \t\t}\n> > \n> > ... this 'unknown' we see here?\n> > \n> > > +\t        if (!defined $email) {\n> > > +\t\t    $email = $name;\n> > > +\t        }\n> > >  \t}\n> > \n> > I would think not -- if that is the case, the codepath you added as a fix\n> > would not trigger.  Which means in some other cases, the 'unknown' we see\n> > above in the context also still happens.  Is it a good thing?  Maybe we\n> > would also want to make it consistently do \"somebody <somebody>\" instead,\n> > by doing...\n> > \n> > \t} else {\n> > \t\t$name = $name_field;\n> > \t}\n> >         if (!defined $email) {\n> > \t    $email = $name;\n> >         }\n> > \n> \n> I don't think Stephen's patch ever gets triggered, either.\n> \n> This section of code was done by Andy, so I can't tell his motivations\n> for using 'unknown' the way he did.\n\nMy motivation was that we had picked up a field which is supposed to be\nin RFC822 From: format, ie Name <email>, and dispite trying pretty hard\nwe had not been able to find something that looked like an email to put\nin the email field of the git author et al.  So we didn't really know,\nhence 'unknown'.\n\nThat said it is not at all clear that putting 'unknown' in this field to\navoid putting an invalid email in this field makes much sense as it of\nitself is just as invalid.  So I would probabally be just as happy with\nyour option here.\n\n> $email does appear to get set correctly for the first two elsifs cases\n> here in the existing code:\n> \n> \t\tif (!defined $name_field) {\n> \t\t\t#\n> \t\t} elsif ($name_field =~ /(.*?)\\s+<(.*)>/) {\n> \t\t\t($name, $email) = ($1, $2);\n> \t\t} elsif ($name_field =~ /(.*)@/) {\n> \t\t\t($name, $email) = ($1, $name_field);\n> \t\t} else {\n> \t\t\t($name, $email) = ($name_field, $name_field);\n> \n> So I propose the following one-line change instead of Stephen's:\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index b151049..301a5b4 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -2432,7 +2432,7 @@ sub make_log_entry {\n>  \t\t} elsif ($name_field =~ /(.*)@/) {\n>  \t\t\t($name, $email) = ($1, $name_field);\n>  \t\t} else {\n> -\t\t\t($name, $email) = ($name_field, 'unknown');\n> +\t\t\t($name, $email) = ($name_field, $name_field);\n>  \t\t}\n>  \t}\n>  \tif (defined $headrev && $self->use_svm_props) {\n> \n> -- \n> Eric Wong\n\n-apw\n"},{"id":"75577","messageId":"20080429211356.GA6825@cuci.nl","threadId":"13288","inReplyTo":"20080429061823.GE24171@muzzle","subject":"Re: [updated PATCH] Same default as cvsimport when using --use-log-author","fromName":"Stephen R. van den Berg","fromEmail":"srb@cuci.nl","sentAt":"2008-04-29T21:13:56Z","receivedAt":"2008-04-29T21:13:56Z","isPatch":true,"sender":{"key":"srb@cuci.nl","avatar":"https://gravatar.com/avatar/f75389059e827634d38e9df2a9b6ecbd50028b5a454442efa1c7205b7ff29c6a?d=mp&s=160"},"body":"Eric Wong wrote:\n>Junio C Hamano <gitster@pobox.com> wrote:\n>> \"Stephen R. van den Berg\" <srb@cuci.nl> writes:\n>> > git-svn supports an experimental option --use-log-author which currently\n>> > results in:\n\n>> > Author: foobaruser <unknown>\n\n>> I have a question about this.  Is the \"<unknown> coming from...\n\nI have to correct myself here.  What happens is that if in the commit\nmessage there is no From: or Signed-off-by: to be found to parse, that\nresults in an empty $name_field, and causes $email to stay undefined,\nwhich eventually results in the same silly generated UUID-domain I'm\ntrying to get rid of.\n\nSo it's not triggering the 'unknown' above.\n\n>> I would think not -- if that is the case, the codepath you added as a fix\n>> would not trigger.  Which means in some other cases, the 'unknown' we see\n>> above in the context also still happens.  Is it a good thing?  Maybe we\n>> would also want to make it consistently do \"somebody <somebody>\" instead,\n>> by doing...\n\n>I don't think Stephen's patch ever gets triggered, either.\n\nWell, it is triggered, but rather because $name_field is empty, and\nconsequently $email is never set.\n\n>$email does appear to get set correctly for the first two elsifs cases\n>here in the existing code:\n\n>So I propose the following one-line change instead of Stephen's:\n\n>diff --git a/git-svn.perl b/git-svn.perl\n>@@ -2432,7 +2432,7 @@ sub make_log_entry {\n>-\t\t\t($name, $email) = ($name_field, 'unknown');\n>+\t\t\t($name, $email) = ($name_field, $name_field);\n\nThat is a good change (IMO), but I still need my patch (or something\nsimilar) to cover the undefined $name_field case.  Proposed new patch\nfollows.\n-- \nSincerely,                                                          srb@cuci.nl\n           Stephen R. van den Berg.\n\n\"There's a lot to be said for not saying a lot.\"\n"},{"id":"75580","messageId":"20080429212032.8983.28194.stgit@aristoteles.cuci.nl","threadId":"13288","inReplyTo":"20080429211356.GA6825@cuci.nl","subject":"[updated2 PATCH] git-svn: Same default as cvsimport when using --use-log-author","fromName":"Stephen R. van den Berg","fromEmail":"srb@cuci.nl","sentAt":"2008-04-29T21:20:32Z","receivedAt":"2008-04-29T21:20:32Z","isPatch":true,"sender":{"key":"srb@cuci.nl","avatar":"https://gravatar.com/avatar/f75389059e827634d38e9df2a9b6ecbd50028b5a454442efa1c7205b7ff29c6a?d=mp&s=160"},"body":"When using git-cvsimport, the author is inferred from the cvs commit,\ne.g. cvs commit logname is foobaruser, then the author field in git\nresults in:\n\nAuthor: foobaruser <foobaruser>\n\nWhich is not perfect, but perfectly acceptable given the circumstances.\n\nThe default git-svn import however, results in:\n\nAuthor: foobaruser <foobaruser@acf43c95-373e-0410-b603-e72c3f656dc1>\n\nWhen using mixes of imports, from CVS and SVN into the same git\nrepository, you'd like to harmonise the imports to the format cvsimport\nuses.\ngit-svn supports an experimental option --use-log-author which currently\nresults in the same logentry as without that option when no From: or\nSigned-off-by: is found in the logentry ($email currently ends up empty,\nand hence is generated again).\n\nThis patches harmonises the result with cvsimport, and makes\ngit-svn --use-log-author produce:\n\nAuthor: foobaruser <foobaruser>\n\nSigned-off-by: Stephen R. van den Berg <srb@cuci.nl>\n---\n\n git-svn.perl |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex b151049..67726c1 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -2426,13 +2426,15 @@ sub make_log_entry {\n \t\t\t$name_field = $1;\n \t\t}\n \t\tif (!defined $name_field) {\n-\t\t\t#\n+\t\t\tif (!defined $email) {\n+\t\t\t\t$email = $name;\n+\t\t\t}\n \t\t} elsif ($name_field =~ /(.*?)\\s+<(.*)>/) {\n \t\t\t($name, $email) = ($1, $2);\n \t\t} elsif ($name_field =~ /(.*)@/) {\n \t\t\t($name, $email) = ($1, $name_field);\n \t\t} else {\n-\t\t\t($name, $email) = ($name_field, 'unknown');\n+\t\t\t($name, $email) = ($name_field, $name_field);\n \t\t}\n \t}\n \tif (defined $headrev && $self->use_svm_props) {\n"},{"id":"75730","messageId":"20080501034732.GA29803@untitled","threadId":"13288","inReplyTo":"20080429212032.8983.28194.stgit@aristoteles.cuci.nl","subject":"Re: [updated2 PATCH] git-svn: Same default as cvsimport when using --use-log-author","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2008-05-01T03:47:32Z","receivedAt":"2008-05-01T03:47:32Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"\"Stephen R. van den Berg\" <srb@cuci.nl> wrote:\n> When using git-cvsimport, the author is inferred from the cvs commit,\n> e.g. cvs commit logname is foobaruser, then the author field in git\n> results in:\n> \n> Author: foobaruser <foobaruser>\n> \n> Which is not perfect, but perfectly acceptable given the circumstances.\n> \n> The default git-svn import however, results in:\n> \n> Author: foobaruser <foobaruser@acf43c95-373e-0410-b603-e72c3f656dc1>\n> \n> When using mixes of imports, from CVS and SVN into the same git\n> repository, you'd like to harmonise the imports to the format cvsimport\n> uses.\n> git-svn supports an experimental option --use-log-author which currently\n> results in the same logentry as without that option when no From: or\n> Signed-off-by: is found in the logentry ($email currently ends up empty,\n> and hence is generated again).\n> \n> This patches harmonises the result with cvsimport, and makes\n> git-svn --use-log-author produce:\n> \n> Author: foobaruser <foobaruser>\n \n> Signed-off-by: Stephen R. van den Berg <srb@cuci.nl>\n\nThanks Stephen,\n\nAcked-by: Eric Wong <normalperson@yhbt.net>\n\n> ---\n> \n>  git-svn.perl |    6 ++++--\n>  1 files changed, 4 insertions(+), 2 deletions(-)\n> \n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index b151049..67726c1 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -2426,13 +2426,15 @@ sub make_log_entry {\n>  \t\t\t$name_field = $1;\n>  \t\t}\n>  \t\tif (!defined $name_field) {\n> -\t\t\t#\n> +\t\t\tif (!defined $email) {\n> +\t\t\t\t$email = $name;\n> +\t\t\t}\n>  \t\t} elsif ($name_field =~ /(.*?)\\s+<(.*)>/) {\n>  \t\t\t($name, $email) = ($1, $2);\n>  \t\t} elsif ($name_field =~ /(.*)@/) {\n>  \t\t\t($name, $email) = ($1, $name_field);\n>  \t\t} else {\n> -\t\t\t($name, $email) = ($name_field, 'unknown');\n> +\t\t\t($name, $email) = ($name_field, $name_field);\n>  \t\t}\n>  \t}\n>  \tif (defined $headrev && $self->use_svm_props) {\n> \n> \n> --\n"}]}