threads / patch / 10933

patchautoconf: Add tests for memmem, strtoumax and mkdtemp functions

Subject: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions

## tl;dr

20 messages between Nov 19, 2007 and Dec 6, 2007. Diffs are folded; open one to read it.

replies: 19people: 8as markdown or json

Jakub Narebski· Nov 19, 2007, 18:47 UTC · lore

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 <jnareb@gmail.com>
---
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(-)
Show changes to 2 files +21 −0

config.mak.in, configure.ac

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
Shawn O. Pearce· Nov 21, 2007, 06:58 UTC · re: Jakub Narebski · lore

Re: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions

Jakub Narebski <jnareb@gmail.com> wrote:
Show 7 quoted lines
> 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· Nov 21, 2007, 18:47 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions

Shawn O. Pearce wrote:
Show 19 quoted lines
> Jakub Narebski <jnareb@gmail.com> 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
Stefan-W. Hahn· Nov 20, 2007, 18:12 UTC · lore

Re: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions

Also sprach Junio C Hamano am Tue, 20 Nov 2007 at 01:00:44 -0800:
Show 13 quoted lines
> Jakub Narebski <jnareb@gmail.com> 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· Nov 21, 2007, 00:08 UTC · lore

Re: [PATCH] autoconf: Add tests for memmem, strtoumax and mkdtemp functions

On Tue, Nov 20, 2007 at 01:00:44AM -0800, Junio C Hamano wrote:
Show 9 quoted lines
> Jakub Narebski <jnareb@gmail.com> 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.

Show 33 quoted lines
> >   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
Jakub Narebski· Dec 5, 2007, 15:45 UTC · lore

[PATCH/RFC] autoconf: Add test for OLD_ICONV

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 <jnareb@gmail.com>
---
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(-)
Show changes to 2 files +22 −0

config.mak.in, configure.ac

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 <iconv.h>
+
+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· Dec 5, 2007, 16:11 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

On Dec 5, 2007, at 10:45 AM, Jakub Narebski wrote:
Show 11 quoted lines
> 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 <jnareb@gmail.com>
> ---
> 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
Jakub Narebski· Dec 5, 2007, 16:33 UTC · re: Brian Gernhardt · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

On Wed, 5 Dec 2007, Brian Gernhardt wrote:
Show 9 quoted lines
> 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
Wincent Colaiuta· Dec 5, 2007, 16:31 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

El 5/12/2007, a las 16:45, Jakub Narebski escribió:
Show 11 quoted lines
> 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 <jnareb@gmail.com>
> ---
> 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· Dec 5, 2007, 16:52 UTC · re: Wincent Colaiuta · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

On Wed, 5 December 2007, Wincent Colaiuta wrote:
Show 26 quoted lines
> 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 <jnareb@gmail.com>
> > ---
> > 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
Wincent Colaiuta· Dec 5, 2007, 20:49 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

El 5/12/2007, a las 17:52, Jakub Narebski escribió:
Show 21 quoted lines
> 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

Show 13 quoted lines
>> 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· Dec 5, 2007, 21:19 UTC · re: Wincent Colaiuta · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Wincent Colaiuta wrote:
Show 51 quoted lines
> 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
Brian Gernhardt· Dec 5, 2007, 21:38 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

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
Wincent Colaiuta· Dec 6, 2007, 19:07 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

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:
<http://marc.info/?l=git&m=119397507006026&w=2>

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
Junio C Hamano· Dec 5, 2007, 19:38 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Jakub Narebski <jnareb@gmail.com> writes:
Show 30 quoted lines
> 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 <iconv.h>
> +
> +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?

Jakub Narebski· Dec 5, 2007, 21:26 UTC · re: Junio C Hamano · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Junio C Hamano wrote:
Show 12 quoted lines
> Jakub Narebski <jnareb@gmail.com> 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
Junio C Hamano· Dec 5, 2007, 22:22 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Jakub Narebski <jnareb@gmail.com> writes:
Show 24 quoted lines
> Junio C Hamano wrote:
>> Jakub Narebski <jnareb@gmail.com> 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· Dec 5, 2007, 23:05 UTC · re: Junio C Hamano · lore

[PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning)

On Wed, 5 Dec 2007, Junio C Hamano wrote:
Show 28 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
>> Junio C Hamano wrote:
>>> Jakub Narebski <jnareb@gmail.com> 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:
Show 18 quoted lines
> 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 <jnareb@gmail.com>
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 <gitster@pobox.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 config.mak.in |    1 +
 configure.ac  |   24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)
Show changes to 2 files +25 −0

config.mak.in, configure.ac

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 <iconv.h>
+
+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· Dec 5, 2007, 23:27 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning)

Jakub Narebski <jnareb@gmail.com> writes:
Show 6 quoted lines
>> 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?
Show 6 quoted lines
> 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.
Pascal Obry· Dec 5, 2007, 21:46 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Jakub Narebski a écrit :
Show 6 quoted lines
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> 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

← back to recent threads