{"thread":{"id":"32838","subject":"Why is ident_is_sufficient different on Windows?","startedAt":"2013-02-06T13:06:55Z","lastAt":"2013-02-06T20:52:39Z","messageCount":4,"participants":["Max Horn","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"208798","messageId":"991CBC1C-912C-4DD6-B911-93F6B41D895E@quendi.de","threadId":"32838","inReplyTo":null,"subject":"Why is ident_is_sufficient different on Windows?","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2013-02-06T13:06:55Z","receivedAt":"2013-02-06T13:06:55Z","isPatch":false,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"Hi there,\n\nwhile trying to understand which parts of the author & committer identity are mandatory (name, email, or both), I ended up in ident.c, looking at ident_is_sufficient(), and to my surprise discovered that this seems to differ between Windows (were both are mandatory) and everyone else:\n\nstatic int ident_is_sufficient(int user_ident_explicitly_given)\n{\n#ifndef WINDOWS\n\treturn (user_ident_explicitly_given & IDENT_MAIL_GIVEN);\n#else\n\treturn (user_ident_explicitly_given == IDENT_ALL_GIVEN);\n#endif\n}\n\n\nAccording to git blame, this was introduced here:\n\ncommit 5aeb3a3a838b2cb03d250f3376cf9c41f4d4608e\nAuthor: Junio C Hamano <gitster@pobox.com>\nDate:   Sun Jan 17 13:54:28 2010 -0800\n\n    user_ident_sufficiently_given(): refactor the logic to be usable from elsewhere\n\n\nThe commit message sounds as if this was only a refactoring, but the patch to me look as if it changes behaviour, too. Of course this could very well be false, say due to code elsewhere that already caused Windows to behave differently; I wouldn't know.\n\n\nStill, I wonder: Why does this difference exist?\n\n\nCheers,\nMax\n"},{"id":"208856","messageId":"7vmwvhb2fm.fsf@alter.siamese.dyndns.org","threadId":"32838","inReplyTo":"991CBC1C-912C-4DD6-B911-93F6B41D895E@quendi.de","subject":"Re: Why is ident_is_sufficient different on Windows?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T20:41:17Z","receivedAt":"2013-02-06T20:41:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Horn <max@quendi.de> writes:\n\n> static int ident_is_sufficient(int user_ident_explicitly_given)\n> {\n> #ifndef WINDOWS\n> \treturn (user_ident_explicitly_given & IDENT_MAIL_GIVEN);\n> #else\n> \treturn (user_ident_explicitly_given == IDENT_ALL_GIVEN);\n> #endif\n> }\n>\n>\n> According to git blame, this was introduced here:\n>\n> commit 5aeb3a3a838b2cb03d250f3376cf9c41f4d4608e\n> Author: Junio C Hamano <gitster@pobox.com>\n> Date:   Sun Jan 17 13:54:28 2010 -0800\n>\n>     user_ident_sufficiently_given(): refactor the logic to be usable from elsewhere\n>\n>\n> The commit message sounds as if this was only a refactoring, but\n> the patch to me look as if it changes behaviour, too. Of course\n> this could very well be false, say due to code elsewhere that\n> already caused Windows to behave differently; I wouldn't know.\n>\n> Still, I wonder: Why does this difference exist?\n\nSorry but I do not recall why these ifdefs are there.  The commit\ndid this to builtin-commit.c:\n\n-               if (user_ident_explicitly_given != IDENT_ALL_GIVEN)\n+               if (!user_ident_sufficiently_given())\n\nI would have written the function to always check with ALL_GIVEN\nmyself, and it is very likely that I was *not* the person who\nnoticed that the function needs to behave differently on Windows, as\nI do not do Windows.\n\nI suspect somebody from the Windows camp saw a patch I posted\nwithout the ifdef, noticed that there is a problem to expect\nIDENT_NAME_GIVEN to be set on Windows for some reason, and resulted\nin a reroll of the function in that shape.\n\nI didn't find anything in the list archive, though.  So I am\nstumped.\n"},{"id":"208858","messageId":"7vip65b25c.fsf@alter.siamese.dyndns.org","threadId":"32838","inReplyTo":"7vmwvhb2fm.fsf@alter.siamese.dyndns.org","subject":"Re: Why is ident_is_sufficient different on Windows?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T20:47:27Z","receivedAt":"2013-02-06T20:47:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I suspect somebody from the Windows camp saw a patch I posted\n> without the ifdef, noticed that there is a problem to expect\n> IDENT_NAME_GIVEN to be set on Windows for some reason, and resulted\n> in a reroll of the function in that shape.\n>\n> I didn't find anything in the list archive, though.  So I am\n> stumped.\n\nThe only thing I can think of is that on Unix we can guess name from\nGECOS, which could be considered sufficiently your name, while on\nWindows we probably do not get anything useful there.\n"},{"id":"208863","messageId":"7vbobxb1wo.fsf@alter.siamese.dyndns.org","threadId":"32838","inReplyTo":"7vip65b25c.fsf@alter.siamese.dyndns.org","subject":"Re: Why is ident_is_sufficient different on Windows?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T20:52:39Z","receivedAt":"2013-02-06T20:52:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> I suspect somebody from the Windows camp saw a patch I posted\n>> without the ifdef, noticed that there is a problem to expect\n>> IDENT_NAME_GIVEN to be set on Windows for some reason, and resulted\n>> in a reroll of the function in that shape.\n>>\n>> I didn't find anything in the list archive, though.  So I am\n>> stumped.\n>\n> The only thing I can think of is that on Unix we can guess name from\n> GECOS, which could be considered sufficiently your name, while on\n> Windows we probably do not get anything useful there.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/137312/focus=137345\n\nThese days, we encourage setting user.name explicitly even on a\nsystem on which it is likely that we will see a good GECOS value, so\nremoving the ifdef and always check with ALL may not hurt anybody.\nI dunno.\n"}]}