# [PATCH] Silence iconv warnings on Leopard 12 messages from 2007-12-06 to 2007-12-07. Participants: Wincent Colaiuta, Blake Ramsdell, Jakub Narebski, Linus Torvalds, Junio C Hamano. Thread: https://gitlist.dev/t/11163 ## Wincent Colaiuta, 2007-12-06 19:07 Subject: [PATCH] Silence iconv warnings on Leopard Message-ID: <1196968023-45284-1-git-send-email-win@wincent.com> URL: https://gitlist.dev/e/1196968023-45284-1-git-send-email-win%40wincent.com ``` Apple ships a newer version of iconv with Leopard (Mac OS X 10.5/Darwin 9). Ensure that OLD_ICONV is not set on any version of Darwin in the 9.x series; this should be good for at least a couple of years, when Darwin 10 comes out and we can invert the sense of the test to specifically check for Darwin 7 or 8. A more sophisticated and robust check is possible for those who use autoconf, but not everybody does that. Signed-off-by: Wincent Colaiuta --- Makefile | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 999391e..4dda340 100644 --- a/Makefile +++ b/Makefile @@ -406,7 +406,9 @@ endif ifeq ($(uname_S),Darwin) NEEDS_SSL_WITH_CRYPTO = YesPlease NEEDS_LIBICONV = YesPlease - OLD_ICONV = UnfortunatelyYes + ifneq ($(shell expr "$(uname_R)" : '9\.'),2) + OLD_ICONV = UnfortunatelyYes + endif NO_STRLCPY = YesPlease NO_MEMMEM = YesPlease endif -- 1.5.3.7.1067.gaa51 ``` ## Blake Ramsdell, 2007-12-06 23:04 Subject: Re: [PATCH] Silence iconv warnings on Leopard Message-ID: <985966520712061504s686395d6jf680363c7b3b9de7@mail.gmail.com> URL: https://gitlist.dev/e/985966520712061504s686395d6jf680363c7b3b9de7%40mail.gmail.com In-Reply-To: <1196968023-45284-1-git-send-email-win@wincent.com> ``` On Dec 6, 2007 11:07 AM, Wincent Colaiuta wrote: > Apple ships a newer version of iconv with Leopard (Mac OS X 10.5/Darwin > 9). Ensure that OLD_ICONV is not set on any version of Darwin in the > 9.x series; this should be good for at least a couple of years, when > Darwin 10 comes out and we can invert the sense of the test to > specifically check for Darwin 7 or 8. This approach seems fine to me, there was some concern about matching the OS type / version in the past, but I haven't really seen a better answer. > A more sophisticated and robust check is possible for those who use > autoconf, but not everybody does that. I did make a patch for configure.ac that does this. If it's interesting, I'll send it along. Blake -- Blake Ramsdell | http://www.blakeramsdell.com ``` ## Jakub Narebski, 2007-12-07 00:11 Subject: Re: [PATCH] Silence iconv warnings on Leopard Message-ID: <200712070111.23283.jnareb@gmail.com> URL: https://gitlist.dev/e/200712070111.23283.jnareb%40gmail.com In-Reply-To: <985966520712061504s686395d6jf680363c7b3b9de7@mail.gmail.com> ``` On Fri, 7 Dec 2007, Blake Ramsdell wrote: > On Dec 6, 2007 11:07 AM, Wincent Colaiuta wrote: >> Apple ships a newer version of iconv with Leopard (Mac OS X 10.5/Darwin >> 9). Ensure that OLD_ICONV is not set on any version of Darwin in the >> 9.x series; this should be good for at least a couple of years, when >> Darwin 10 comes out and we can invert the sense of the test to >> specifically check for Darwin 7 or 8. >> A more sophisticated and robust check is possible for those who use >> autoconf, but not everybody does that. > > I did make a patch for configure.ac that does this. If it's > interesting, I'll send it along. I would be interested, as I tried to make a patch to configure.ac which does that. The problem is that it should be check that tests for compile time _warnings_; my solution was to use '-Werror' flag to make warning into error, and AC_COMPILE_IFELSE, but this might be gcc only solution. Message-ID: <1196895948-25115-1-git-send-email-jnareb@gmail.com> http://permalink.gmane.org/gmane.comp.version-control.git/67209 So please send it. Thanks in advance -- Jakub Narebski Poland ``` ## Blake Ramsdell, 2007-12-07 00:12 Subject: Re: [PATCH] Silence iconv warnings on Leopard Message-ID: <985966520712061612r3dd167dax3adf433313a40a0d@mail.gmail.com> URL: https://gitlist.dev/e/985966520712061612r3dd167dax3adf433313a40a0d%40mail.gmail.com In-Reply-To: <200712070111.23283.jnareb@gmail.com> ``` On Dec 6, 2007 4:11 PM, Jakub Narebski wrote: > I would be interested, as I tried to make a patch to configure.ac which > does that. The problem is that it should be check that tests for compile > time _warnings_; my solution was to use '-Werror' flag to make warning > into error, and AC_COMPILE_IFELSE, but this might be gcc only solution. That's precisely what I did. If this isn't suitable, then bummer -- impossible ;). Blake -- Blake Ramsdell | http://www.blakeramsdell.com ``` ## Linus Torvalds, 2007-12-07 00:30 Subject: Re: [PATCH] Silence iconv warnings on Leopard Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.9999.0712061628070.13796%40woody.linux-foundation.org In-Reply-To: <200712070111.23283.jnareb@gmail.com> ``` On Fri, 7 Dec 2007, Jakub Narebski wrote: > > The problem is that it should be check that tests for compile time > _warnings_; my solution was to use '-Werror' flag to make warning into > error, and AC_COMPILE_IFELSE, but this might be gcc only solution. > > Message-ID: <1196895948-25115-1-git-send-email-jnareb@gmail.com> > http://permalink.gmane.org/gmane.comp.version-control.git/67209 > > So please send it. Umm. Why not just make the test be whether the following compiles cleanly? #include extern size_t iconv(iconv_t cd, char **inbuf, size_t *inbytesleft, char **outbuf, size_t *outbytesleft); because if the compiler has seen a "const char **inbuf", then it should error out with a "conflicting types for ‘iconv’" style message.. Just do $CC -c test-iconv.c or something. Totally untested. I don't do autoconf. Linus ``` ## Blake Ramsdell, 2007-12-07 00:41 Subject: Re: [PATCH] Silence iconv warnings on Leopard Message-ID: <985966520712061641y4eba054dj5dea1d0308d50b12@mail.gmail.com> URL: https://gitlist.dev/e/985966520712061641y4eba054dj5dea1d0308d50b12%40mail.gmail.com In-Reply-To: ``` On Dec 6, 2007 4:30 PM, Linus Torvalds wrote: > Umm. Why not just make the test be whether the following compiles cleanly? > > #include > > extern size_t iconv(iconv_t cd, > char **inbuf, size_t *inbytesleft, > char **outbuf, size_t *outbytesleft); > > because if the compiler has seen a "const char **inbuf", then it should > error out with a "conflicting types for 'iconv'" style message.. Yeah, this is what I did: diff --git a/configure.ac b/configure.ac index 5f8a15b..675d3e0 100644 --- a/configure.ac +++ b/configure.ac @@ -182,6 +182,29 @@ AC_SUBST(NEEDS_LIBICONV) AC_SUBST(NO_ICONV) test -n "$NEEDS_LIBICONV" && LIBS="$LIBS -liconv" # +# Define OLD_ICONV if the iconv function prototype uses const** (Darwin and +# some FreeBSD installations). +AC_DEFUN([OLDICONVTEST_SRC], [ +#include + +int main(void) +{ + char* value = "test"; + + (void) iconv (NULL, &value, NULL, NULL, NULL); +} +]) +AC_MSG_CHECKING([for old iconv]) +old_CFLAGS="$CFLAGS" +CFLAGS="$CFLAGS -Werror" +AC_COMPILE_IFELSE(OLDICONVTEST_SRC, + [AC_MSG_RESULT([no]) + OLD_ICONV=], + [AC_MSG_RESULT([yes]) + OLD_ICONV=UnfortunatelyYes]) +CFLAGS="$old_CFLAGS" +AC_SUBST(OLD_ICONV) +# # Define NO_DEFLATE_BOUND if deflateBound is missing from zlib. AC_DEFUN([ZLIBTEST_SRC], [ #include The problem is that AC_COMPILE_IFELSE doesn't barf on warnings, so I had to put in the CFLAGS hack to do -Werror (this is what Jakub did also, I think). So if this isn't rude to use -Werror (which is probably gcc-specific in one or more ways), then fine. If it is rude to use -Werror, then yeah, there needs to be some check for the warning, which I confess in my five minutes of learning autoconf I don't understand well enough to say if it's possible. Blake -- Blake Ramsdell | http://www.blakeramsdell.com ``` ## Blake Ramsdell, 2007-12-07 00:44 Subject: Re: [PATCH] Silence iconv warnings on Leopard Message-ID: <985966520712061644k72a4c4ecpfe26d4e00d14b337@mail.gmail.com> URL: https://gitlist.dev/e/985966520712061644k72a4c4ecpfe26d4e00d14b337%40mail.gmail.com In-Reply-To: <985966520712061641y4eba054dj5dea1d0308d50b12@mail.gmail.com> ``` On Dec 6, 2007 4:41 PM, Blake Ramsdell wrote: > On Dec 6, 2007 4:30 PM, Linus Torvalds wrote: > > Umm. Why not just make the test be whether the following compiles cleanly? > > > > #include > > > > extern size_t iconv(iconv_t cd, > > char **inbuf, size_t *inbytesleft, > > char **outbuf, size_t *outbytesleft); > > > > because if the compiler has seen a "const char **inbuf", then it should > > error out with a "conflicting types for 'iconv'" style message.. > > Yeah, this is what I did: My apologies. Your suggestion is completely different, and should work without -Werror. Let me try that. Blake -- Blake Ramsdell | http://www.blakeramsdell.com ``` ## Jakub Narebski, 2007-12-07 01:27 Subject: [PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning) Message-ID: <1196990840-1168-1-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/1196990840-1168-1-git-send-email-jnareb%40gmail.com In-Reply-To: ``` Update configure.ac (and config.mak.in) to keep up with git development by adding [compile] test whether your library has an old iconv(), where the second (input buffer pointer) parameter is declared with type (const char **) (OLD_ICONV). Test-proposed-by: Linus Torvalds Signed-off-by: Jakub Narebski --- CC-ed to all parties (in this and previous thread). I should probably used AC_LANG_PROGRAM like for NO_C99_FORMAT instead of generating whole programlet^W test program by hand (I hope that for example I haven't missed some header file which needs to be included); I have followed example for NO_ICONV / NEEDS_LIBICONV and NO_DEFLATE_BOUND test. I'm also not sure if I have put this test in the correct autoconf section, but that is probably matter of taste. On Fri, 7 Dec 2007, Linus Torvalds wrote: > > On Fri, 7 Dec 2007, Jakub Narebski wrote: >> >> The problem is that it should be check that tests for compile time >> _warnings_; my solution was to use '-Werror' flag to make warning into >> error, and AC_COMPILE_IFELSE, but this might be gcc only solution. >> >> Message-ID: <1196895948-25115-1-git-send-email-jnareb@gmail.com> >> http://permalink.gmane.org/gmane.comp.version-control.git/67209 >> >> So please send it. > > Umm. Why not just make the test be whether the following compiles cleanly? > > #include > > extern size_t iconv(iconv_t cd, > char **inbuf, size_t *inbytesleft, > char **outbuf, size_t *outbytesleft); > > because if the compiler has seen a "const char **inbuf", then it should > error out with a "conflicting types for ?iconv?" style message.. [...] > Totally untested. I don't do autoconf. Thanks for a suggestion. Implemented in the patch below. It should work correctly, but could those who have old iconv(), or used to have old iconv() please check if works correctly for them? By the way, perhaps the previous idea would work using AC_LANG_WERROR instead of passing adding [temporarily] GCC `-Werror' option to `CFLAGS', or use parts of its implementation. - Macro: AC_LANG_WERROR Normally Autoconf ignores warnings generated by the compiler, linker, and preprocessor. If this macro is used, warnings will be treated as fatal errors instead for the current language. This macro is useful when the results of configuration will be used where warnings are unacceptable; for instance, if parts of a program are built with the GCC `-Werror' option. If the whole program will be built using `-Werror' it is often simpler to put `-Werror' in the compiler flags (`CFLAGS' etc.). On Fri, 7 Dec 2007, Blake Ramsdell wrote: > On Dec 6, 2007 4:41 PM, Blake Ramsdell wrote: >> On Dec 6, 2007 4:30 PM, Linus Torvalds wrote: >>> Umm. Why not just make the test be whether the following compiles cleanly? >>> >>> #include >>> >>> extern size_t iconv(iconv_t cd, >>> char **inbuf, size_t *inbytesleft, >>> char **outbuf, size_t *outbytesleft); >>> >>> because if the compiler has seen a "const char **inbuf", then it should >>> error out with a "conflicting types for 'iconv'" style message.. >> >> Yeah, this is what I did: > > My apologies. Your suggestion is completely different, and should work > without -Werror. Let me try that. Is something like the patch below what you wanted to try? config.mak.in | 1 + configure.ac | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-) diff --git a/config.mak.in b/config.mak.in index 11d256e..7d5df9b 100644 --- a/config.mak.in +++ b/config.mak.in @@ -41,4 +41,5 @@ NO_STRTOUMAX=@NO_STRTOUMAX@ NO_SETENV=@NO_SETENV@ NO_MKDTEMP=@NO_MKDTEMP@ NO_ICONV=@NO_ICONV@ +OLD_ICONV=@OLD_ICONV@ NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@ diff --git a/configure.ac b/configure.ac index 5f8a15b..86be19a 100644 --- a/configure.ac +++ b/configure.ac @@ -212,6 +212,28 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket" ## Checks for header files. +AC_MSG_NOTICE([CHECKS for header files]) +# +# Define OLD_ICONV if your library has an old iconv(), where the second +# (input buffer pointer) parameter is declared with type (const char **). +AC_DEFUN([OLDICONVTEST_SRC], [[ +#include + +extern size_t iconv(iconv_t cd, + char **inbuf, size_t *inbytesleft, + char **outbuf, size_t *outbytesleft); + +int main(void) +{ + return 0; +} +]]) +AC_MSG_CHECKING([for old iconv()]) +AC_COMPILE_IFELSE(OLDICONVTEST_SRC, + [AC_MSG_RESULT([no])], + [AC_MSG_RESULT([yes]) + OLD_ICONV=UnfortunatelyYes]) +AC_SUBST(OLD_ICONV) ## Checks for typedefs, structures, and compiler characteristics. -- 1.5.3.7 ``` ## Junio C Hamano, 2007-12-07 07:26 Subject: Re: [PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning) Message-ID: <7v7ijr7yph.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7ijr7yph.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1196990840-1168-1-git-send-email-jnareb@gmail.com> ``` Jakub Narebski writes: > On Fri, 7 Dec 2007, Blake Ramsdell wrote: >> On Dec 6, 2007 4:41 PM, Blake Ramsdell wrote: >>> On Dec 6, 2007 4:30 PM, Linus Torvalds wrote: >>>> Umm. Why not just make the test be whether the following compiles cleanly? >>>> >>>> #include >>>> >>>> extern size_t iconv(iconv_t cd, >>>> char **inbuf, size_t *inbytesleft, >>>> char **outbuf, size_t *outbytesleft); >>>> >>>> because if the compiler has seen a "const char **inbuf", then it should >>>> error out with a "conflicting types for 'iconv'" style message.. >>> >>> Yeah, this is what I did: >> >> My apologies. Your suggestion is completely different, and should work >> without -Werror. Let me try that. > > Is something like the patch below what you wanted to try? This looks sensible. Will apply. ``` ## Wincent Colaiuta, 2007-12-07 08:22 Subject: Re: [PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning) Message-ID: <280C2FED-C7EB-42D1-BB71-1680558C7CBA@wincent.com> URL: https://gitlist.dev/e/280C2FED-C7EB-42D1-BB71-1680558C7CBA%40wincent.com In-Reply-To: <1196990840-1168-1-git-send-email-jnareb@gmail.com> ``` El 7/12/2007, a las 2:27, Jakub Narebski escribió: > Update configure.ac (and config.mak.in) to keep up with git > development by adding [compile] test whether your library has an old > iconv(), where the second (input buffer pointer) parameter is declared > with type (const char **) (OLD_ICONV). > > Test-proposed-by: Linus Torvalds > Signed-off-by: Jakub Narebski Tested and working here on Mac OS X 10.5.1/Darwin 9.1.0 (where we no longer have an old iconv): configure: CHECKS for header files checking for old iconv()... no Cheers, Wincent ``` ## Wincent Colaiuta, 2007-12-07 08:25 Subject: Re: [PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning) Message-ID: <27036A24-2636-4278-B0D5-4128201DF35F@wincent.com> URL: https://gitlist.dev/e/27036A24-2636-4278-B0D5-4128201DF35F%40wincent.com In-Reply-To: <7v7ijr7yph.fsf@gitster.siamese.dyndns.org> ``` El 7/12/2007, a las 8:26, Junio C Hamano escribió: > Jakub Narebski writes: > >> Is something like the patch below what you wanted to try? > > This looks sensible. Will apply. This works for those using autoconf. Junio, did you get a chance to look at the patch I posted for people who don't use autoconf? Cheers, Wincent ``` ## Blake Ramsdell, 2007-12-07 08:29 Subject: Re: [PATCH/RFC (take 3)] autoconf: Add test for OLD_ICONV (squelching compiler warning) Message-ID: <985966520712070029y2a3402caia81524f8dc0c608e@mail.gmail.com> URL: https://gitlist.dev/e/985966520712070029y2a3402caia81524f8dc0c608e%40mail.gmail.com In-Reply-To: <1196990840-1168-1-git-send-email-jnareb@gmail.com> ``` On Dec 6, 2007 5:27 PM, Jakub Narebski wrote: > Is something like the patch below what you wanted to try? Yes. Not sure if main() is required, so it might be shorter if that's removed, but not really relevant as long as it works. And CHECKS for header files might be considered chatty. Blake -- Blake Ramsdell | http://www.blakeramsdell.com ```