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

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

[Jakub Narebski]

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(-)

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

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

[Stefan-W. Hahn]

Also sprach Junio C Hamano am Tue, 20 Nov 2007 at 01:00:44 -0800:
> 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.			

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

[Matt Kraai]

On Tue, Nov 20, 2007 at 01:00:44AM -0800, Junio C Hamano wrote:
> 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.

> >   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

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

[Shawn O. Pearce]

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.

Ouch.

-- 
Shawn.

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

[Jakub Narebski]

Shawn O. Pearce wrote:
> 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

[PATCH/RFC] autoconf: Add test for OLD_ICONV

[Jakub Narebski]

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(-)

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

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

[Brian Gernhardt]

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 <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

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

[Wincent Colaiuta]

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

This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1).

Cheers,
Wincent

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

[Jakub Narebski]

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

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

[Jakub Narebski]

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 <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

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

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> 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 <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?

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

[Wincent Colaiuta]

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

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

[Jakub Narebski]

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

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

[Jakub Narebski]

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).
-- 
Jakub Narebski
Poland

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

[Brian Gernhardt]

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

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

[Pascal Obry]

Jakub Narebski a écrit :
> 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

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

[Junio C Hamano]

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?

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

[Jakub Narebski]

On Wed, 5 Dec 2007, Junio C Hamano wrote:
> 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:
> 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(-)

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

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

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> 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.

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

[Wincent Colaiuta]

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