# [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions 20 messages from 2007-11-19 to 2007-12-06. Participants: Jakub Narebski, Stefan-W. Hahn, Matt Kraai, Shawn O. Pearce, Brian Gernhardt, Wincent Colaiuta, Junio C Hamano, Pascal Obry. Thread: https://gitlist.dev/t/10933 ## Jakub Narebski, 2007-11-19 18:47 Subject: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions Message-ID: <200711191947.05960.jnareb@gmail.com> URL: https://gitlist.dev/e/200711191947.05960.jnareb%40gmail.com ``` Update configure.ac (and config.mak.in) to keep up with git development by adding tests for memmem (NO_MEMMEM), strtoumax (NO_STRTOUMAX) and mkdtemp (NO_MKDTEMP) functions. Signed-off-by: Jakub Narebski --- This is beginning of "bring configure up to date" thingy. By the way, do you have idea how to test for the following in configure.ac: * Define NO_PREAD if you have a problem with pread() system call (e.g. cygwin.dll before v1.5.22). - what is the problem? how to detect it? * Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is generally faster on your platform than accessing the working directory. - if at all possible * Define NO_TRUSTABLE_FILEMODE if your filesystem may claim to support the executable mode bit, but doesn't really do so. - I think there were some code here * Define NO_R_TO_GCC_LINKER if your gcc does not like "-R/path/lib" that tells runtime paths to dynamic libraries; "-Wl,-rpath=/path/lib" is used instead. * Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's MakeMaker (e.g. using ActiveState under Cygwin). * Define ASCIIDOC8 if you want to format documentation with AsciiDoc 8 * Define DOCBOOK_XSL_172 if you want to format man pages with DocBook XSL v1.72. - it needs some portable way to check asciidoc and docbook-xsl version * Define OLD_ICONV if your library has an old iconv(), where the second (input buffer pointer) parameter is declared with type (const char **). - perhaps compile with new iconv and check for compile errors? config.mak.in | 3 +++ configure.ac | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 0 deletions(-) diff --git a/config.mak.in b/config.mak.in index 776b805..11d256e 100644 --- a/config.mak.in +++ b/config.mak.in @@ -35,7 +35,10 @@ NO_SOCKADDR_STORAGE=@NO_SOCKADDR_STORAGE@ NO_IPV6=@NO_IPV6@ NO_C99_FORMAT=@NO_C99_FORMAT@ NO_STRCASESTR=@NO_STRCASESTR@ +NO_MEMMEM=@NO_MEMMEM@ NO_STRLCPY=@NO_STRLCPY@ +NO_STRTOUMAX=@NO_STRTOUMAX@ NO_SETENV=@NO_SETENV@ +NO_MKDTEMP=@NO_MKDTEMP@ NO_ICONV=@NO_ICONV@ NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@ diff --git a/configure.ac b/configure.ac index 53e9a17..7bcf1a4 100644 --- a/configure.ac +++ b/configure.ac @@ -289,18 +289,36 @@ AC_CHECK_FUNC(strcasestr, [NO_STRCASESTR=YesPlease]) AC_SUBST(NO_STRCASESTR) # +# Define NO_MEMMEM if you don't have memmem. +AC_CHECK_FUNC(memmem, +[NO_MEMMEM=], +[NO_MEMMEM=YesPlease]) +AC_SUBST(NO_MEMMEM) +# # Define NO_STRLCPY if you don't have strlcpy. AC_CHECK_FUNC(strlcpy, [NO_STRLCPY=], [NO_STRLCPY=YesPlease]) AC_SUBST(NO_STRLCPY) # +# Define NO_STRTOUMAX if you don't have strtoumax in the C library. +AC_CHECK_FUNC(strtoumax, +[NO_STRTOUMAX=], +[NO_STRTOUMAX=YesPlease]) +AC_SUBST(NO_STRTOUMAX) +# # Define NO_SETENV if you don't have setenv in the C library. AC_CHECK_FUNC(setenv, [NO_SETENV=], [NO_SETENV=YesPlease]) AC_SUBST(NO_SETENV) # +# Define NO_MKDTEMP if you don't have mkdtemp in the C library. +AC_CHECK_FUNC(mkdtemp, +[NO_MKDTEMP=], +[NO_MKDTEMP=YesPlease]) +AC_SUBST(NO_MKDTEMP) +# # Define NO_MMAP if you want to avoid mmap. # # Define NO_ICONV if your libc does not properly support iconv. -- 1.5.3.5 ``` ## Stefan-W. Hahn, 2007-11-20 18:12 Subject: Re: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions Message-ID: <20071120181202.GA12389@scotty.home> URL: https://gitlist.dev/e/20071120181202.GA12389%40scotty.home In-Reply-To: <7vd4u5l29v.fsf@gitster.siamese.dyndns.org> ``` Also sprach Junio C Hamano am Tue, 20 Nov 2007 at 01:00:44 -0800: > Jakub Narebski writes: > > > By the way, do you have idea how to test for the following > > in configure.ac: > > > > * Define NO_PREAD if you have a problem with pread() system call (e.g. > > cygwin.dll before v1.5.22). > > > > - what is the problem? how to detect it? > > 6900679c2f6d937a5a6ef616869c8887690ad19d (Replacing the system > call pread() with lseek()/xread()/lseek() sequence.) > For me it happened when using git (e.g. git commit). Detecting perhaps is possible with "cygcheck" command which shows the version of cygwin system dlls. Perhaps: cygcheck -p 'cygwin1.dll' Shows name and version of found cygwin1.dll. Stefan -- Stefan-W. Hahn It is easy to make things. / mailto:stefan.hahn@s-hahn.de / It is hard to make things simple. ``` ## Matt Kraai, 2007-11-21 00:08 Subject: Re: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions Message-ID: <20071121000824.GA25959@ftbfs.org> URL: https://gitlist.dev/e/20071121000824.GA25959%40ftbfs.org In-Reply-To: <7vd4u5l29v.fsf@gitster.siamese.dyndns.org> ``` On Tue, Nov 20, 2007 at 01:00:44AM -0800, Junio C Hamano wrote: > Jakub Narebski writes: > > * Define NO_R_TO_GCC_LINKER if your gcc does not like "-R/path/lib" > > that tells runtime paths to dynamic libraries; > > "-Wl,-rpath=/path/lib" is used instead. > > Try with -R/path/lib and see if it complains, perhaps? > > 206488774238c94ff337dd5c997be441dbde5112 (Correct the name of > NO_R_TO_GCC_LINKER in the comment describing it.) I don't know how to test this, unfortunately. I'll be happy to test any tests you'd like me to try, though. > > MakeMaker (e.g. using ActiveState under Cygwin). > > f848718a6980ebda0eb5afb2ca49c3bc1e7b2b1d (Make perl/ build > procedure ActiveState friendly.) > > Alex CC'ed. > > > * Define ASCIIDOC8 if you want to format documentation with AsciiDoc 8 > > case "$(asciidoc --version)" in > asciidoc' '7*) : not def ;; > asciidoc' '8) : def ;; > esac > > > * Define DOCBOOK_XSL_172 if you want to format man pages with DocBook XSL v1.72. > > > > - it needs some portable way to check asciidoc and docbook-xsl version > > Hmmm. > > > * Define OLD_ICONV if your library has an old iconv(), where the second > > (input buffer pointer) parameter is declared with type (const char **). > > > > - perhaps compile with new iconv and check for compile errors? > > Yeah, with appropriate includes, > > char *ibp, *obp; > size_t insz, outsz; > iconv(conv, &ibp, &insz, &obp, &outsz); > > would cause compilation trouble if iconv() wants "const char *" > for ibp. -- Matt ``` ## Shawn O. Pearce, 2007-11-21 06:58 Subject: Re: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions Message-ID: <20071121065858.GU14735@spearce.org> URL: https://gitlist.dev/e/20071121065858.GU14735%40spearce.org In-Reply-To: <200711191947.05960.jnareb@gmail.com> ``` Jakub Narebski wrote: > By the way, do you have idea how to test for the following > in configure.ac: > > * Define NO_PREAD if you have a problem with pread() system call (e.g. > cygwin.dll before v1.5.22). > > - what is the problem? how to detect it? This also appears to be broken on HP-UX (e.g. we need to set NO_PREAD there). Thiago on IRC talked about this the other day when he was trying to build git on a bunch of different systems. Unfortunately this may just need to be an OS based test. The code compiles just fine but when index-pack tries to use pread to walk back through the deltas and generate their SHA-1s it barfs without an error code. I suspect the failure is pread claiming it put the data into the user buffer but not actually doing so, which causes zlib's inflate() to then see data corruption. Ouch. -- Shawn. ``` ## Jakub Narebski, 2007-11-21 18:47 Subject: Re: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions Message-ID: <200711211947.50661.jnareb@gmail.com> URL: https://gitlist.dev/e/200711211947.50661.jnareb%40gmail.com In-Reply-To: <20071121065858.GU14735@spearce.org> ``` Shawn O. Pearce wrote: > Jakub Narebski wrote: >> By the way, do you have idea how to test for the following >> in configure.ac: >> >> * Define NO_PREAD if you have a problem with pread() system call (e.g. >> cygwin.dll before v1.5.22). >> >> - what is the problem? how to detect it? > > This also appears to be broken on HP-UX (e.g. we need to set > NO_PREAD there). Thiago on IRC talked about this the other day > when he was trying to build git on a bunch of different systems. > > Unfortunately this may just need to be an OS based test. The code > compiles just fine but when index-pack tries to use pread to walk > back through the deltas and generate their SHA-1s it barfs without > an error code. I suspect the failure is pread claiming it put the > data into the user buffer but not actually doing so, which causes > zlib's inflate() to then see data corruption. Can you write a short program which demonstrates this error (either fails with some error code, or produces wrong output)? That would be enough for configure.ac (see for example NO_C99_FORMAT in configure.ac) Unless this is Schroedinger bug... -- Jakub Narebski Poland ``` ## Jakub Narebski, 2007-12-05 15:45 Subject: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <1196869526-2197-1-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/1196869526-2197-1-git-send-email-jnareb%40gmail.com In-Reply-To: <7vd4u5l29v.fsf@gitster.siamese.dyndns.org> ``` 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). Signed-off-by: Jakub Narebski --- This patch needs checking if it correctly sets OLD_ICONV when needed. I have checked only that it is not set when with new iconv() declaration. Could people using Cygwin (and other with OLD_ICONV: Darwin) test it? 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. P.S. Is there any convention on where to use YesPlease, and where UnfortunatelyYes when setting Makefile build configuration variables? config.mak.in | 1 + configure.ac | 21 +++++++++++++++++++++ 2 files changed, 22 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..5d2936e 100644 --- a/configure.ac +++ b/configure.ac @@ -212,6 +212,27 @@ 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 + +int main(void) +{ + iconv_t cd; + char *ibp, *obp; + size_t insz, outsz; + iconv(cd, &ibp, &insz, &obp, &outsz); +} +]]) +AC_MSG_CHECKING([for old iconv()]) +AC_COMPILE_IFELSE(OLDICONVTEST_SRC, + [AC_MSG_RESULT([no])], + [AC_MSG_RESULT([yes]) + OLD_ICONV=YesPlease]) +AC_SUBST(OLD_ICONV) ## Checks for typedefs, structures, and compiler characteristics. -- 1.5.3.6 ``` ## Brian Gernhardt, 2007-12-05 16:11 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <82D48BF0-E01D-4CE4-92F3-44B555531624@silverinsanity.com> URL: https://gitlist.dev/e/82D48BF0-E01D-4CE4-92F3-44B555531624%40silverinsanity.com In-Reply-To: <1196869526-2197-1-git-send-email-jnareb@gmail.com> ``` On Dec 5, 2007, at 10:45 AM, Jakub Narebski wrote: > 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). > > Signed-off-by: Jakub Narebski > --- > This patch needs checking if it correctly sets OLD_ICONV > when needed. I have checked only that it is not set when > with new iconv() declaration. Could people using Cygwin > (and other with OLD_ICONV: Darwin) test it? I could not get git am to apply the patch cleanly, but will try again later when I have more time. That said, 10.5's default /usr/include/ iconv.h uses the newer non-const definition of iconv_open. I've had to add a "OLD_ICONV=" line to my config.mak to avoid the warning. I'll still test to look for a false positive. > P.S. Is there any convention on where to use YesPlease, and where > UnfortunatelyYes when setting Makefile build configuration variables? AFAICT, YesPlease is the "normal" choice and UnfortunatelyYes is used when the person who added it considered it an obnoxious hack or a stupid failing in a system. The test in the Makefile is just if the variable is defined, so you technically could use "IDontLikeSillyVariableValuesInMyMakefiles", and it would work just fine. ;-) ~~ Brian ``` ## Wincent Colaiuta, 2007-12-05 16:31 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: URL: https://gitlist.dev/e/ADB12552-AD67-4781-B194-AD15CA4A7B44%40wincent.com In-Reply-To: <1196869526-2197-1-git-send-email-jnareb@gmail.com> ``` El 5/12/2007, a las 16:45, 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). > > Signed-off-by: Jakub Narebski > --- > This patch needs checking if it correctly sets OLD_ICONV > when needed. I have checked only that it is not set when > with new iconv() declaration. Could people using Cygwin > (and other with OLD_ICONV: Darwin) test it? Before applying your patch: CC utf8.o utf8.c: In function ‘reencode_string’: utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible pointer type CC convert.o After applying your patch: CC utf8.o CC convert.o This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1). Cheers, Wincent ``` ## Jakub Narebski, 2007-12-05 16:33 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <200712051733.11590.jnareb@gmail.com> URL: https://gitlist.dev/e/200712051733.11590.jnareb%40gmail.com In-Reply-To: <82D48BF0-E01D-4CE4-92F3-44B555531624@silverinsanity.com> ``` On Wed, 5 Dec 2007, Brian Gernhardt wrote: > On Dec 5, 2007, at 10:45 AM, Jakub Narebski wrote: > > > This patch needs checking if it correctly sets OLD_ICONV > > when needed. I have checked only that it is not set when > > with new iconv() declaration. Could people using Cygwin > > (and other with OLD_ICONV: Darwin) test it? > > I could not get git am to apply the patch cleanly, but will try again > later when I have more time. Hmmm... it should apply cleanly to both 'master' (7a4a2e1f797) and 'next' (bd1cfb7fbddc). It just adds some contents... -- Jakub Narebski Poland ``` ## Jakub Narebski, 2007-12-05 16:52 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <200712051752.28667.jnareb@gmail.com> URL: https://gitlist.dev/e/200712051752.28667.jnareb%40gmail.com In-Reply-To: ``` On Wed, 5 December 2007, Wincent Colaiuta wrote: > El 5/12/2007, a las 16:45, 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). > > > > Signed-off-by: Jakub Narebski > > --- > > This patch needs checking if it correctly sets OLD_ICONV > > when needed. I have checked only that it is not set when > > with new iconv() declaration. Could people using Cygwin > > (and other with OLD_ICONV: Darwin) test it? > > Before applying your patch: > > CC utf8.o > utf8.c: In function ‘reencode_string’: > utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible > pointer type > CC convert.o > > After applying your patch: > > CC utf8.o > CC convert.o Do I understand correctly that above is excerpt from the output of the following sequence of commands before and after this patch applied? $ make configure $ ./configure [options] $ make Do you have something like below in ./configure output? configure: CHECKS for header files checking for old iconv()... yes > This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1). Strange... in Makefile there is ifeq ($(uname_S),Darwin) NEEDS_SSL_WITH_CRYPTO = YesPlease NEEDS_LIBICONV = YesPlease OLD_ICONV = UnfortunatelyYes NO_STRLCPY = YesPlease NO_MEMMEM = YesPlease endif so the uname based guessing should set OLD_ICONV on Darwin... -- Jakub Narebski Poland ``` ## Junio C Hamano, 2007-12-05 19:38 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <7vtzmxortn.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vtzmxortn.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1196869526-2197-1-git-send-email-jnareb@gmail.com> ``` Jakub Narebski writes: > diff --git a/configure.ac b/configure.ac > index 5f8a15b..5d2936e 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -212,6 +212,27 @@ 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 > + > +int main(void) > +{ > + iconv_t cd; > + char *ibp, *obp; > + size_t insz, outsz; > + iconv(cd, &ibp, &insz, &obp, &outsz); > +} > +]]) > +AC_MSG_CHECKING([for old iconv()]) > +AC_COMPILE_IFELSE(OLDICONVTEST_SRC, > + [AC_MSG_RESULT([no])], > + [AC_MSG_RESULT([yes]) > + OLD_ICONV=YesPlease]) > +AC_SUBST(OLD_ICONV) > Which result does COMPILE_IFELSE give for non error warnings? Ok, or Bad? ``` ## Wincent Colaiuta, 2007-12-05 20:49 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <52A4CC8B-EB11-4E3F-A3B6-06826F860E5D@wincent.com> URL: https://gitlist.dev/e/52A4CC8B-EB11-4E3F-A3B6-06826F860E5D%40wincent.com In-Reply-To: <200712051752.28667.jnareb@gmail.com> ``` El 5/12/2007, a las 17:52, Jakub Narebski escribió: > On Wed, 5 December 2007, Wincent Colaiuta wrote: >> >> Before applying your patch: >> >> CC utf8.o >> utf8.c: In function ‘reencode_string’: >> utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible >> pointer type >> CC convert.o >> >> After applying your patch: >> >> CC utf8.o >> CC convert.o > > Do I understand correctly that above is excerpt from the output of the > following sequence of commands before and after this patch applied? > > $ make configure > $ ./configure [options] > $ make Yes, that's right, but with a "make clean" before anything else. > Do you have something like below in ./configure output? > > configure: CHECKS for header files > checking for old iconv()... yes This: configure: CHECKS for header files checking for old iconv()... no >> This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1). > > Strange... in Makefile there is > > ifeq ($(uname_S),Darwin) > NEEDS_SSL_WITH_CRYPTO = YesPlease > NEEDS_LIBICONV = YesPlease > OLD_ICONV = UnfortunatelyYes > NO_STRLCPY = YesPlease > NO_MEMMEM = YesPlease > endif > > so the uname based guessing should set OLD_ICONV on Darwin... That happens *before* config.mak.autogen is included in the Makefile, so it gets overridden. Cheers, Wincent ``` ## Jakub Narebski, 2007-12-05 21:19 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <200712052219.00930.jnareb@gmail.com> URL: https://gitlist.dev/e/200712052219.00930.jnareb%40gmail.com In-Reply-To: <52A4CC8B-EB11-4E3F-A3B6-06826F860E5D@wincent.com> ``` Wincent Colaiuta wrote: > El 5/12/2007, a las 17:52, Jakub Narebski escribió: >> On Wed, 5 December 2007, Wincent Colaiuta wrote: >>> >>> Before applying your patch: >>> >>> CC utf8.o >>> utf8.c: In function ‘reencode_string’: >>> utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible >>> pointer type >>> CC convert.o >>> >>> After applying your patch: >>> >>> CC utf8.o >>> CC convert.o >> >> Do I understand correctly that above is excerpt from the output of the >> following sequence of commands before and after this patch applied? >> >> $ make configure >> $ ./configure [options] >> $ make > > Yes, that's right, but with a "make clean" before anything else. > >> Do you have something like below in ./configure output? >> >> configure: CHECKS for header files >> checking for old iconv()... yes > > This: > > configure: CHECKS for header files > checking for old iconv()... no > >>> This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1). >> >> Strange... in Makefile there is >> >> ifeq ($(uname_S),Darwin) >> NEEDS_SSL_WITH_CRYPTO = YesPlease >> NEEDS_LIBICONV = YesPlease >> OLD_ICONV = UnfortunatelyYes >> NO_STRLCPY = YesPlease >> NO_MEMMEM = YesPlease >> endif >> >> so the uname based guessing should set OLD_ICONV on Darwin... > > That happens *before* config.mak.autogen is included in the Makefile, > so it gets overridden. Ahhh... now I understand. You have installed new iconv() on your computer, and generic 'uname -s' (OS name) based guessing in Makefile guesses wrongly that you need OLD_ICONV, while ./configure script actually tests it and correctly decides to unset OLD_ICONV ! BTW. Perhaps it whould be written more explicitely: +AC_COMPILE_IFELSE(OLDICONVTEST_SRC, + [AC_MSG_RESULT([no]) + OLD_ICONV=], + [AC_MSG_RESULT([yes]) + OLD_ICONV=YesPlease]) -- Jakub Narebski Poland ``` ## Jakub Narebski, 2007-12-05 21:26 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <200712052226.53543.jnareb@gmail.com> URL: https://gitlist.dev/e/200712052226.53543.jnareb%40gmail.com In-Reply-To: <7vtzmxortn.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano wrote: > Jakub Narebski writes: > >> +AC_MSG_CHECKING([for old iconv()]) >> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC, >> + [AC_MSG_RESULT([no])], >> + [AC_MSG_RESULT([yes]) >> + OLD_ICONV=YesPlease]) >> +AC_SUBST(OLD_ICONV) >> > > Which result does COMPILE_IFELSE give for non error warnings? Ok, or > Bad? - Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND]) Run the compiler and compilation flags of the current language (*note Language Choice::) on the INPUT, run the shell commands ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise. The INPUT can be made by `AC_LANG_PROGRAM' and friends. And if I have checked correctly code which causes only warnings returns Ok (in this case print 'no' after 'checking for old iconv()... ' and do not set OLD_ICONV, which means it will be unset). -- Jakub Narebski Poland ``` ## Brian Gernhardt, 2007-12-05 21:38 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <9B4498E3-295F-4D0B-896C-34E54A869B5E@silverinsanity.com> URL: https://gitlist.dev/e/9B4498E3-295F-4D0B-896C-34E54A869B5E%40silverinsanity.com In-Reply-To: <200712052219.00930.jnareb@gmail.com> ``` On Dec 5, 2007, at 4:19 PM, Jakub Narebski wrote: > Ahhh... now I understand. You have installed new iconv() on your > computer, and generic 'uname -s' (OS name) based guessing in Makefile > guesses wrongly that you need OLD_ICONV, while ./configure script > actually tests it and correctly decides to unset OLD_ICONV ! As far as the "installed new iconv()" goes, you may be wrong there. The latest major release of OS X includes a version of iconv that does not need OLD_ICONV as part of the base system. I've had to unset it in my config.mak in order to avoid the warning. ~~ Brian Gernhardt ``` ## Pascal Obry, 2007-12-05 21:46 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <47571C4B.5060907@obry.net> URL: https://gitlist.dev/e/47571C4B.5060907%40obry.net In-Reply-To: <1196869526-2197-1-git-send-email-jnareb@gmail.com> ``` Jakub Narebski a écrit : > Signed-off-by: Jakub Narebski > --- > This patch needs checking if it correctly sets OLD_ICONV > when needed. I have checked only that it is not set when > with new iconv() declaration. Could people using Cygwin > (and other with OLD_ICONV: Darwin) test it? Not working on Cygwin: $ autoconf $ ./configure --prefix=/usr/local --build=i686-pc-cygwin ... configure: CHECKS for header files checking for old iconv()... no It should be yes above. And in config.mak.autogen we have: OLD_ICONV= Note also that you should remove all the hard-coded settings in Makefile anyway. Pascal. -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ``` ## Junio C Hamano, 2007-12-05 22:22 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: <7vd4tkn5mk.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd4tkn5mk.fsf%40gitster.siamese.dyndns.org In-Reply-To: <200712052226.53543.jnareb@gmail.com> ``` Jakub Narebski writes: > Junio C Hamano wrote: >> Jakub Narebski writes: >> >>> +AC_MSG_CHECKING([for old iconv()]) >>> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC, >>> + [AC_MSG_RESULT([no])], >>> + [AC_MSG_RESULT([yes]) >>> + OLD_ICONV=YesPlease]) >>> +AC_SUBST(OLD_ICONV) >>> >> >> Which result does COMPILE_IFELSE give for non error warnings? Ok, or >> Bad? > > - Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND], > [ACTION-IF-NOT-FOUND]) > Run the compiler and compilation flags of the current language > (*note Language Choice::) on the INPUT, run the shell commands > ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise. The INPUT > can be made by `AC_LANG_PROGRAM' and friends. > > And if I have checked correctly code which causes only warnings > returns Ok (in this case print 'no' after 'checking for old iconv()... ' > and do not set OLD_ICONV, which means it will be unset). Which means the real-life compilation will get the warning on type mismatch. Wasn't OLD_ICONV about squelching that? ``` ## Jakub Narebski, 2007-12-05 23:05 Subject: [PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning) Message-ID: <1196895948-25115-1-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/1196895948-25115-1-git-send-email-jnareb%40gmail.com In-Reply-To: <7vd4tkn5mk.fsf@gitster.siamese.dyndns.org> ``` On Wed, 5 Dec 2007, Junio C Hamano wrote: > Jakub Narebski writes: >> Junio C Hamano wrote: >>> Jakub Narebski writes: >>> >>>> +AC_MSG_CHECKING([for old iconv()]) >>>> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC, >>>> + [AC_MSG_RESULT([no])], >>>> + [AC_MSG_RESULT([yes]) >>>> + OLD_ICONV=YesPlease]) >>>> +AC_SUBST(OLD_ICONV) >>>> >>> >>> Which result does COMPILE_IFELSE give for non error warnings? >>> Ok, or Bad? >> >> - Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND], >> [ACTION-IF-NOT-FOUND]) >> Run the compiler and compilation flags of the current language >> (*note Language Choice::) on the INPUT, run the shell commands >> ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise. The INPUT >> can be made by `AC_LANG_PROGRAM' and friends. >> >> And if I have checked correctly code which causes only warnings >> returns Ok (in this case print 'no' after 'checking for old iconv()... ' >> and do not set OLD_ICONV, which means it will be unset). > > Which means the real-life compilation will get the warning on type > mismatch. Wasn't OLD_ICONV about squelching that? Gah, I don't know why I though OLD_ICONV was about compile errors, and not about compile warnings. This version uses -Werror to check for warnings; I hope it doesn't give false positives... On Wed, 5 Dec 2007, Pascal Obry wrote: > Jakub Narebski a écrit : >> --- >> This patch needs checking if it correctly sets OLD_ICONV >> when needed. I have checked only that it is not set when >> with new iconv() declaration. Could people using Cygwin >> (and other with OLD_ICONV: Darwin) test it? > > Not working on Cygwin: > > $ autoconf > $ ./configure --prefix=/usr/local --build=i686-pc-cygwin > ... > configure: CHECKS for header files > checking for old iconv()... no > > It should be yes above. And in config.mak.autogen we have: > > OLD_ICONV= Check out current version of patch. It should work correctly now (I thought OLD_ICONV was about compile errors, and it is about squelching compile warnings). It should give now: $ make configure $ ./configure --prefix=/usr/local --build=i686-pc-cygwin configure: CHECKS for header files checking for old iconv()... yes $ cat config.mak.autogen OLD_ICONV=UnfortunatelyYes > Note also that you should remove all the hard-coded settings > in Makefile anyway. No, I should not. ./configure script is purely optional in git, and compiling should work with reasonable defaults even if you don't have autoconf installed and/or you don't want to run ./configure script (because e.g. it is too slow). -- >8 -- From: Jakub Narebski Subject: [PATCH] autoconf: Add test for OLD_ICONV (squelching compiler warning) 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: Junio C Hamano Signed-off-by: Jakub Narebski --- config.mak.in | 1 + configure.ac | 24 ++++++++++++++++++++++++ 2 files changed, 25 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..196ab3e 100644 --- a/configure.ac +++ b/configure.ac @@ -212,6 +212,30 @@ 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 + +int main(void) +{ + iconv_t cd; + char *ibp, *obp; + size_t insz, outsz; + iconv(cd, &ibp, &insz, &obp, &outsz); +} +]]) +AC_MSG_CHECKING([for old iconv()]) +CFLAGS_ORIG=$CFLAGS +CFLAGS="$CFLAGS -Werror" +AC_COMPILE_IFELSE(OLDICONVTEST_SRC, + [AC_MSG_RESULT([no])], + [AC_MSG_RESULT([yes]) + OLD_ICONV=UnfortunatelyYes]) +CFLAGS=$CFLAGS_ORIG +AC_SUBST(OLD_ICONV) ## Checks for typedefs, structures, and compiler characteristics. -- 1.5.3.7 ``` ## Junio C Hamano, 2007-12-05 23:27 Subject: Re: [PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning) Message-ID: <7vd4tklo2r.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd4tklo2r.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1196895948-25115-1-git-send-email-jnareb@gmail.com> ``` Jakub Narebski writes: >> Which means the real-life compilation will get the warning on type >> mismatch. Wasn't OLD_ICONV about squelching that? > > Gah, I don't know why I though OLD_ICONV was about compile errors, and > not about compile warnings. This version uses -Werror to check for > warnings; I hope it doesn't give false positives... But use of -Werror means you are married to gcc, doesn't it? How important is it to detect OLD_ICONV anyway, I have to wonder? > On Wed, 5 Dec 2007, Pascal Obry wrote: > ... >> Note also that you should remove all the hard-coded settings >> in Makefile anyway. > > No, I should not. ./configure script is purely optional in git, Correct. Thanks. ``` ## Wincent Colaiuta, 2007-12-06 19:07 Subject: Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV Message-ID: URL: https://gitlist.dev/e/BCD45527-C564-4CD3-9797-36F6251377E6%40wincent.com In-Reply-To: <200712052219.00930.jnareb@gmail.com> ``` El 5/12/2007, a las 22:19, Jakub Narebski escribió: > Ahhh... now I understand. You have installed new iconv() on your > computer, and generic 'uname -s' (OS name) based guessing in Makefile > guesses wrongly that you need OLD_ICONV, while ./configure script > actually tests it and correctly decides to unset OLD_ICONV ! No, nothing installed. It's that Mac OS X 10.5 (Leopard)/Darwin 9 comes with a newer version of iconv. So while there was no warning prior to Leopard there is one now, which your patch fixes for those using configure. This came up before here: Still, I'd like to fix the warning for those who don't use configure. Have posted a separate patch for that just now. Cheers, > Wincent ```