{"thread":{"id":"42141","subject":"definition for _attribute() in remote.c","startedAt":"2016-04-25T21:02:38Z","lastAt":"2016-04-26T13:19:05Z","messageCount":8,"participants":["Philip Oakley","Jeff King","Ramsay Jones"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"284479","messageId":"D7C0C4062A7242B6912E56480CBB06F4@PhilipOakley","threadId":"42141","inReplyTo":null,"subject":"definition for _attribute() in remote.c","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-04-25T21:02:38Z","receivedAt":"2016-04-25T21:02:38Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi,\n\nI'm looking at getting Git for Windows to compile via Visual Studio \n(https://github.com/git-for-windows/git/pull/256).\n\nHowever the use of __attribute() in remote.c at L1662 \n(https://github.com/git-for-windows/git/blob/master/remote.c#L1662) has got \nme confused in that I can't see how the regular definition of __attribute() \nis #included in this case. A definition is given in \ngit\\compat\\regex\\regex_internal.h but doesn't appear to be on remote.c's \ninclude path.\n\nThe line was introduced by 3a429d0 (remote.c: report specific errors from \nbranch_get_upstream, 2015-05-21) which appears to be later than the previous \nMSVC testers had looked at.\n\nAny guidance on what is happening in this case would be welcomed as this is \nthe last compile error I'm seeing.\n\n--\n\nPhilip\n"},{"id":"284481","messageId":"20160425211030.GA10309@sigill.intra.peff.net","threadId":"42141","inReplyTo":"D7C0C4062A7242B6912E56480CBB06F4@PhilipOakley","subject":"Re: definition for _attribute() in remote.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-25T21:10:30Z","receivedAt":"2016-04-25T21:10:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 25, 2016 at 10:02:38PM +0100, Philip Oakley wrote:\n\n> I'm looking at getting Git for Windows to compile via Visual Studio\n> (https://github.com/git-for-windows/git/pull/256).\n> \n> However the use of __attribute() in remote.c at L1662\n> (https://github.com/git-for-windows/git/blob/master/remote.c#L1662) has got\n> me confused in that I can't see how the regular definition of __attribute()\n> is #included in this case. A definition is given in\n> git\\compat\\regex\\regex_internal.h but doesn't appear to be on remote.c's\n> include path.\n> \n> The line was introduced by 3a429d0 (remote.c: report specific errors from\n> branch_get_upstream, 2015-05-21) which appears to be later than the previous\n> MSVC testers had looked at.\n\nIt should be handled in git-compat-util.h, which is included by cache.h,\nwhich is included by remote.c.\n\nThere we have:\n\n #ifndef __GNUC__\n #ifndef __attribute__\n #define __attribute__(x)\n #endif\n #endif\n\nwhich should make it a noop on compilers which don't know about it. Is\nVS (or another file) setting __GNUC__?\n\n-Peff\n"},{"id":"284482","messageId":"20160425211523.GA11227@sigill.intra.peff.net","threadId":"42141","inReplyTo":"20160425211030.GA10309@sigill.intra.peff.net","subject":"[PATCH] remote.c: spell __attribute__ correctly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-25T21:15:23Z","receivedAt":"2016-04-25T21:15:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 25, 2016 at 05:10:30PM -0400, Jeff King wrote:\n\n> It should be handled in git-compat-util.h, which is included by cache.h,\n> which is included by remote.c.\n> \n> There we have:\n> \n> #ifndef __GNUC__\n> #ifndef __attribute__\n> #define __attribute__(x)\n> #endif\n> #endif\n> \n> which should make it a noop on compilers which don't know about it. Is\n> VS (or another file) setting __GNUC__?\n\nOf course it helps if we spell the name right...\n\n-- >8 --\nSubject: remote.c: spell __attribute__ correctly\n\nWe want to tell the compiler that error_buf() uses\nprintf()-style arguments via the __attribute__ mechanism,\nbut the original commit (3a429d0), forgot the trailing \"__\".\nThis happens to work with real GNUC-compatible compilers\nlike gcc and clang, but confuses our fallback macro in\ngit-compat-util.h, which only matches the official name (and\nthus the build fails on compilers like Visual Studio).\n\nReported-by: Philip Oakley \nSigned-off-by: Jeff King \n---\n remote.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/remote.c b/remote.c\nindex 28fd676..ddc4f8f 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1660,7 +1660,7 @@ int branch_merge_matches(struct branch *branch,\n \treturn refname_match(branch->merge[i]->src, refname);\n }\n \n-__attribute((format (printf,2,3)))\n+__attribute__((format (printf,2,3)))\n static const char *error_buf(struct strbuf *err, const char *fmt, ...)\n {\n \tif (err) {\n-- \n2.8.1.562.gc7e1b3c\n"},{"id":"284489","messageId":"F23050BD4C62408D97A64B1D885007A8@PhilipOakley","threadId":"42141","inReplyTo":"20160425211030.GA10309@sigill.intra.peff.net","subject":"Re: definition for _attribute() in remote.c","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-04-25T21:34:27Z","receivedAt":"2016-04-25T21:34:27Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Jeff King\" \n> On Mon, Apr 25, 2016 at 10:02:38PM +0100, Philip Oakley wrote:\n>\n>> I'm looking at getting Git for Windows to compile via Visual Studio\n>> (https://github.com/git-for-windows/git/pull/256).\n>>\n>> However the use of __attribute() in remote.c at L1662\n>> (https://github.com/git-for-windows/git/blob/master/remote.c#L1662) has \n>> got\n>> me confused in that I can't see how the regular definition of \n>> __attribute()\n>> is #included in this case. A definition is given in\n>> git\\compat\\regex\\regex_internal.h but doesn't appear to be on remote.c's\n>> include path.\n>>\n>> The line was introduced by 3a429d0 (remote.c: report specific errors from\n>> branch_get_upstream, 2015-05-21) which appears to be later than the \n>> previous\n>> MSVC testers had looked at.\n>\n> It should be handled in git-compat-util.h, which is included by cache.h,\n> which is included by remote.c.\n>\n> There we have:\n>\n> #ifndef __GNUC__\n> #ifndef __attribute__\n> #define __attribute__(x)\n> #endif\n> #endif\n>\n> which should make it a noop on compilers which don't know about it. Is\n> VS (or another file) setting __GNUC__?\n\nIt's not the __attribute__ definition (a Gnu C ism), rather its the \n__attribute variant, which has a definition in regex_internal.h, and is used \nin the regex code. It's that one that's used in remote.c that I can't fathom \n(i.e. how it worked in normally)\n\nregex_internal.h#L160-164\n#ifdef __GNUC__\n# define __attribute(arg) __attribute__ (arg)\n#else\n# define __attribute(arg)\n#endif\n\nthus when the compilation get to remote.c#L1662 it fails to find that \ndefinition.\n\nShould that line use the gnu extension name?\n\n--\nPhilip \n"},{"id":"284491","messageId":"20160425213926.GC11227@sigill.intra.peff.net","threadId":"42141","inReplyTo":"F23050BD4C62408D97A64B1D885007A8@PhilipOakley","subject":"Re: definition for _attribute() in remote.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-25T21:39:26Z","receivedAt":"2016-04-25T21:39:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 25, 2016 at 10:34:27PM +0100, Philip Oakley wrote:\n\n> It's not the __attribute__ definition (a Gnu C ism), rather its the\n> __attribute variant, which has a definition in regex_internal.h, and is used\n> in the regex code. It's that one that's used in remote.c that I can't fathom\n> (i.e. how it worked in normally)\n> \n> regex_internal.h#L160-164\n> #ifdef __GNUC__\n> # define __attribute(arg) __attribute__ (arg)\n> #else\n> # define __attribute(arg)\n> #endif\n> \n> thus when the compilation get to remote.c#L1662 it fails to find that\n> definition.\n> \n> Should that line use the gnu extension name?\n\nYeah, sorry, see my followup response. We don't use \"__attribute\" at\nall ourselves. The reference you found is in compat code that we pulled\nin (so it does its own thing entirely), and the one in remote.c is just\na typo/think-o that happened to work on gcc, et al.\n\n-Peff\n"},{"id":"284495","messageId":"2FDBFACB68254498A8F83367553AED80@PhilipOakley","threadId":"42141","inReplyTo":"20160425211523.GA11227@sigill.intra.peff.net","subject":"Re: [PATCH] remote.c: spell __attribute__ correctly","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-04-25T21:50:53Z","receivedAt":"2016-04-25T21:50:53Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Jeff King\" \n> On Mon, Apr 25, 2016 at 05:10:30PM -0400, Jeff King wrote:\n>\n>> It should be handled in git-compat-util.h, which is included by cache.h,\n>> which is included by remote.c.\n>>\n>> There we have:\n>>\n>> #ifndef __GNUC__\n>> #ifndef __attribute__\n>> #define __attribute__(x)\n>> #endif\n>> #endif\n>>\n>> which should make it a noop on compilers which don't know about it. Is\n>> VS (or another file) setting __GNUC__?\n>\n> Of course it helps if we spell the name right...\n>\n> -- >8 --\n> Subject: remote.c: spell __attribute__ correctly\n>\n> We want to tell the compiler that error_buf() uses\n> printf()-style arguments via the __attribute__ mechanism,\n> but the original commit (3a429d0), forgot the trailing \"__\".\n> This happens to work with real GNUC-compatible compilers\n> like gcc and clang, but confuses our fallback macro in\n> git-compat-util.h, which only matches the official name (and\n> thus the build fails on compilers like Visual Studio).\n>\n> Reported-by: Philip Oakley \n> Signed-off-by: Jeff King \n> ---\n> remote.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/remote.c b/remote.c\n> index 28fd676..ddc4f8f 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -1660,7 +1660,7 @@ int branch_merge_matches(struct branch *branch,\n> return refname_match(branch->merge[i]->src, refname);\n> }\n>\n> -__attribute((format (printf,2,3)))\n> +__attribute__((format (printf,2,3)))\n> static const char *error_buf(struct strbuf *err, const char *fmt, ...)\n> {\n> if (err) {\n> -- \n\nThanks for clarifying that (sorry about the crossed emails). The compile is \nnow looking good.\nI'm just left with some unresolved external symbol link errors now.\n\nThe same naming issue in compat/regex/regcomp.c, compat/regex/regexec.c, \ncompat/regex/regex_internal.c and compat/regex/regex_internal.h was \nprobably what lead me astray...\n\nPhilip\n"},{"id":"284500","messageId":"571E96CE.80606@ramsayjones.plus.com","threadId":"42141","inReplyTo":"2FDBFACB68254498A8F83367553AED80@PhilipOakley","subject":"Re: [PATCH] remote.c: spell __attribute__ correctly","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-04-25T22:14:38Z","receivedAt":"2016-04-25T22:14:38Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 25/04/16 22:50, Philip Oakley wrote:\n> From: \"Jeff King\" \n>> On Mon, Apr 25, 2016 at 05:10:30PM -0400, Jeff King wrote:\n>>\n>>> It should be handled in git-compat-util.h, which is included by cache.h,\n>>> which is included by remote.c.\n>>>\n>>> There we have:\n>>>\n>>> #ifndef __GNUC__\n>>> #ifndef __attribute__\n>>> #define __attribute__(x)\n>>> #endif\n>>> #endif\n>>>\n>>> which should make it a noop on compilers which don't know about it. Is\n>>> VS (or another file) setting __GNUC__?\n>>\n>> Of course it helps if we spell the name right...\n\nIndeed! ;-)\n\nNot that it matters, but the above #define in git-compat-util.h is not\nthe relevant definition - msvc will not see it. However, it does see\nthe #define on line 12 of compat/msvc.h. :-D\n\nATB,\nRamsay Jones\n"},{"id":"284583","messageId":"05BBD86AE63E4FA0ADE63F05DBD834C0@PhilipOakley","threadId":"42141","inReplyTo":"571E96CE.80606@ramsayjones.plus.com","subject":"Re: [PATCH] remote.c: spell __attribute__ correctly","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-04-26T13:19:05Z","receivedAt":"2016-04-26T13:19:05Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Thnx,\nFrom: \"Ramsay Jones\" \n> On 25/04/16 22:50, Philip Oakley wrote:\n>> From: \"Jeff King\" \n>>> On Mon, Apr 25, 2016 at 05:10:30PM -0400, Jeff King wrote:\n>>>\n>>>> It should be handled in git-compat-util.h, which is included by \n>>>> cache.h,\n>>>> which is included by remote.c.\n>>>>\n>>>> There we have:\n>>>>\n>>>> #ifndef __GNUC__\n>>>> #ifndef __attribute__\n>>>> #define __attribute__(x)\n>>>> #endif\n>>>> #endif\n>>>>\n>>>> which should make it a noop on compilers which don't know about it. Is\n>>>> VS (or another file) setting __GNUC__?\n>>>\n>>> Of course it helps if we spell the name right...\n>\n> Indeed! ;-)\n>\n> Not that it matters, but the above #define in git-compat-util.h is not\n> the relevant definition - msvc will not see it.\n\nAh, I see that that block is further guarded with other if/elif/else clauses \nso that it's not seen if _MSC_VER is defined.\n\ngit-compat-util.h#L400-411\n\n> However, it does see\n> the #define on line 12 of compat/msvc.h. :-D\n>\n> ATB,\n> Ramsay Jones\n>\n"}]}