git am mangles commit author name.

git am mangles commit author name.

[Daniel F]

Hi all,

Have a problem with git am. I am trying to apply a patch (created with
git format-patch) that I have in a file.
The file contains the line (along with the rest of the patch, of course):
From: username <emailaddress>

So I do the following:
git am --signoff < patchfile.patch
It applies just fine, but the commit author shows in in git log as:
emailaddress <emailaddress>
(i.e., the username is nowhere to be seen)

I'm using the latest git release, 1.7.1.1, on ubuntu intrepid.

Would anyone happen to have some suggestions as to what i'm missing,
and how I can get git am to properly treat the commit author name?

Thank you all in advance,
Daniel

Re: git am mangles commit author name.

[Jonathan Nieder]

Hi Daniel,

Daniel F wrote:

> The file contains the line (along with the rest of the patch, of course):
> From: username <emailaddress>
> 
> So I do the following:
> git am --signoff < patchfile.patch
> It applies just fine, but the commit author shows in in git log as:
> emailaddress <emailaddress>
> (i.e., the username is nowhere to be seen)

Because I am lazy I tried with the latest 1.7.2 release candidate.
Here is what I did:

 $ git fetch git://repo.or.cz/git/jrn.git rr/svn-fe
 $ git checkout FETCH_HEAD
 $ git format-patch HEAD^..HEAD
 $ git reset --keep HEAD^
 $ git am --signoff <0001-*

The commit author showed up with name and email address as I expected.
What did I do wrong?

Alternatively: could you send the first few lines of your .patch file?

Thanks,
Jonathan

Re: git am mangles commit author name.

[Daniel F]

Hi Jonathan,

Thanks for your response!

On 7/13/10, Jonathan Nieder <jrnieder@gmail.com> wrote:
> The commit author showed up with name and email address as I expected.
> What did I do wrong?
>
> Alternatively: could you send the first few lines of your .patch file?

I figured it out :) This was the from line:
From: bc <email@somedomain.net>

Apparently, git-am fails when the username is just one word.
if i put a space between the b and the c, it works just fine.

However... do you not think that it is still a bug? Is it not quite reasonable
for a person to want to identify with just a nick, rather than a full
first and last name?

And more to the immediate point... is there any way i can get git am
to do what i want? :)

Thanks,
Daniel

Re: git am mangles commit author name.

[Jay Soffian]

On Tue, Jul 13, 2010 at 10:54 AM, Daniel F <nanotube@gmail.com> wrote:
> Apparently, git-am fails when the username is just one word.

And is less than 3 characters.

> if i put a space between the b and the c, it works just fine.
>
> However... do you not think that it is still a bug? Is it not quite reasonable
> for a person to want to identify with just a nick, rather than a full
> first and last name?
>
> And more to the immediate point... is there any way i can get git am
> to do what i want? :)

This is happening due to the get_sane_name check in mailinfo.c. The
rules for a "sane" name are actually from Linus' original code. From
2744b23 (Start of early patch applicator tools for git., 2005-04-11):

+static char *sanity_check(char *name, char *email)
+{
+       int len = strlen(name);
+       if (len < 3 || len > 60)
+               return email;
+       if (strchr(name, '@') || strchr(name, '<') || strchr(name, '>'))
+               return email;
+       return name;
+}

You could add an option to mailinfo to disable the sanity check and
then plumb that option into its various callers.

j.

Re: git am mangles commit author name.

[Daniel F]

On 7/13/10, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Tue, Jul 13, 2010 at 10:54 AM, Daniel F <nanotube@gmail.com> wrote:
>> Apparently, git-am fails when the username is just one word.
>
> And is less than 3 characters.
>
> This is happening due to the get_sane_name check in mailinfo.c. The
> rules for a "sane" name are actually from Linus' original code. From
> 2744b23 (Start of early patch applicator tools for git., 2005-04-11):
>
> +static char *sanity_check(char *name, char *email)
> +{
> +       int len = strlen(name);
> +       if (len < 3 || len > 60)
> +               return email;
> +       if (strchr(name, '@') || strchr(name, '<') || strchr(name, '>'))
> +               return email;
> +       return name;
> +}
>
> You could add an option to mailinfo to disable the sanity check and
> then plumb that option into its various callers.
>

