Re: [PATCH] fix handling of iconv configuration options

Marco Nelissen [off-list ref] writes:

Hmm, have you tested this with both NO_ICONV defined and undefined?

Because ...

	$ test -z ; echo $?
        0
	$ test -n ; echo $?
        0

... I would feel better if you had dq around $NO_ICONV in both tests.

On Mon, Jun 8, 2009 at 2:50 PM, Junio C Hamano[off-list ref] wrote:

Marco Nelissen [off-list ref] writes:

quoted

diff --git a/configure.ac b/configure.ac
index 108a97f..3388036 100644
--- a/configure.ac
+++ b/configure.ac

@@ -385,6 +385,8 @@ AC_SUBST(NO_EXPAT)

 # some Solaris installations).
 # Define NO_ICONV if neither libc nor libiconv support iconv.

+if test -z $NO_ICONV; then
+
 GIT_STASH_FLAGS($ICONVDIR)

 AC_DEFUN([ICONVTEST_SRC], [

@@ -431,6 +433,12 @@ GIT_UNSTASH_FLAGS($ICONVDIR)

 AC_SUBST(NEEDS_LIBICONV)
 AC_SUBST(NO_ICONV)

+if test -n $NO_ICONV; then
+    NEEDS_LIBICONV=
+fi
+
+fi
+

Hmm, have you tested this with both NO_ICONV defined and undefined?

Because ...

       $ test -z ; echo $?
       0
       $ test -n ; echo $?
       0

... I would feel better if you had dq around $NO_ICONV in both tests.

Ah, you're right. I tested that it didn't incorrectly set
NEEDS_LIBICONV on a system that doesn't have/need it, but didn't test
that it doesn't unset it on a system that does need it (because I
don't have such a system).