{"thread":{"id":"11163","subject":"[PATCH] Silence iconv warnings on Leopard","startedAt":"2007-12-06T19:07:03Z","lastAt":"2007-12-07T08:29:42Z","messageCount":12,"participants":["Wincent Colaiuta","Blake Ramsdell","Jakub Narebski","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"62185","messageId":"1196968023-45284-1-git-send-email-win@wincent.com","threadId":"11163","inReplyTo":null,"subject":"[PATCH] Silence iconv warnings on Leopard","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-06T19:07:03Z","receivedAt":"2007-12-06T19:07:03Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"Apple ships a newer version of iconv with Leopard (Mac OS X 10.5/Darwin\n9). Ensure that OLD_ICONV is not set on any version of Darwin in the\n9.x series; this should be good for at least a couple of years, when\nDarwin 10 comes out and we can invert the sense of the test to\nspecifically check for Darwin 7 or 8.\n\nA more sophisticated and robust check is possible for those who use\nautoconf, but not everybody does that.\n\nSigned-off-by: Wincent Colaiuta \n---\n Makefile | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 999391e..4dda340 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -406,7 +406,9 @@ endif\n ifeq ($(uname_S),Darwin)\n \tNEEDS_SSL_WITH_CRYPTO = YesPlease\n \tNEEDS_LIBICONV = YesPlease\n-\tOLD_ICONV = UnfortunatelyYes\n+\tifneq ($(shell expr \"$(uname_R)\" : '9\\.'),2)\n+\t\tOLD_ICONV = UnfortunatelyYes\n+\tendif\n \tNO_STRLCPY = YesPlease\n \tNO_MEMMEM = YesPlease\n endif\n-- \n1.5.3.7.1067.gaa51\n"},{"id":"62214","messageId":"985966520712061504s686395d6jf680363c7b3b9de7@mail.gmail.com","threadId":"11163","inReplyTo":"1196968023-45284-1-git-send-email-win@wincent.com","subject":"Re: [PATCH] Silence iconv warnings on Leopard","fromName":"Blake Ramsdell","fromEmail":"blaker@gmail.com","sentAt":"2007-12-06T23:04:08Z","receivedAt":"2007-12-06T23:04:08Z","isPatch":true,"sender":{"key":"blaker@gmail.com","avatar":"https://gravatar.com/avatar/2bfd39d25cb244dc4354573d5ca413a1adb793017b48cfa84baaf98b77be2a3b?d=mp&s=160"},"body":"On Dec 6, 2007 11:07 AM, Wincent Colaiuta wrote:\n> Apple ships a newer version of iconv with Leopard (Mac OS X 10.5/Darwin\n> 9). Ensure that OLD_ICONV is not set on any version of Darwin in the\n> 9.x series; this should be good for at least a couple of years, when\n> Darwin 10 comes out and we can invert the sense of the test to\n> specifically check for Darwin 7 or 8.\n\nThis approach seems fine to me, there was some concern about matching\nthe OS type / version in the past, but I haven't really seen a better\nanswer.\n\n> A more sophisticated and robust check is possible for those who use\n> autoconf, but not everybody does that.\n\nI did make a patch for configure.ac that does this. If it's\ninteresting, I'll send it along.\n\nBlake\n-- \nBlake Ramsdell | http://www.blakeramsdell.com\n"},{"id":"62218","messageId":"200712070111.23283.jnareb@gmail.com","threadId":"11163","inReplyTo":"985966520712061504s686395d6jf680363c7b3b9de7@mail.gmail.com","subject":"Re: [PATCH] Silence iconv warnings on Leopard","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-12-07T00:11:21Z","receivedAt":"2007-12-07T00:11:21Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 7 Dec 2007, Blake Ramsdell wrote:\n> On Dec 6, 2007 11:07 AM, Wincent Colaiuta wrote:\n\n>> Apple ships a newer version of iconv with Leopard (Mac OS X 10.5/Darwin\n>> 9). Ensure that OLD_ICONV is not set on any version of Darwin in the\n>> 9.x series; this should be good for at least a couple of years, when\n>> Darwin 10 comes out and we can invert the sense of the test to\n>> specifically check for Darwin 7 or 8.\n\n>> A more sophisticated and robust check is possible for those who use\n>> autoconf, but not everybody does that.\n> \n> I did make a patch for configure.ac that does this. If it's\n> interesting, I'll send it along.\n\nI would be interested, as I tried to make a patch to configure.ac which\ndoes that. The problem is that it should be check that tests for compile\ntime _warnings_; my solution was to use '-Werror' flag to make warning\ninto error, and AC_COMPILE_IFELSE, but this might be gcc only solution.\n\n Message-ID: <1196895948-25115-1-git-send-email-jnareb@gmail.com>\n http://permalink.gmane.org/gmane.comp.version-control.git/67209\n\nSo please send it.\n\nThanks in advance\n-- \nJakub Narebski\nPoland\n"},{"id":"62219","messageId":"985966520712061612r3dd167dax3adf433313a40a0d@mail.gmail.com","threadId":"11163","inReplyTo":"200712070111.23283.jnareb@gmail.com","subject":"Re: [PATCH] Silence iconv warnings on Leopard","fromName":"Blake Ramsdell","fromEmail":"blaker@gmail.com","sentAt":"2007-12-07T00:12:49Z","receivedAt":"2007-12-07T00:12:49Z","isPatch":true,"sender":{"key":"blaker@gmail.com","avatar":"https://gravatar.com/avatar/2bfd39d25cb244dc4354573d5ca413a1adb793017b48cfa84baaf98b77be2a3b?d=mp&s=160"},"body":"On Dec 6, 2007 4:11 PM, Jakub Narebski wrote:\n> I would be interested, as I tried to make a patch to configure.ac which\n> does that. The problem is that it should be check that tests for compile\n> time _warnings_; my solution was to use '-Werror' flag to make warning\n> into error, and AC_COMPILE_IFELSE, but this might be gcc only solution.\n\nThat's precisely what I did. If this isn't suitable, then bummer --\nimpossible ;).\n\nBlake\n-- \nBlake Ramsdell | http://www.blakeramsdell.com\n"},{"id":"62221","messageId":"alpine.LFD.0.9999.0712061628070.13796@woody.linux-foundation.org","threadId":"11163","inReplyTo":"200712070111.23283.jnareb@gmail.com","subject":"Re: [PATCH] Silence iconv warnings on Leopard","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-07T00:30:15Z","receivedAt":"2007-12-07T00:30:15Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 7 Dec 2007, Jakub Narebski wrote:\n>\n> The problem is that it should be check that tests for compile time \n> _warnings_; my solution was to use '-Werror' flag to make warning into \n> error, and AC_COMPILE_IFELSE, but this might be gcc only solution.\n> \n> Message-ID: <1196895948-25115-1-git-send-email-jnareb@gmail.com>\n> http://permalink.gmane.org/gmane.comp.version-control.git/67209\n> \n> So please send it.\n\nUmm. Why not just make the test be whether the following compiles cleanly?\n\n\t#include \n\t\n\textern size_t iconv(iconv_t cd,\n\t char **inbuf, size_t *inbytesleft,\n\t char **outbuf, size_t *outbytesleft);\n\nbecause if the compiler has seen a \"const char **inbuf\", then it should \nerror out with a \"conflicting types for ‘iconv’\" style message..\n\nJust do\n\n\t $CC -c test-iconv.c\n\nor something.\n\nTotally untested. I don't do autoconf.\n\n\t\tLinus\n"},{"id":"62222","messageId":"985966520712061641y4eba054dj5dea1d0308d50b12@mail.gmail.com","threadId":"11163","inReplyTo":"alpine.LFD.0.9999.0712061628070.13796@woody.linux-foundation.org","subject":"Re: [PATCH] Silence iconv warnings on Leopard","fromName":"Blake Ramsdell","fromEmail":"blaker@gmail.com","sentAt":"2007-12-07T00:41:24Z","receivedAt":"2007-12-07T00:41:24Z","isPatch":true,"sender":{"key":"blaker@gmail.com","avatar":"https://gravatar.com/avatar/2bfd39d25cb244dc4354573d5ca413a1adb793017b48cfa84baaf98b77be2a3b?d=mp&s=160"},"body":"On Dec 6, 2007 4:30 PM, Linus Torvalds wrote:\n> Umm. Why not just make the test be whether the following compiles cleanly?\n>\n> #include \n>\n> extern size_t iconv(iconv_t cd,\n> char **inbuf, size_t *inbytesleft,\n> char **outbuf, size_t *outbytesleft);\n>\n> because if the compiler has seen a \"const char **inbuf\", then it should\n> error out with a \"conflicting types for 'iconv'\" style message..\n\nYeah, this is what I did:\n\ndiff --git a/configure.ac b/configure.ac\nindex 5f8a15b..675d3e0 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -182,6 +182,29 @@ AC_SUBST(NEEDS_LIBICONV)\n AC_SUBST(NO_ICONV)\n test -n \"$NEEDS_LIBICONV\" && LIBS=\"$LIBS -liconv\"\n #\n+# Define OLD_ICONV if the iconv function prototype uses const** (Darwin and\n+# some FreeBSD installations).\n+AC_DEFUN([OLDICONVTEST_SRC], [\n+#include \n+\n+int main(void)\n+{\n+ char* value = \"test\";\n+\n+ (void) iconv (NULL, &value, NULL, NULL, NULL);\n+}\n+])\n+AC_MSG_CHECKING([for old iconv])\n+old_CFLAGS=\"$CFLAGS\"\n+CFLAGS=\"$CFLAGS -Werror\"\n+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,\n+\t[AC_MSG_RESULT([no])\n+\tOLD_ICONV=],\n+\t[AC_MSG_RESULT([yes])\n+\tOLD_ICONV=UnfortunatelyYes])\n+CFLAGS=\"$old_CFLAGS\"\n+AC_SUBST(OLD_ICONV)\n+#\n # Define NO_DEFLATE_BOUND if deflateBound is missing from zlib.\n AC_DEFUN([ZLIBTEST_SRC], [\n #include \n\nThe problem is that AC_COMPILE_IFELSE doesn't barf on warnings, so I\nhad to put in the CFLAGS hack to do -Werror (this is what Jakub did\nalso, I think).\n\nSo if this isn't rude to use -Werror (which is probably gcc-specific\nin one or more ways), then fine. If it is rude to use -Werror, then\nyeah, there needs to be some check for the warning, which I confess in\nmy five minutes of learning autoconf I don't understand well enough to\nsay if it's possible.\n\nBlake\n-- \nBlake Ramsdell | http://www.blakeramsdell.com\n"},{"id":"62223","messageId":"985966520712061644k72a4c4ecpfe26d4e00d14b337@mail.gmail.com","threadId":"11163","inReplyTo":"985966520712061641y4eba054dj5dea1d0308d50b12@mail.gmail.com","subject":"Re: [PATCH] Silence iconv warnings on Leopard","fromName":"Blake Ramsdell","fromEmail":"blaker@gmail.com","sentAt":"2007-12-07T00:44:04Z","receivedAt":"2007-12-07T00:44:04Z","isPatch":true,"sender":{"key":"blaker@gmail.com","avatar":"https://gravatar.com/avatar/2bfd39d25cb244dc4354573d5ca413a1adb793017b48cfa84baaf98b77be2a3b?d=mp&s=160"},"body":"On Dec 6, 2007 4:41 PM, Blake Ramsdell wrote:\n> On Dec 6, 2007 4:30 PM, Linus Torvalds wrote:\n> > Umm. Why not just make the test be whether the following compiles cleanly?\n> >\n> > #include \n> >\n> > extern size_t iconv(iconv_t cd,\n> > char **inbuf, size_t *inbytesleft,\n> > char **outbuf, size_t *outbytesleft);\n> >\n> > because if the compiler has seen a \"const char **inbuf\", then it should\n> > error out with a \"conflicting types for 'iconv'\" style message..\n>\n> Yeah, this is what I did:\n\nMy apologies. Your suggestion is completely different, and should work\nwithout -Werror. Let me try that.\n\nBlake\n-- \nBlake Ramsdell | http://www.blakeramsdell.com\n"},{"id":"62224","messageId":"1196990840-1168-1-git-send-email-jnareb@gmail.com","threadId":"11163","inReplyTo":"alpine.LFD.0.9999.0712061628070.13796@woody.linux-foundation.org","subject":"[PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-12-07T01:27:20Z","receivedAt":"2007-12-07T01:27:20Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Update configure.ac (and config.mak.in) to keep up with git\ndevelopment by adding [compile] test whether your library has an old\niconv(), where the second (input buffer pointer) parameter is declared\nwith type (const char **) (OLD_ICONV).\n\nTest-proposed-by: Linus Torvalds \nSigned-off-by: Jakub Narebski \n---\nCC-ed to all parties (in this and previous thread).\n\nI should probably used AC_LANG_PROGRAM like for NO_C99_FORMAT instead\nof generating whole programlet^W test program by hand (I hope that for\nexample I haven't missed some header file which needs to be included);\nI have followed example for NO_ICONV / NEEDS_LIBICONV and\nNO_DEFLATE_BOUND test. I'm also not sure if I have put this test in\nthe correct autoconf section, but that is probably matter of taste.\n\nOn Fri, 7 Dec 2007, Linus Torvalds wrote:\n> \n> On Fri, 7 Dec 2007, Jakub Narebski wrote:\n>>\n>> The problem is that it should be check that tests for compile time \n>> _warnings_; my solution was to use '-Werror' flag to make warning into \n>> error, and AC_COMPILE_IFELSE, but this might be gcc only solution.\n>> \n>> Message-ID: <1196895948-25115-1-git-send-email-jnareb@gmail.com>\n>> http://permalink.gmane.org/gmane.comp.version-control.git/67209\n>> \n>> So please send it.\n> \n> Umm. Why not just make the test be whether the following compiles cleanly?\n> \n> \t#include \n> \t\n> \textern size_t iconv(iconv_t cd,\n> \t char **inbuf, size_t *inbytesleft,\n> \t char **outbuf, size_t *outbytesleft);\n> \n> because if the compiler has seen a \"const char **inbuf\", then it should \n> error out with a \"conflicting types for ?iconv?\" style message..\n\n[...]\n\n> Totally untested. I don't do autoconf.\n\nThanks for a suggestion. Implemented in the patch below. It should\nwork correctly, but could those who have old iconv(), or used to have\nold iconv() please check if works correctly for them?\n\nBy the way, perhaps the previous idea would work using AC_LANG_WERROR\ninstead of passing adding [temporarily] GCC `-Werror' option to\n`CFLAGS', or use parts of its implementation.\n\n - Macro: AC_LANG_WERROR\n Normally Autoconf ignores warnings generated by the compiler,\n linker, and preprocessor. If this macro is used, warnings will be\n treated as fatal errors instead for the current language. This\n macro is useful when the results of configuration will be used\n where warnings are unacceptable; for instance, if parts of a\n program are built with the GCC `-Werror' option. If the whole\n program will be built using `-Werror' it is often simpler to put\n `-Werror' in the compiler flags (`CFLAGS' etc.).\n\n\nOn Fri, 7 Dec 2007, Blake Ramsdell wrote:\n> On Dec 6, 2007 4:41 PM, Blake Ramsdell wrote:\n>> On Dec 6, 2007 4:30 PM, Linus Torvalds wrote:\n>>> Umm. Why not just make the test be whether the following compiles cleanly?\n>>>\n>>> #include \n>>>\n>>> extern size_t iconv(iconv_t cd,\n>>> char **inbuf, size_t *inbytesleft,\n>>> char **outbuf, size_t *outbytesleft);\n>>>\n>>> because if the compiler has seen a \"const char **inbuf\", then it should\n>>> error out with a \"conflicting types for 'iconv'\" style message..\n>>\n>> Yeah, this is what I did:\n> \n> My apologies. Your suggestion is completely different, and should work\n> without -Werror. Let me try that.\n\nIs something like the patch below what you wanted to try?\n\n config.mak.in | 1 +\n configure.ac | 22 ++++++++++++++++++++++\n 2 files changed, 23 insertions(+), 0 deletions(-)\n\ndiff --git a/config.mak.in b/config.mak.in\nindex 11d256e..7d5df9b 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -41,4 +41,5 @@ NO_STRTOUMAX=@NO_STRTOUMAX@\n NO_SETENV=@NO_SETENV@\n NO_MKDTEMP=@NO_MKDTEMP@\n NO_ICONV=@NO_ICONV@\n+OLD_ICONV=@OLD_ICONV@\n NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@\ndiff --git a/configure.ac b/configure.ac\nindex 5f8a15b..86be19a 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -212,6 +212,28 @@ test -n \"$NEEDS_SOCKET\" && LIBS=\"$LIBS -lsocket\"\n \n \n ## Checks for header files.\n+AC_MSG_NOTICE([CHECKS for header files])\n+#\n+# Define OLD_ICONV if your library has an old iconv(), where the second\n+# (input buffer pointer) parameter is declared with type (const char **).\n+AC_DEFUN([OLDICONVTEST_SRC], [[\n+#include \n+\n+extern size_t iconv(iconv_t cd,\n+ char **inbuf, size_t *inbytesleft,\n+ char **outbuf, size_t *outbytesleft);\n+\n+int main(void)\n+{\n+\treturn 0;\n+}\n+]])\n+AC_MSG_CHECKING([for old iconv()])\n+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,\n+\t[AC_MSG_RESULT([no])],\n+\t[AC_MSG_RESULT([yes])\n+\tOLD_ICONV=UnfortunatelyYes])\n+AC_SUBST(OLD_ICONV)\n \n \n ## Checks for typedefs, structures, and compiler characteristics.\n-- \n1.5.3.7\n"},{"id":"62252","messageId":"7v7ijr7yph.fsf@gitster.siamese.dyndns.org","threadId":"11163","inReplyTo":"1196990840-1168-1-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-07T07:26:02Z","receivedAt":"2007-12-07T07:26:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> On Fri, 7 Dec 2007, Blake Ramsdell wrote:\n>> On Dec 6, 2007 4:41 PM, Blake Ramsdell wrote:\n>>> On Dec 6, 2007 4:30 PM, Linus Torvalds wrote:\n>>>> Umm. Why not just make the test be whether the following compiles cleanly?\n>>>>\n>>>> #include \n>>>>\n>>>> extern size_t iconv(iconv_t cd,\n>>>> char **inbuf, size_t *inbytesleft,\n>>>> char **outbuf, size_t *outbytesleft);\n>>>>\n>>>> because if the compiler has seen a \"const char **inbuf\", then it should\n>>>> error out with a \"conflicting types for 'iconv'\" style message..\n>>>\n>>> Yeah, this is what I did:\n>> \n>> My apologies. Your suggestion is completely different, and should work\n>> without -Werror. Let me try that.\n>\n> Is something like the patch below what you wanted to try?\n\nThis looks sensible. Will apply.\n"},{"id":"62258","messageId":"280C2FED-C7EB-42D1-BB71-1680558C7CBA@wincent.com","threadId":"11163","inReplyTo":"1196990840-1168-1-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning)","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-07T08:22:58Z","receivedAt":"2007-12-07T08:22:58Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 7/12/2007, a las 2:27, Jakub Narebski escribió:\n\n> Update configure.ac (and config.mak.in) to keep up with git\n> development by adding [compile] test whether your library has an old\n> iconv(), where the second (input buffer pointer) parameter is declared\n> with type (const char **) (OLD_ICONV).\n>\n> Test-proposed-by: Linus Torvalds \n> Signed-off-by: Jakub Narebski \n\nTested and working here on Mac OS X 10.5.1/Darwin 9.1.0 (where we no \nlonger have an old iconv):\n\nconfigure: CHECKS for header files\nchecking for old iconv()... no\n\nCheers,\nWincent\n"},{"id":"62259","messageId":"27036A24-2636-4278-B0D5-4128201DF35F@wincent.com","threadId":"11163","inReplyTo":"7v7ijr7yph.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning)","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-07T08:25:04Z","receivedAt":"2007-12-07T08:25:04Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 7/12/2007, a las 8:26, Junio C Hamano escribió:\n\n> Jakub Narebski writes:\n>\n>> Is something like the patch below what you wanted to try?\n>\n> This looks sensible. Will apply.\n\n\nThis works for those using autoconf. Junio, did you get a chance to \nlook at the patch I posted for people who don't use autoconf?\n\nCheers,\nWincent\n"},{"id":"62260","messageId":"985966520712070029y2a3402caia81524f8dc0c608e@mail.gmail.com","threadId":"11163","inReplyTo":"1196990840-1168-1-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning)","fromName":"Blake Ramsdell","fromEmail":"blaker@gmail.com","sentAt":"2007-12-07T08:29:42Z","receivedAt":"2007-12-07T08:29:42Z","isPatch":true,"sender":{"key":"blaker@gmail.com","avatar":"https://gravatar.com/avatar/2bfd39d25cb244dc4354573d5ca413a1adb793017b48cfa84baaf98b77be2a3b?d=mp&s=160"},"body":"On Dec 6, 2007 5:27 PM, Jakub Narebski wrote:\n> Is something like the patch below what you wanted to try?\n\nYes. Not sure if main() is required, so it might be shorter if that's\nremoved, but not really relevant as long as it works.\n\nAnd CHECKS for header files might be considered chatty.\n\nBlake\n-- \nBlake Ramsdell | http://www.blakeramsdell.com\n"}]}