Ah yes, indeed... Well, is there any reason to even have that
length sanity check in the first place? If someone wants to be
identified with a nick of 1 or 2 chars, what's wrong with that?

I have approximately zero familiarity with various git internals,
so not sure if anything else depends on this length checkbeing there... But
if not, if it is essentially arbitrary... why add complexity with
extra options, instead of removing complexity by simply taking
out the minimum length check?

At any rate... seeing as how it is hard-coded at the moment,
I guess my immediate solution would be to discuss with my
patch submitter about choosing a longer nick :)

Thanks,
Daniel

Re: git am mangles commit author name.

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 13, 2010 at 15:48, Daniel F <nanotube@gmail.com> wrote:

> Ah yes, indeed... Well, is there any reason to even have that
> length sanity check in the first place? If someone wants to be
> identified with a nick of 1 or 2 chars, what's wrong with that?

I don't know. The whole thing seems a bit silly. I changed it:

    -   if (name->len < 3 || 60 < name->len || strchr(name->buf, '@')
||
                   +   if (name->len < 1 || 60 < name->len ||
strchr(name->buf, '@') ||

And nothing in the test suite broke. Removing the `name->len <' clause
completely broke one test in t5100-mailinfo.sh however.

But I don't understand that part of Git, maybe it really will break
horribly somewhere with:

    Author: av <a@v.net>

Re: git am mangles commit author name.

[Tor Arntsen]

On Tue, Jul 13, 2010 at 17:11, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Tue, Jul 13, 2010 at 10:54 AM, Daniel F <nanotube@gmail.com> wrote:
>> Apparently, git-am fails when the username is just one word.
>
> And is less than 3 characters.
[..]

When I was a student a long time ago there was another (European)
student there with a single-letter name.
His legal name was 'Z'. It was printed like that in all official
documents I ever saw, including bank accounts and loan papers, so it
wasn't just some nickname he was using.

-Tor

Re: git am mangles commit author name.

[Ævar Arnfjörð Bjarmason]

On Wed, Jul 14, 2010 at 14:16, Tor Arntsen <tor@spacetec.no> wrote:
> On Tue, Jul 13, 2010 at 17:11, Jay Soffian <jaysoffian@gmail.com> wrote:
>> On Tue, Jul 13, 2010 at 10:54 AM, Daniel F <nanotube@gmail.com> wrote:
>>> Apparently, git-am fails when the username is just one word.
>>
>> And is less than 3 characters.
> [..]
>
> When I was a student a long time ago there was another (European)
> student there with a single-letter name.
> His legal name was 'Z'. It was printed like that in all official
> documents I ever saw, including bank accounts and loan papers, so it
> wasn't just some nickname he was using.

A lot of people also use their nicknames for their real name in Git,
and plenty of people have 1-3 letter nicks.

Re: git am mangles commit author name.

[Daniel F]

On 7/14/10, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Wed, Jul 14, 2010 at 14:16, Tor Arntsen <tor@spacetec.no> wrote:
>> On Tue, Jul 13, 2010 at 17:11, Jay Soffian <jaysoffian@gmail.com> wrote:
>>> On Tue, Jul 13, 2010 at 10:54 AM, Daniel F <nanotube@gmail.com> wrote:
>>>> Apparently, git-am fails when the username is just one word.
>>>
>>> And is less than 3 characters.
>> [..]

so... any git maintainers have thoughts on how this should
be addressed, or whether this should be addressed at all?
would simply setting minimum length to 1 have any negative
side effects?

Re: git am mangles commit author name.

[Jonathan Nieder]

Hi Daniel,

Daniel F wrote: 

> would simply setting minimum length to 1 have any negative
> side effects?

I am not a git maintainer, but I would suggest writing a patch to do
that.  Then you can try it and see, and send us a copy of the patch
when reporting the results.

My guess is Linus wanted to make sure he had meaningful names in
patches he applies[1].  There is no technical limit I know of that would
lead one to forbid an author name longer than 60 characters, either.
So the questions are:

 - did any code learn to rely on those limits later?
 - are any _people_ relying on those limits now?

I suspect the answer to question 2, at least for the lower bound,
is no.

Hope that helps,
Jonathan

[1] See v0.99.2~51^2~21 (Start of early patch applicator tools for
git., 2005-04-11) for the original script.