{"thread":{"id":"49031","subject":"[PATCH] Makefile: enable DEVELOPER by default","startedAt":"2018-08-04T02:00:16Z","lastAt":"2018-09-01T21:06:37Z","messageCount":26,"participants":["Stefan Beller","Jonathan Nieder","Duy Nguyen","Junio C Hamano","Eric Sunshine","Ævar Arnfjörð Bjarmason","Jeff King","Randall S. Becker","Kaartic Sivaraam"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"354472","messageId":"20180804020009.224582-1-sbeller@google.com","threadId":"49031","inReplyTo":null,"subject":"[PATCH] Makefile: enable DEVELOPER by default","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-04T02:00:09Z","receivedAt":"2018-08-04T02:00:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Reviewer bandwidth is limited, so let's have the machine of the\n(potentially new) contributor warn about issues with the code by default.\n\nAs setting DEVELOPER, the compiler is stricter and we may run into problems\non some architectures. But packagers of said platforms are knowledgeable\nenough to turn off this flag. (Also they are fewer than the number of new\ncontributors)\n\nSigned-off-by: Stefan Beller \n---\n Makefile | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 41b93689add..95aa3ff3185 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -497,6 +497,8 @@ ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n STRIP ?= strip\n \n+DEVELOPER=1\n+\n # Create as necessary, replace existing, make ranlib unneeded.\n ARFLAGS = rcs\n \n-- \n2.18.0.597.ga71716f1ad-goog\n\n"},{"id":"354473","messageId":"20180804020255.225573-1-sbeller@google.com","threadId":"49031","inReplyTo":"20180804020009.224582-1-sbeller@google.com","subject":"[PATCH] Makefile: enable DEVELOPER by default","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-04T02:02:55Z","receivedAt":"2018-08-04T02:03:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Reviewer bandwidth is limited, so let's have the machine of the\n(potentially new) contributor warn about issues with the code by default.\n\nAs setting DEVELOPER, the compiler is stricter and we may run into problems\non some architectures. But packagers of said platforms are knowledgeable\nenough to turn off this flag. (Also they are fewer than the number of new\ncontributors)\n\nSigned-off-by: Stefan Beller \n---\n\nResending with cc'd.\n\n Makefile | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 41b93689add..95aa3ff3185 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -497,6 +497,8 @@ ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n STRIP ?= strip\n \n+DEVELOPER=1\n+\n # Create as necessary, replace existing, make ranlib unneeded.\n ARFLAGS = rcs\n \n-- \n2.18.0.597.ga71716f1ad-goog\n\n"},{"id":"354480","messageId":"20180804060928.GB55869@aiede.svl.corp.google.com","threadId":"49031","inReplyTo":"20180804020255.225573-1-sbeller@google.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-04T06:09:28Z","receivedAt":"2018-08-04T06:09:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nStefan Beller wrote:\n\n> Reviewer bandwidth is limited, so let's have the machine of the\n> (potentially new) contributor warn about issues with the code by default.\n>\n> As setting DEVELOPER, the compiler is stricter and we may run into problems\n> on some architectures. But packagers of said platforms are knowledgeable\n> enough to turn off this flag. (Also they are fewer than the number of new\n> contributors)\n\nWhich architectures would we run into problems on? Aren't those bugs\nthat should themselves be fixed?\n\nI think you are right that the packagers will cope with whatever\nsetting we choose. My main concern is not about them but about other\npeople building from source in order to run (instead of to develop)\nGit, and by extension, the people they go to for help when it doesn't\nwork. I have lots of bitter experience of -Werror being a support\nheadache and leading to bad workarounds when someone upgrades their\ncompiler and the build starts failing due to a new warning it has\nintroduced.\n\n> Signed-off-by: Stefan Beller \n> ---\n> Makefile | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/Makefile b/Makefile\n> index 41b93689add..95aa3ff3185 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -497,6 +497,8 @@ ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n> ALL_LDFLAGS = $(LDFLAGS)\n> STRIP ?= strip\n> \n> +DEVELOPER=1\n\nI like the idea of making this more discoverable to new contributors.\nIt seems that Documentation/SubmittingPatches doesn't mention this\nsetting. Should it?\n\nShould a non-DEVELOPER build print a note encouraging enabling this\nsetting in case you're developing patches meant for submission to the\nproject?\n\nShould we have a CONTRIBUTING.md file suggesting this setting? Other\nideas for ensuring it's enabled for those who need it?\n\nThanks and hope that helps,\nJonathan\n"},{"id":"354483","messageId":"CACsJy8DxSDLD7B8Z+GBFOuU7d7VQ4-M=BP=wptra5rBiZGspSQ@mail.gmail.com","threadId":"49031","inReplyTo":"20180804060928.GB55869@aiede.svl.corp.google.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-08-04T06:38:14Z","receivedAt":"2018-08-04T06:38:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Aug 4, 2018 at 8:11 AM Jonathan Nieder wrote:\n> My main concern is not about them but about other\n> people building from source in order to run (instead of to develop)\n> Git, and by extension, the people they go to for help when it doesn't\n> work. I have lots of bitter experience of -Werror being a support\n> headache and leading to bad workarounds when someone upgrades their\n> compiler and the build starts failing due to a new warning it has\n> introduced.\n\nEven old compilers can also throw some silly, false positive warnings\n(which now turn into errors) because they are not as smart as new\nones.\n-- \nDuy\n"},{"id":"354497","messageId":"xmqqo9eirqwp.fsf@gitster-ct.c.googlers.com","threadId":"49031","inReplyTo":"CACsJy8DxSDLD7B8Z+GBFOuU7d7VQ4-M=BP=wptra5rBiZGspSQ@mail.gmail.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-04T17:19:02Z","receivedAt":"2018-08-04T17:19:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Sat, Aug 4, 2018 at 8:11 AM Jonathan Nieder wrote:\n>> My main concern is not about them but about other\n>> people building from source in order to run (instead of to develop)\n>> Git, and by extension, the people they go to for help when it doesn't\n>> work. I have lots of bitter experience of -Werror being a support\n>> headache and leading to bad workarounds when someone upgrades their\n>> compiler and the build starts failing due to a new warning it has\n>> introduced.\n>\n> Even old compilers can also throw some silly, false positive warnings\n> (which now turn into errors) because they are not as smart as new\n> ones.\n\nI agree with both of the above. I do not think the pros-and-cons\nare in favor of forcing the developer bit to everybody, even though\nI am sympathetic to the desire to see people throw fewer bad changes\nthat waste review bandwidth by not compiling or passing its own\ntests at us.\n\n"},{"id":"354519","messageId":"CAPig+cRA87UZsynme-by+s2ZmQW2Aus9KQscCU9mXmALCBKkKQ@mail.gmail.com","threadId":"49031","inReplyTo":"CACsJy8DxSDLD7B8Z+GBFOuU7d7VQ4-M=BP=wptra5rBiZGspSQ@mail.gmail.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-05T02:42:34Z","receivedAt":"2018-08-05T02:42:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Aug 4, 2018 at 2:38 AM Duy Nguyen wrote:\n> On Sat, Aug 4, 2018 at 8:11 AM Jonathan Nieder wrote:\n> > My main concern is not about them but about other\n> > people building from source in order to run (instead of to develop)\n> > Git, and by extension, the people they go to for help when it doesn't\n> > work. I have lots of bitter experience of -Werror being a support\n> > headache and leading to bad workarounds when someone upgrades their\n> > compiler and the build starts failing due to a new warning it has\n> > introduced.\n>\n> Even old compilers can also throw some silly, false positive warnings\n> (which now turn into errors) because they are not as smart as new\n> ones.\n\nAnd, compilation warnings are not limited to old compilers. Even my\nfully up-to-date FreeBSD 11.2 installation is not warning-free[1].\n\n[1]: For instance:\nutf8.c:486:28: warning: passing 'iconv_ibp *' (aka 'const char **') to parameter\n of type 'char **' discards qualifiers in nested pointer types\n [-Wincompatible-pointer-types-discards-qualifiers]\n"},{"id":"354520","messageId":"20180805031709.GF258270@aiede.svl.corp.google.com","threadId":"49031","inReplyTo":"CAPig+cRA87UZsynme-by+s2ZmQW2Aus9KQscCU9mXmALCBKkKQ@mail.gmail.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-05T03:17:09Z","receivedAt":"2018-08-05T03:17:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Eric Sunshine wrote:\n\n> And, compilation warnings are not limited to old compilers. Even my\n> fully up-to-date FreeBSD 11.2 installation is not warning-free[1].\n>\n> [1]: For instance:\n> utf8.c:486:28: warning: passing 'iconv_ibp *' (aka 'const char **') to parameter\n> of type 'char **' discards qualifiers in nested pointer types\n> [-Wincompatible-pointer-types-discards-qualifiers]\n\nOh, good catch! POSIX documents iconv has having signature\n\n\tsize_t iconv(iconv_t cd, char **restrict inbuf,\n\t size_t *restrict inbytesleft, char **restrict outbuf,\n\t size_t *restrict outbytesleft);\n\nThe Makefile explains\n\n\t# Define OLD_ICONV if your library has an old iconv(), where the second\n\t# (input buffer pointer) parameter is declared with type (const char **).\n\nwhich is implemented as\n\n\t#if defined(OLD_ICONV) || (defined(__sun__) && !defined(_XPG6))\n\t\ttypedef const char * iconv_ibp;\n\t#else\n\t\ttypedef char * iconv_ibp;\n\t#endif\n\nconfig.mak.uname contains\n\n\tifeq ($(uname_S),FreeBSD)\n\t\tNEEDS_LIBICONV = YesPlease\n\t\tOLD_ICONV = YesPlease\n\nSo it looks like FreeBSD has modernized and we need to make that\nconditional in config.mak.uname on $(uname_R). Do you know which\nversion of FreeBSD changed the signature? Care to write a patch?\n\nThanks,\nJonathan\n"},{"id":"354522","messageId":"CAPig+cRjxLgGZbROZAuH-VF3xLVUxQTRj7gKPFurypbwz2zzjg@mail.gmail.com","threadId":"49031","inReplyTo":"20180805031709.GF258270@aiede.svl.corp.google.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-05T03:33:42Z","receivedAt":"2018-08-05T03:33:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Aug 4, 2018 at 11:17 PM Jonathan Nieder wrote:\n> > utf8.c:486:28: warning: passing 'iconv_ibp *' (aka 'const char **') to parameter\n> > of type 'char **' discards qualifiers in nested pointer types\n> > [-Wincompatible-pointer-types-discards-qualifiers]\n>\n> Oh, good catch! POSIX documents iconv has having signature\n>\n> size_t iconv(iconv_t cd, char **restrict inbuf,\n> size_t *restrict inbytesleft, char **restrict outbuf,\n> size_t *restrict outbytesleft);\n>\n> config.mak.uname contains\n>\n> ifeq ($(uname_S),FreeBSD)\n> NEEDS_LIBICONV = YesPlease\n> OLD_ICONV = YesPlease\n>\n> So it looks like FreeBSD has modernized and we need to make that\n> conditional in config.mak.uname on $(uname_R). Do you know which\n> version of FreeBSD changed the signature? Care to write a patch?\n\nUnfortunately, I don't know in which version of FreeBSD that changed.\nI rarely fire up that virtual machine (only in rare cases when I want\nto verify some change to Git also builds/runs/whatever on FreeBSD), so\nI haven't really been paying attention to it. I know that this warning\nwas present in 11.1 (and I'm guessing all of 11.x), but I don't recall\nif it manifested in 10.x. I guess it shouldn't be too hard to install\nvarious versions of FreeBSD to determine this, but it would be quite\ntime-consuming. I'm not very familiar with FreeBSD-land, but I would\nhope there would be an easier way to determine when it changed than by\ninstalling old versions. Does FreeBSD have historic package\nrepositories (containing headers, for instance) which one could\nconsult instead?\n"},{"id":"354527","messageId":"CAPig+cTEGtsmUyoYsKEx+erMsXKm5=c6TJuNAgeky2pcgw18JQ@mail.gmail.com","threadId":"49031","inReplyTo":"CAPig+cRjxLgGZbROZAuH-VF3xLVUxQTRj7gKPFurypbwz2zzjg@mail.gmail.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-05T04:58:32Z","receivedAt":"2018-08-05T04:58:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Aug 4, 2018 at 11:33 PM Eric Sunshine wrote:\n> On Sat, Aug 4, 2018 at 11:17 PM Jonathan Nieder wrote:\n> > So it looks like FreeBSD has modernized and we need to make that\n> > conditional in config.mak.uname on $(uname_R). Do you know which\n> > version of FreeBSD changed the signature? Care to write a patch?\n>\n> I'm not very familiar with FreeBSD-land, but I would\n> hope there would be an easier way to determine when it changed than by\n> installing old versions. Does FreeBSD have historic package\n> repositories (containing headers, for instance) which one could\n> consult instead?\n\nI thought perhaps we could figure out the timeframe by looking at the\nGit package[1] in the FreeBSD ports tree to see when they added,\nremoved, or changed a patch file for config.mak.uname, but that didn't\npan out since they invoke the Git 'configure' script which determines\nOLD_ICONV automatically.\n\n[1]: https://github.com/freebsd/freebsd-ports/tree/master/devel/git\n"},{"id":"354537","messageId":"20180805075736.GF44140@aiede.svl.corp.google.com","threadId":"49031","inReplyTo":"CAPig+cRjxLgGZbROZAuH-VF3xLVUxQTRj7gKPFurypbwz2zzjg@mail.gmail.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-05T07:57:36Z","receivedAt":"2018-08-05T07:57:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nEric Sunshine wrote:\n> On Sat, Aug 4, 2018 at 11:17 PM Jonathan Nieder wrote:\n\n>>> utf8.c:486:28: warning: passing 'iconv_ibp *' (aka 'const char **') to parameter\n>>> of type 'char **' discards qualifiers in nested pointer types\n>>> [-Wincompatible-pointer-types-discards-qualifiers]\n>>\n>> Oh, good catch! POSIX documents iconv has having signature\n>>\n>> size_t iconv(iconv_t cd, char **restrict inbuf,\n>> size_t *restrict inbytesleft, char **restrict outbuf,\n>> size_t *restrict outbytesleft);\n>>\n>> config.mak.uname contains\n>>\n>> ifeq ($(uname_S),FreeBSD)\n>> NEEDS_LIBICONV = YesPlease\n>> OLD_ICONV = YesPlease\n>>\n>> So it looks like FreeBSD has modernized and we need to make that\n>> conditional in config.mak.uname on $(uname_R). Do you know which\n>> version of FreeBSD changed the signature? Care to write a patch?\n>\n> Unfortunately, I don't know in which version of FreeBSD that changed.\n\n\"git blame\" tells me it's from r281550[1] (Remove the const qualifier\nfrom iconv(3) to comply with POSIX, 2015-04-15), which was part of\nFreeBSD 11. r282275[2] (2015-04-30) backported the same change to\nFreeBSD 10 and is part of 10.2.\n\nFreeBSD 9 has\n\n\t#define iconv(cd, in, insize, out, outsize) libiconv(cd, __DECONST(char **, in), insize, out, outsize)\n\nfrom r219019[3] (2011-02-25). I don't know what to make of it. The\nunderlying libiconv function it calls takes char ** (the modern thing)\nbut the macro does the __DECONST thing for compatibility with GNU\nlibiconv.\n\nOlder versions, going back at least as far as FreeBSD 3, behave the\nsame way as FreeBSD 9. So that must be what was tested with\nOLD_ICONV=YesPlease.\n\nFreeBSD 10.1.0 and 10.0.0 have\n\n\tsize_t iconv(iconv_t, const char ** __restrict,\n\t\t size_t * __restrict, char ** __restrict,\n\t\t size_t * __restrict);\n\nwhich also needs OLD_ICONV. If I assume everyone on 10.x is using\n10.2 or newer, then the patch could be something like this (completely\nuntested):\n\ndiff --git i/config.mak.uname w/config.mak.uname\nindex 684fc5bf026..8078c099313 100644\n--- i/config.mak.uname\n+++ w/config.mak.uname\n@@ -192,7 +192,9 @@ ifeq ($(uname_O),Cygwin)\n endif\n ifeq ($(uname_S),FreeBSD)\n \tNEEDS_LIBICONV = YesPlease\n-\tOLD_ICONV = YesPlease\n+\tifeq ($(shell expr \"$(uname_R)\" : '[1-9]\\.'),2)\n+\t\tOLD_ICONV = YesPlease\n+\tendif\n \tNO_MEMMEM = YesPlease\n \tBASIC_CFLAGS += -I/usr/local/include\n \tBASIC_LDFLAGS += -L/usr/local/lib\n\nDeciding whether to fix the pattern matching to also include 10.0 and\n10.1 (and doing so if warranted) is left as an exercise to the reader.\n\nSigned-off-by: Jonathan Nieder \n\n[1] https://github.com/freebsd/freebsd/commit/b0813ee288f64f677a2cebf7815754b027a8215b\n[2] https://github.com/freebsd/freebsd/commit/b709ec868adb5170d09bc5a66b18d0e0d5987ab6\n[3] https://github.com/freebsd/freebsd/commit/c91ab1769b1237e3663d59888cebe31ceee47570\n"},{"id":"354641","messageId":"87zhxzsb2p.fsf@evledraar.gmail.com","threadId":"49031","inReplyTo":"xmqqo9eirqwp.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-06T16:40:14Z","receivedAt":"2018-08-06T16:40:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Aug 04 2018, Junio C Hamano wrote:\n\n> Duy Nguyen writes:\n>\n>> On Sat, Aug 4, 2018 at 8:11 AM Jonathan Nieder wrote:\n>>> My main concern is not about them but about other\n>>> people building from source in order to run (instead of to develop)\n>>> Git, and by extension, the people they go to for help when it doesn't\n>>> work. I have lots of bitter experience of -Werror being a support\n>>> headache and leading to bad workarounds when someone upgrades their\n>>> compiler and the build starts failing due to a new warning it has\n>>> introduced.\n>>\n>> Even old compilers can also throw some silly, false positive warnings\n>> (which now turn into errors) because they are not as smart as new\n>> ones.\n>\n> I agree with both of the above. I do not think the pros-and-cons\n> are in favor of forcing the developer bit to everybody, even though\n> I am sympathetic to the desire to see people throw fewer bad changes\n> that waste review bandwidth by not compiling or passing its own\n> tests at us.\n\nI agree.\n\nResponding to the thread in general, perhaps people would like this more\nif we turned DEVELOPER=1 DEVOPTS=no-error on by default?\n\nThat's basically why I added it in 99f763baf5 (\"Makefile: add a DEVOPTS\nto suppress -Werror under DEVELOPER\", 2018-04-14), because I wanted the\nabilty to have verbose informative output without the build dying on\nsome older systems / compilers.\n\nIt's fine and understandable if you're someone who's just building a\npackage on some older system if you get a bunch of compiler warnings,\nbut more annoying if you have to dig into how to disable a default\n-Werror.\n"},{"id":"354645","messageId":"20180806170211.GD19053@sigill.intra.peff.net","threadId":"49031","inReplyTo":"87zhxzsb2p.fsf@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-06T17:02:11Z","receivedAt":"2018-08-06T17:02:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 06, 2018 at 06:40:14PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Responding to the thread in general, perhaps people would like this more\n> if we turned DEVELOPER=1 DEVOPTS=no-error on by default?\n> \n> That's basically why I added it in 99f763baf5 (\"Makefile: add a DEVOPTS\n> to suppress -Werror under DEVELOPER\", 2018-04-14), because I wanted the\n> abilty to have verbose informative output without the build dying on\n> some older systems / compilers.\n> \n> It's fine and understandable if you're someone who's just building a\n> package on some older system if you get a bunch of compiler warnings,\n> but more annoying if you have to dig into how to disable a default\n> -Werror.\n\nI had the impression that DEVELOPER=1 was allowed to set flags that old\nversions might not even know about. Hence they might actually barf, even\nwithout -Werror. Maybe that's better since the introduction of the\ndetect-compiler script, though.\n\nI do think we may have a skewed view of the population on this list.\nWe're developers ourselves, and we interact with new developers that we\nwant to help. But there are masses of people[1] building Git who are\n_not_ developers, and want the default to be as robust as possible.\nThey're probably not going to show up in this thread.\n\n-Peff\n\n[1] I actually wonder how large that mass is. Clearly there are many\n orders of magnitude more users than there are developers. But I have\n no idea what percentage of them build from source versus using\n somebody else's binary package.\n"},{"id":"354646","messageId":"001001d42da7$4a9e6570$dfdb3050$@nexbridge.com","threadId":"49031","inReplyTo":"87zhxzsb2p.fsf@evledraar.gmail.com","subject":"RE: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-08-06T17:02:37Z","receivedAt":"2018-08-06T17:02:49Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On August 6, 2018 12:40 PM, Ævar Arnfjörð Bjarmason wrote:\n> On Sat, Aug 04 2018, Junio C Hamano wrote:\n> \n> > Duy Nguyen writes:\n> >\n> >> On Sat, Aug 4, 2018 at 8:11 AM Jonathan Nieder \n> wrote:\n> >>> My main concern is not about them but about other people building\n> >>> from source in order to run (instead of to develop) Git, and by\n> >>> extension, the people they go to for help when it doesn't work. I\n> >>> have lots of bitter experience of -Werror being a support headache\n> >>> and leading to bad workarounds when someone upgrades their compiler\n> >>> and the build starts failing due to a new warning it has introduced.\n> >>\n> >> Even old compilers can also throw some silly, false positive warnings\n> >> (which now turn into errors) because they are not as smart as new\n> >> ones.\n> >\n> > I agree with both of the above. I do not think the pros-and-cons are\n> > in favor of forcing the developer bit to everybody, even though I am\n> > sympathetic to the desire to see people throw fewer bad changes that\n> > waste review bandwidth by not compiling or passing its own tests at\n> > us.\n> \n> I agree.\n> \n> Responding to the thread in general, perhaps people would like this more if\n> we turned DEVELOPER=1 DEVOPTS=no-error on by default?\n> \n> That's basically why I added it in 99f763baf5 (\"Makefile: add a DEVOPTS to\n> suppress -Werror under DEVELOPER\", 2018-04-14), because I wanted the\n> abilty to have verbose informative output without the build dying on some\n> older systems / compilers.\n> \n> It's fine and understandable if you're someone who's just building a package\n> on some older system if you get a bunch of compiler warnings, but more\n> annoying if you have to dig into how to disable a default -Werror.\n\nAny idea when this is going to be in an official release, and exactly what the settings will be for \"Not Developer\". I assume DEVELOPER=0 and DEVOPTS=error, which is the current behaviour, correct? I am the platform maintainer for HPE NonStop and need to make sure I'm not packaging DEV builds to anyone, since I'm the only one doing this for the platform. It's another hoop, but hopefully not a bad one. The question is the best place to set this, assuming we are using Jenkins for our builds, and I'd rather keep the existing config.mak.uname the same, since at least it seems stable. We currently just run \"make\" for our build. So make arguments? And is making the change now non-destructive in preparation so that I don't forget when the time comes?\n\nCheers,\nRandall\n\n-- Brief whoami:\n NonStop developer since approximately 211288444200000000\n UNIX developer since approximately 421664400\n-- In my real life, I talk too much.\n\n\n\n"},{"id":"354647","messageId":"001101d42da7$8128a560$8379f020$@nexbridge.com","threadId":"49031","inReplyTo":"20180806170211.GD19053@sigill.intra.peff.net","subject":"RE: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-08-06T17:04:09Z","receivedAt":"2018-08-06T17:04:18Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On August 6, 2018 1:02 PM, Peff wrote:\n> To: Ævar Arnfjörð Bjarmason \n> Cc: Junio C Hamano ; Duy Nguyen\n> ; Jonathan Nieder ; Stefan\n> Beller ; Git Mailing List ; git-\n> packagers@googlegroups.com; Han-Wen Nienhuys \n> Subject: Re: [PATCH] Makefile: enable DEVELOPER by default\n> \n> On Mon, Aug 06, 2018 at 06:40:14PM +0200, Ævar Arnfjörð Bjarmason\n> wrote:\n> \n> > Responding to the thread in general, perhaps people would like this\n> > more if we turned DEVELOPER=1 DEVOPTS=no-error on by default?\n> >\n> > That's basically why I added it in 99f763baf5 (\"Makefile: add a\n> > DEVOPTS to suppress -Werror under DEVELOPER\", 2018-04-14), because I\n> > wanted the abilty to have verbose informative output without the build\n> > dying on some older systems / compilers.\n> >\n> > It's fine and understandable if you're someone who's just building a\n> > package on some older system if you get a bunch of compiler warnings,\n> > but more annoying if you have to dig into how to disable a default\n> > -Werror.\n> \n> I had the impression that DEVELOPER=1 was allowed to set flags that old\n> versions might not even know about. Hence they might actually barf, even\n> without -Werror. Maybe that's better since the introduction of the detect-\n> compiler script, though.\n> \n> I do think we may have a skewed view of the population on this list.\n> We're developers ourselves, and we interact with new developers that we\n> want to help. But there are masses of people[1] building Git who are _not_\n> developers, and want the default to be as robust as possible.\n> They're probably not going to show up in this thread.\n> \n> -Peff\n> \n> [1] I actually wonder how large that mass is. Clearly there are many\n> orders of magnitude more users than there are developers. But I have\n> no idea what percentage of them build from source versus using\n> somebody else's binary package.\n\nOne? 😉\n\nJokingly,\nRandall\n\n-- Brief whoami:\n NonStop developer since approximately 211288444200000000\n UNIX developer since approximately 421664400\n-- In my real life, I talk too much.\n\n\n\n"},{"id":"354648","messageId":"20180806171119.GC120433@aiede.svl.corp.google.com","threadId":"49031","inReplyTo":"20180806170211.GD19053@sigill.intra.peff.net","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-06T17:11:19Z","receivedAt":"2018-08-06T17:11:24Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJeff King wrote:\n\n> I had the impression that DEVELOPER=1 was allowed to set flags that old\n> versions might not even know about. Hence they might actually barf, even\n> without -Werror.\n\nYes.\n\n[...]\n> We're developers ourselves, and we interact with new developers that we\n> want to help. But there are masses of people[1] building Git who are\n> _not_ developers, and want the default to be as robust as possible.\n> They're probably not going to show up in this thread.\n>\n> -Peff\n>\n> [1] I actually wonder how large that mass is. Clearly there are many\n> orders of magnitude more users than there are developers. But I have\n> no idea what percentage of them build from source versus using\n> somebody else's binary package.\n\nRelatedly, we need to think about the incentives these defaults\ncreate. Personally, I want *more* naive users to be building from\nsource, because then they are better able to test recent versions,\nbisect, test my patches, etc.\n\nAs I hinted in my earlier reply, I think it would be best to try some\nbasic things to make DEVELOPER more visible first. If that fails,\nthen we can revisit how to make this more drastic change in a way that\nminimizes the harm (and I am not sure yet that that is possible).\n\nThanks,\nJonathan\n"},{"id":"354652","messageId":"87y3djs8bs.fsf@evledraar.gmail.com","threadId":"49031","inReplyTo":"20180806170211.GD19053@sigill.intra.peff.net","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-06T17:39:35Z","receivedAt":"2018-08-06T17:39:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Aug 06 2018, Jeff King wrote:\n\n> On Mon, Aug 06, 2018 at 06:40:14PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Responding to the thread in general, perhaps people would like this more\n>> if we turned DEVELOPER=1 DEVOPTS=no-error on by default?\n>>\n>> That's basically why I added it in 99f763baf5 (\"Makefile: add a DEVOPTS\n>> to suppress -Werror under DEVELOPER\", 2018-04-14), because I wanted the\n>> abilty to have verbose informative output without the build dying on\n>> some older systems / compilers.\n>>\n>> It's fine and understandable if you're someone who's just building a\n>> package on some older system if you get a bunch of compiler warnings,\n>> but more annoying if you have to dig into how to disable a default\n>> -Werror.\n>\n> I had the impression that DEVELOPER=1 was allowed to set flags that old\n> versions might not even know about. Hence they might actually barf, even\n> without -Werror. Maybe that's better since the introduction of the\n> detect-compiler script, though.\n\nI misrecalled that -Wimadethis-up wouldn't error on e.g. GCC today in\nthe interest of forward-compatibility, but it does. So that changes\nthings.\n\nAlthough we could today pick some set of flags greater than what we use\ntoday if there's interest, and enough compiler compatibility.\n\n> I do think we may have a skewed view of the population on this list.\n> We're developers ourselves, and we interact with new developers that we\n> want to help. But there are masses of people[1] building Git who are\n> _not_ developers, and want the default to be as robust as possible.\n> They're probably not going to show up in this thread.\n\nIndeed.\n"},{"id":"354653","messageId":"87wot3s87t.fsf@evledraar.gmail.com","threadId":"49031","inReplyTo":"001001d42da7$4a9e6570$dfdb3050$@nexbridge.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-06T17:41:58Z","receivedAt":"2018-08-06T17:42:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Aug 06 2018, Randall S. Becker wrote:\n\n> On August 6, 2018 12:40 PM, Ævar Arnfjörð Bjarmason wrote:\n>> On Sat, Aug 04 2018, Junio C Hamano wrote:\n>>\n>> > Duy Nguyen writes:\n>> >\n>> >> On Sat, Aug 4, 2018 at 8:11 AM Jonathan Nieder \n>> wrote:\n>> >>> My main concern is not about them but about other people building\n>> >>> from source in order to run (instead of to develop) Git, and by\n>> >>> extension, the people they go to for help when it doesn't work. I\n>> >>> have lots of bitter experience of -Werror being a support headache\n>> >>> and leading to bad workarounds when someone upgrades their compiler\n>> >>> and the build starts failing due to a new warning it has introduced.\n>> >>\n>> >> Even old compilers can also throw some silly, false positive warnings\n>> >> (which now turn into errors) because they are not as smart as new\n>> >> ones.\n>> >\n>> > I agree with both of the above. I do not think the pros-and-cons are\n>> > in favor of forcing the developer bit to everybody, even though I am\n>> > sympathetic to the desire to see people throw fewer bad changes that\n>> > waste review bandwidth by not compiling or passing its own tests at\n>> > us.\n>>\n>> I agree.\n>>\n>> Responding to the thread in general, perhaps people would like this more if\n>> we turned DEVELOPER=1 DEVOPTS=no-error on by default?\n>>\n>> That's basically why I added it in 99f763baf5 (\"Makefile: add a DEVOPTS to\n>> suppress -Werror under DEVELOPER\", 2018-04-14), because I wanted the\n>> abilty to have verbose informative output without the build dying on some\n>> older systems / compilers.\n>>\n>> It's fine and understandable if you're someone who's just building a package\n>> on some older system if you get a bunch of compiler warnings, but more\n>> annoying if you have to dig into how to disable a default -Werror.\n>\n> I am the platform maintainer for HPE NonStop and need to make sure I'm\n> not packaging DEV builds to anyone\n\nPerhaps confusingly, the DEVELOPER=1 flag in git is not like the\ndeveloper flag in some other projects. It's purely there to turn on\nextra compiler warnings (by default, fatal), it doesn't e.g. turn on\nextra asserts, tracing, or suppress stripping of the binaries.\n\nSo if we enabled some variant of it by default it would be fine to ship\nthe result of that to your users, e.g. I ship DEVELOPER=1 builds to\nusers.\n"},{"id":"354666","messageId":"CAGZ79kYzPYg=MXdWij-voDwNg52srXZOACgK5OOKrquekU-5eg@mail.gmail.com","threadId":"49031","inReplyTo":"20180806170211.GD19053@sigill.intra.peff.net","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-06T18:38:53Z","receivedAt":"2018-08-06T18:39:07Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> I had the impression that DEVELOPER=1 was allowed to set flags that old\n> versions might not even know about. Hence they might actually barf, even\n> without -Werror. Maybe that's better since the introduction of the\n> detect-compiler script, though.\n>\n> I do think we may have a skewed view of the population on this list.\n> We're developers ourselves, and we interact with new developers that we\n> want to help. But there are masses of people[1] building Git who are\n> _not_ developers, and want the default to be as robust as possible.\n> They're probably not going to show up in this thread.\n\nGood point about the skewed perception of people who compile Git\nthemselves.\n\nAt first I thought this could be mitigated by detecting if someone is\na developer by being more clever, for example if they have a commit\n(at HEAD) that is not contained in any remote, it is a pretty good indicator\nthat fresh code was written. But this could also be the case for some\nnon mainstream platform, that needs fixing.\n\nBut then I entertained the thought that we actually do not care about\npeople committing non conforming code to their copy of git.git, but\nactually only care about the code when we see it (i.e. upon review).\n\nSo I wonder if we want to \"fork\" git-send-email and provide a tool to\nsend emails with a pre-send compile&lint check specific to our code\nbase.\n\nBut then all these sound a lot more complicated than a simple knob\nthat we turn on and off. So I don't look any further into it.\n\nStefan\n"},{"id":"354671","messageId":"20180806185938.GB18119@sigill.intra.peff.net","threadId":"49031","inReplyTo":"20180806171119.GC120433@aiede.svl.corp.google.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-06T18:59:39Z","receivedAt":"2018-08-06T18:59:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 06, 2018 at 10:11:19AM -0700, Jonathan Nieder wrote:\n\n> > We're developers ourselves, and we interact with new developers that we\n> > want to help. But there are masses of people[1] building Git who are\n> > _not_ developers, and want the default to be as robust as possible.\n> > They're probably not going to show up in this thread.\n> >\n> > -Peff\n> >\n> > [1] I actually wonder how large that mass is. Clearly there are many\n> > orders of magnitude more users than there are developers. But I have\n> > no idea what percentage of them build from source versus using\n> > somebody else's binary package.\n> \n> Relatedly, we need to think about the incentives these defaults\n> create. Personally, I want *more* naive users to be building from\n> source, because then they are better able to test recent versions,\n> bisect, test my patches, etc.\n> \n> As I hinted in my earlier reply, I think it would be best to try some\n> basic things to make DEVELOPER more visible first. If that fails,\n> then we can revisit how to make this more drastic change in a way that\n> minimizes the harm (and I am not sure yet that that is possible).\n\nYes, I agree very much with both of those paragraphs.\n\n-Peff\n"},{"id":"354672","messageId":"001b01d42dba$8cd1e580$a675b080$@nexbridge.com","threadId":"49031","inReplyTo":"87wot3s87t.fsf@evledraar.gmail.com","subject":"RE: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-08-06T19:20:29Z","receivedAt":"2018-08-06T19:20:38Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On August 6, 2018 1:42 PM, Ævar Arnfjörð Bjarmason wrote:\n> On Mon, Aug 06 2018, Randall S. Becker wrote:\n> \n> > On August 6, 2018 12:40 PM, Ævar Arnfjörð Bjarmason wrote:\n> >> On Sat, Aug 04 2018, Junio C Hamano wrote:\n> >>\n> >> > Duy Nguyen writes:\n> >> >\n> >> >> On Sat, Aug 4, 2018 at 8:11 AM Jonathan Nieder\n> >> >> \n> >> wrote:\n> >> >>> My main concern is not about them but about other people building\n> >> >>> from source in order to run (instead of to develop) Git, and by\n> >> >>> extension, the people they go to for help when it doesn't work.\n> >> >>> I have lots of bitter experience of -Werror being a support\n> >> >>> headache and leading to bad workarounds when someone upgrades\n> >> >>> their compiler and the build starts failing due to a new warning it has\n> introduced.\n> >> >>\n> >> >> Even old compilers can also throw some silly, false positive\n> >> >> warnings (which now turn into errors) because they are not as\n> >> >> smart as new ones.\n> >> >\n> >> > I agree with both of the above. I do not think the pros-and-cons\n> >> > are in favor of forcing the developer bit to everybody, even though\n> >> > I am sympathetic to the desire to see people throw fewer bad\n> >> > changes that waste review bandwidth by not compiling or passing its\n> >> > own tests at us.\n> >>\n> >> I agree.\n> >>\n> >> Responding to the thread in general, perhaps people would like this\n> >> more if we turned DEVELOPER=1 DEVOPTS=no-error on by default?\n> >>\n> >> That's basically why I added it in 99f763baf5 (\"Makefile: add a\n> >> DEVOPTS to suppress -Werror under DEVELOPER\", 2018-04-14), because I\n> >> wanted the abilty to have verbose informative output without the\n> >> build dying on some older systems / compilers.\n> >>\n> >> It's fine and understandable if you're someone who's just building a\n> >> package on some older system if you get a bunch of compiler warnings,\n> >> but more annoying if you have to dig into how to disable a default -\n> Werror.\n> >\n> > I am the platform maintainer for HPE NonStop and need to make sure I'm\n> > not packaging DEV builds to anyone\n> \n> Perhaps confusingly, the DEVELOPER=1 flag in git is not like the developer\n> flag in some other projects. It's purely there to turn on extra compiler\n> warnings (by default, fatal), it doesn't e.g. turn on extra asserts, tracing, or\n> suppress stripping of the binaries.\n> \n> So if we enabled some variant of it by default it would be fine to ship the\n> result of that to your users, e.g. I ship DEVELOPER=1 builds to users.\n\nThat works for me. I generally consider warnings to be errors in my builds anyway, by policy (except when I have no choice in the matter and the warning is explainable).\n\nCheers,\nRandall\n\n-- Brief whoami:\n NonStop developer since approximately 211288444200000000\n UNIX developer since approximately 421664400\n-- In my real life, I talk too much.\n\n\n\n"},{"id":"357074","messageId":"20180831083342.34836-1-sunshine@sunshineco.com","threadId":"49031","inReplyTo":"20180805075736.GF44140@aiede.svl.corp.google.com","subject":"[PATCH] config.mak.uname: resolve FreeBSD iconv-related compilation warning","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-31T08:33:42Z","receivedAt":"2018-08-31T08:34:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Jonathan Nieder \n\nOLD_ICONV has long been needed by FreeBSD so config.mak.uname defines\nit unconditionally. However, recent versions do not need it, and its\npresence results in compilation warnings. Resolve this issue by defining\nOLD_ICONV only for older FreeBSD versions.\n\nSpecifically, revision r281550[1], which is part of FreeBSD 11, removed\nthe need for OLD_ICONV, and r282275[2] back-ported that change to 10.2.\nVersions prior to 10.2 do need it.\n\n[1] https://github.com/freebsd/freebsd/commit/b0813ee288f64f677a2cebf7815754b027a8215b\n[2] https://github.com/freebsd/freebsd/commit/b709ec868adb5170d09bc5a66b18d0e0d5987ab6\n\n[es: commit message; tweak version check to distinguish 10.x versions]\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Eric Sunshine \n---\n\nThis is a follow-up to [1] which encapsulates Jonathan's proposed change\nas a proper patch. I made Jonathan as the author since he did all the\nhard research and formulated the core of the change (whereas I only\nreported the issue and extended the version check to correctly handle\nFreeBSD 10.0 and 10.1). Jonathan's sign-off comes from [1].\n\n[1]: https://public-inbox.org/git/20180805075736.GF44140@aiede.svl.corp.google.com/\n\n config.mak.uname | 12 +++++++++++-\n 1 file changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 2be2f19811..e47af72e01 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -192,7 +192,17 @@ ifeq ($(uname_O),Cygwin)\n endif\n ifeq ($(uname_S),FreeBSD)\n \tNEEDS_LIBICONV = YesPlease\n-\tOLD_ICONV = YesPlease\n+\t# Versions up to 10.1 require OLD_ICONV; 10.2 and beyond don't.\n+\t# A typical version string looks like \"10.2-RELEASE\".\n+\tifeq ($(shell expr \"$(uname_R)\" : '[1-9]\\.'),2)\n+\t\tOLD_ICONV = YesPlease\n+\tendif\n+\tifeq ($(firstword $(subst -, ,$(uname_R))),10.0)\n+\t\tOLD_ICONV = YesPlease\n+\tendif\n+\tifeq ($(firstword $(subst -, ,$(uname_R))),10.1)\n+\t\tOLD_ICONV = YesPlease\n+\tendif\n \tNO_MEMMEM = YesPlease\n \tBASIC_CFLAGS += -I/usr/local/include\n \tBASIC_LDFLAGS += -L/usr/local/lib\n-- \n2.19.0.rc1.352.gb1634b371d\n\n"},{"id":"357082","messageId":"CACBZZX6uCQkCermt8O6v2XB2MnUkt5ER3_z5GYJtGdSk-9JNLA@mail.gmail.com","threadId":"49031","inReplyTo":"20180831083342.34836-1-sunshine@sunshineco.com","subject":"Re: [PATCH] config.mak.uname: resolve FreeBSD iconv-related compilation warning","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-31T11:54:05Z","receivedAt":"2018-08-31T11:54:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Aug 31, 2018 at 11:52 AM Eric Sunshine wrote:\n> OLD_ICONV has long been needed by FreeBSD so config.mak.uname defines\n> it unconditionally. However, recent versions do not need it, and its\n> presence results in compilation warnings. Resolve this issue by defining\n> OLD_ICONV only for older FreeBSD versions.\n\nThis seems sane, but just for context is FreeBSD ports itself just\ncompiling without iconv entirely?\n\n[CC FreeBSD devel/git maintainer]\n\n$ uname -r; grep -ri iconv /usr/ports/devel/git\n11.2-RELEASE-p2\n/usr/ports/devel/git/Makefile:OPTIONS_DEFINE= GUI SVN GITWEB CONTRIB\nP4 CVS HTMLDOCS PERL ICONV CURL \\\n/usr/ports/devel/git/Makefile:OPTIONS_DEFAULT= CONTRIB P4 CVS PERL\nGITWEB ICONV CURL SEND_EMAIL PCRE \\\n/usr/ports/devel/git/Makefile:ICONV_USES= iconv\n/usr/ports/devel/git/Makefile:ICONV_MAKE_ARGS_OFF= NO_ICONV=1\n\nI have little clue about how ports works, but just noticed that\nthey're not monkeypatching in OLD_ICONV there.\n"},{"id":"357102","messageId":"20180831170013.GA41433@aiede.svl.corp.google.com","threadId":"49031","inReplyTo":"20180831083342.34836-1-sunshine@sunshineco.com","subject":"Re: [PATCH] config.mak.uname: resolve FreeBSD iconv-related compilation warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-31T17:00:13Z","receivedAt":"2018-08-31T17:00:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Eric Sunshine wrote:\n\n> From: Jonathan Nieder \n>\n> OLD_ICONV has long been needed by FreeBSD so config.mak.uname defines\n> it unconditionally. However, recent versions do not need it, and its\n> presence results in compilation warnings. Resolve this issue by defining\n> OLD_ICONV only for older FreeBSD versions.\n>\n> Specifically, revision r281550[1], which is part of FreeBSD 11, removed\n> the need for OLD_ICONV, and r282275[2] back-ported that change to 10.2.\n> Versions prior to 10.2 do need it.\n>\n> [1] https://github.com/freebsd/freebsd/commit/b0813ee288f64f677a2cebf7815754b027a8215b\n> [2] https://github.com/freebsd/freebsd/commit/b709ec868adb5170d09bc5a66b18d0e0d5987ab6\n>\n> [es: commit message; tweak version check to distinguish 10.x versions]\n>\n> Signed-off-by: Jonathan Nieder \n> Signed-off-by: Eric Sunshine \n> ---\n> config.mak.uname | 12 +++++++++++-\n> 1 file changed, 11 insertions(+), 1 deletion(-)\n\nI think it makes sense for you to take credit for this one. You\nnoticed the original problem, tested on FreeBSD, wrote the\nexplanation, and figured out the firstword hackery. All I did was to\nsay \"somebody should fix this\" and run \"git log -S\" a few times. In\nany event,\n\nReviewed-by: Jonathan Nieder \n"},{"id":"357109","messageId":"CAPig+cR5VdUjR3zNrNxfkwLnCEE3_KUD9HRhm+BR6LF8FCQ6QA@mail.gmail.com","threadId":"49031","inReplyTo":"CACBZZX6uCQkCermt8O6v2XB2MnUkt5ER3_z5GYJtGdSk-9JNLA@mail.gmail.com","subject":"Re: [PATCH] config.mak.uname: resolve FreeBSD iconv-related compilation warning","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-31T18:31:00Z","receivedAt":"2018-08-31T18:31:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 31, 2018 at 7:54 AM Ævar Arnfjörð Bjarmason\n wrote:\n> On Fri, Aug 31, 2018 at 11:52 AM Eric Sunshine wrote:\n> > OLD_ICONV has long been needed by FreeBSD so config.mak.uname defines\n> > it unconditionally. However, recent versions do not need it, and its\n> > presence results in compilation warnings. Resolve this issue by defining\n> > OLD_ICONV only for older FreeBSD versions.\n>\n> This seems sane, but just for context is FreeBSD ports itself just\n> compiling without iconv entirely?\n>\n> I have little clue about how ports works, but just noticed that\n> they're not monkeypatching in OLD_ICONV there.\n\nMy experience with FreeBSD ports is pretty limited too, but, as I\ndiscovered in [1], they run Git's configure script, so OLD_ICONV is\ndetermined dynamically, as far as I can tell.\n\n[1]: https://public-inbox.org/git/CAPig+cTEGtsmUyoYsKEx+erMsXKm5=c6TJuNAgeky2pcgw18JQ@mail.gmail.com/\n"},{"id":"357131","messageId":"CAPig+cRfakQqNP3ih4Sp0uzvkSNkqpsxBVFSnB9HYmqvavGuxA@mail.gmail.com","threadId":"49031","inReplyTo":"20180831170013.GA41433@aiede.svl.corp.google.com","subject":"Re: [PATCH] config.mak.uname: resolve FreeBSD iconv-related compilation warning","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-31T20:59:25Z","receivedAt":"2018-08-31T20:59:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 31, 2018 at 1:00 PM Jonathan Nieder wrote:\n> > From: Jonathan Nieder \n> > OLD_ICONV has long been needed by FreeBSD so config.mak.uname defines\n> > it unconditionally. However, recent versions do not need it, and its\n> > presence results in compilation warnings. Resolve this issue by defining\n> > OLD_ICONV only for older FreeBSD versions.\n>\n> I think it makes sense for you to take credit for this one. You\n> noticed the original problem, tested on FreeBSD, wrote the\n> explanation, and figured out the firstword hackery. All I did was to\n> say \"somebody should fix this\" and run \"git log -S\" a few times. [...]\n\nI'm fine going either way with authorship, and I can re-roll with that\nminor change (if Junio isn't interested in tweaking it while\nqueueing).\n"},{"id":"357163","messageId":"9f7b6b5b0525b0d360423e3416d7baacd4c5a5cc.camel@gmail.com","threadId":"49031","inReplyTo":"001001d42da7$4a9e6570$dfdb3050$@nexbridge.com","subject":"Re: [PATCH] Makefile: enable DEVELOPER by default","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-09-01T21:01:55Z","receivedAt":"2018-09-01T21:06:37Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Mon, 2018-08-06 at 13:02 -0400, Randall S. Becker wrote:\n> \n> Any idea when this is going to be in an official release, and exactly\n> what the settings will be for \"Not Developer\". I assume DEVELOPER=0\n> and DEVOPTS=error, which is the current behaviour, correct? I am the\n> platform maintainer for HPE NonStop and need to make sure I'm not\n> packaging DEV builds to anyone, since I'm the only one doing this for\n> the platform. It's another hoop, but hopefully not a bad one. The\n> question is the best place to set this, assuming we are using Jenkins\n> for our builds, and I'd rather keep the existing config.mak.uname the\n> same, since at least it seems stable.\n\nJust a FYI and in case you aren't aware, you could create a\n\"config.mak\" to store your custom configurations. You can be sure it's\nused due to the following Makefile part:\n\n...\n\ninclude config.mak.uname\n-include config.mak.autogen\n-include config.mak\n...\n\nIt's just not a hard dependency.\n\nHope that helps,\nSivaraam\n\n"}]}