How does this look for another attempt at the configure file?

On Tue, Nov 18, 2014 at 3:55 PM, Darshit Shah <[email protected]> wrote:
> On 11/18, Tim Rühsen wrote:
>>
>> On Tuesday 18 November 2014 09:51:18 Darshit Shah wrote:
>>>
>>> On 11/17, Tim Rühsen wrote:
>>> >On Monday 17 November 2014 19:39:12 Darshit Shah wrote:
>>> >> On 11/17, Tim Rühsen wrote:
>>> >> >On Monday 17 November 2014 09:48:30 Darshit Shah wrote:
>>> >> >> On 11/16, Tim Rühsen wrote:
>>> >> >> >Am Sonntag, 16. November 2014, 16:59:18 schrieb Darshit Shah:
>>> >> >> >> I cleanup up configure.ac slightly.
>>> >> >> >>
>>> >> >> >> It's not complete, but this is in my opinion a step up from what
>>> >> >> >> we
>>> >> >> >> had
>>> >> >> >> already. If everyone is fine with it, I'll push this version of
>>> >> >> >> the
>>> >> >> >> patch.
>>> >> >> >>
>>> >> >> >> The first patch adds the -Wextra CFLAG by default on systems
>>> >> >> >> where
>>> >> >> >> a
>>> >> >> >> CFLAG
>>> >> >> >> variable is not already defined.
>>> >> >> >
>>> >> >> >It works for me, just a comment.
>>> >> >> >
>>> >> >> >configure.ac seems to have 2 spaces indentation - now we have 4
>>> >> >> >spaces
>>> >> >> >mixed with 2 spaces indentation. At some places the indentation is
>>> >> >> >wrong.
>>> >> >> >I personally prefer 2 spaces - but it really doesn't matter. Maybe
>>> >> >> >you
>>> >> >> >amend configure.ac so that we have the same indentation everywhere
>>> >> >> > !?
>>> >> >>
>>> >> >> You're right. The indentation and formatting is quite a bit off. In
>>> >> >> fact,
>>> >> >> that is one reason which is preventing any further work on the
>>> >> >> configure.ac
>>> >> >> file for me.
>>> >> >>
>>> >> >> I'm stuck and unable to understand how that code ought to be
>>> >> >> formatted.
>>> >> >> I'll look into it once again and see if I can finally format it
>>> >> >> with
>>> >> >> two
>>> >> >> spaces and other stylistic guidelines.
>>> >> >
>>> >> >I prefer 2 spaces indent (but just make your own choice). As long as
>>> >> > the
>>> >> >formatting is consistent throughout the whole file.
>>> >>
>>> >> Forget it. This works right now and I'm not going to touch the
>>> >> configure
>>> >> code unless really required.
>>> >>
>>> >> Slight changes are breaking the entire build system for me. Trash
>>> >> these
>>> >> patches.
>>> >
>>> >Indentation should not break anything !?
>>> >I had issues when changing configure.ac as well - though they all were
>>> > my
>>> >own faults, either f*cking up brackets or macro misspelling (e.g. AS_IF
>>> >instead AC_IF). These things are often reported far away from the point
>>> >where they are. So you stare and stare and can't see anything.
>>> >My strategy now is to autoreconf after each little change and if it
>>> > breaks,
>>> >CTRL-z until it works again, than continue changes at that point...
>>> >
>>> >But of course, don't waste your time with these things.
>>>
>>> Not indentation. But simply moving the libpsl detection code from its
>>> current location to where zlib and openssl are detected causes a spurious
>>> failure in the generation of the LIBS variable. It's very weird and after
>>> a
>>> couple of hours of debugging, I simply gave up. The effort wasn't worth
>>> it.
>>
>>
>> I would not call this spurious. If you added pkc-config code to detect
>> libpsl,
>> you will experience problems due to a broken pkg-config file. Dependant on
>> the
>> position in configure.ac. Simply look at config.log and you'll see what
>> the
>> problem is (e.g. -llibpsl can't be found).
>
>
> I removed the pkg-config code for libpsl and went back to the old
> PKG_SEARCH_LIBS code. The issue was that if the check for libpsl,
> irrespective of how it is done, is done later in the file, with zlib and
> openssl, a dot (.) was added very randomly, which caused the Makefile to
> attempt to compile ftp-opie..c instead of ftp-opie.c. I'll re-apply my
> patch, and share my output in a while.
>
>>
>> The pkg-config bug is fixed since v0.6.0 (latest libpsl release is 0.6.2)
>> -
>> ask your distribution to upgrade it. I asked my Debian sponsor (needed for
>> package review and uploading), but no reaction yet.
>>
>
> I'm the one maintaining libpsl on Arch Linux's AUR. It hasn't been included
> in the official repositories yet. And Arch isn't using Libpsl with Wget 1.16
> either. I plan on opening a bug report for that, but its a discussion for
> another time.
>
>> As a workaround - here is the code from Mget's configure.ac:
>> AC_ARG_WITH(libpsl, AS_HELP_STRING([--without-libpsl], [disable support
>> for
>> libpsl cookie checking]), with_libpsl=$withval, with_libpsl=yes)
>> AS_IF([test "x$with_libpsl" != xno], [
>>  PKG_CHECK_MODULES([LIBPSL], libpsl, [
>>    with_libpsl=yes
>>    # correct $LIBPSL_LIBS (in libpsl <= 0.6.0)
>>    AS_IF([test "x$LIBPSL_LIBS" = "x-llibpsl "], [LIBPSL_LIBS="-lpsl"])
>>    LIBS="$LIBPSL_LIBS $LIBS"
>>    CFLAGS="$LIBPSL_CFLAGS $CFLAGS"
>>    AC_DEFINE([WITH_LIBPSL], [1], [PSL support enabled])
>>  ], [
>>    AC_SEARCH_LIBS(psl_builtin, psl,
>>      [with_libpsl=yes; AC_DEFINE([WITH_LIBPSL], [1], [PSL support
>> enabled])],
>>      [with_libpsl=no;  AC_MSG_WARN(*** libpsl was not found. Fallback to
>> builtin cookie checking.)])
>>  ])
>> ])
>
>
> Thanks for the code, I'll check it out.
>>
>>
>> Tim
>
>
>
> --
> Thanking You,
> Darshit Shah



-- 
Thanking You,
Darshit Shah
From a94a980633f5f4a115fc5acc67348072c5e6ff08 Mon Sep 17 00:00:00 2001
From: Darshit Shah <[email protected]>
Date: Thu, 20 Nov 2014 13:58:02 +0530
Subject: [PATCH] Autotoolize configure.ac

---
 ChangeLog    |   4 ++
 configure.ac | 117 +++++++++++++++++++++++++++++++++++++----------------------
 2 files changed, 78 insertions(+), 43 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0be7e9f..a3dc9b4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2014-11-20  Darshit Shah  <[email protected]>
+
+	* configure.ac: Use autoconf macros where available
+
 2014-11-19  Tim Ruehsen <[email protected]>
 
 	* configure.ac: Check for random()
diff --git a/configure.ac b/configure.ac
index e589b93..f1065ee 100644
--- a/configure.ac
+++ b/configure.ac
@@ -65,68 +65,99 @@ dnl maintainers to compile without assertions.
 AC_HEADER_ASSERT
 
 dnl
-dnl Process features.
+dnl Process External Libraries
 dnl
 
-AC_ARG_WITH(libpsl,
-    AS_HELP_STRING([--without-libpsl],
-                   [disable support for libpsl cookie checking.]),
-    [],
-    [with_libpsl=yes])
+dnl Libpsl: Public Suffix List checking
+AC_ARG_WITH([libpsl],
+  [AS_HELP_STRING([--without-libpsl], [disable support for libpsl cookie checking.])],
+  [],
+  [with_libpsl=yes]
+)
 
 AS_IF([test "x$with_libpsl" = xyes],
-    [AC_SEARCH_LIBS(psl_builtin, psl,
-                    [ENABLE_PSL=yes; AC_DEFINE([HAVE_LIBPSL], [1], [PSL Support Enabled])],
-                    [ENABLE_PSL=no; AC_MSG_WARN(*** libpsl not found. Falling back to Wget builtin cookie checking.)])],
-    [ENABLE_PSL=no])
-
-AC_ARG_WITH(ssl,
-[[  --without-ssl           disable SSL autodetection
-  --with-ssl={gnutls,openssl} specify the SSL backend.  GNU TLS is the default.]])
+  [AC_SEARCH_LIBS(psl_builtin, psl,
+    [ENABLE_PSL=yes; AC_DEFINE([HAVE_LIBPSL], [1], [PSL Support Enabled])],
+    [ENABLE_PSL=no; AC_MSG_WARN(*** libpsl not found. Falling back to Wget builtin cookie checking.)])],
+  [ENABLE_PSL=no]
+)
 
-AC_ARG_WITH(zlib,
-[[  --without-zlib          disable zlib ]])
 
-AC_ARG_ENABLE(opie,
-[  --disable-opie          disable support for opie or s/key FTP login],
-ENABLE_OPIE=$enableval, ENABLE_OPIE=yes)
-test x"${ENABLE_OPIE}" = xyes && AC_DEFINE([ENABLE_OPIE], 1,
-   [Define if you want the Opie support for FTP compiled in.])
+dnl SSL: Configure SSL backend to use
+AC_ARG_WITH([ssl],
+  [AS_HELP_STRING([--with-ssl={gnutls,openssl}], [specify SSL backend. GNU TLS is the default.])])
 
-AC_ARG_ENABLE(digest,
-[  --disable-digest        disable support for HTTP digest authorization],
-ENABLE_DIGEST=$enableval, ENABLE_DIGEST=yes)
-test x"${ENABLE_DIGEST}" = xyes && AC_DEFINE([ENABLE_DIGEST], 1,
-   [Define if you want the HTTP Digest Authorization compiled in.])
 
-AC_ARG_ENABLE(ntlm,
-[  --disable-ntlm          disable support for NTLM authorization],
-[ENABLE_NTLM=$enableval], [ENABLE_NTLM=auto])
+dnl Zlib: Configure use iof zlib for compression
+AC_ARG_WITH([zlib],
+  [AS_HELP_STRING([--without-zlib], [disable zlib.])])
 
-AC_ARG_ENABLE(debug,
-[  --disable-debug         disable support for debugging output],
-ENABLE_DEBUG=$enableval, ENABLE_DEBUG=yes)
-test x"${ENABLE_DEBUG}" = xyes && AC_DEFINE([ENABLE_DEBUG], 1,
-   [Define if you want the debug output support compiled in.])
 
 dnl
-dnl Check for valgrind
+dnl Process features
 dnl
+
+dnl Opie: Support for opie s/key FTP logins
+AC_ARG_ENABLE([opie],
+  [AS_HELP_STRING([--disable-opie], [disable support for opie or s/key FTP login])],
+  [ENABLE_OPIE=$enableval],
+  [ENABLE_OPIE=yes])
+
+AS_IF([test "x$ENABLE_OPIE" = xyes],
+  [AC_DEFINE([ENABLE_OPIE], [1], [Define if you want Opie support for FTP compiled in.])],
+  []
+)
+
+
+dnl Digest: Support for HTTP Digest Authentication
+AC_ARG_ENABLE([digest],
+  [AS_HELP_STRING([--disable-digest], [disable support for HTTP digest authorization])],
+  [ENABLE_DIGEST=$enableval],
+  [ENABLE_DIGEST=yes])
+
+AS_IF([test "x$ENABLE_DIGEST" = xyes],
+  [AC_DEFINE([ENABLE_DIGEST], [1], [Define if you want the HTTP Digest Authorization compiled in.])],
+  []
+)
+
+
+dnl NTLM: Support for HTTP NTLM Authentication
+AC_ARG_ENABLE([ntlm],
+  [AS_HELP_STRING([--disable-ntlm], [disable support for NTLM authorization])],
+  [ENABLE_NTLM=$enableval],
+  [ENABLE_NTLM=auto]
+)
+
+
+dnl Debug: Support for printing debugging output
+AC_ARG_ENABLE([debug],
+  [AS_HELP_STRING([--disable-debug], [disable support for debugging output])],
+  [ENABLE_DEBUG=$enableval],
+  [ENABLE_DEBUG=yes])
+
+AS_IF([test "x$ENABLE_DEBUG" = xyes],
+  [AC_DEFINE([ENABLE_DEBUG], [1], [Define if you want the debug output support compiled in.])],
+  []
+)
+
+dnl Valgrind-tests: Should test suite be run under valgrind?
 AC_ARG_ENABLE(valgrind-tests,
-  AS_HELP_STRING([--enable-valgrind-tests], [enable using Valgrind for tests]),
-  [ac_enable_valgrind=$enableval], [ac_enable_valgrind=no])
-if test "${ac_enable_valgrind}" != "no" ; then
+  [AS_HELP_STRING([--enable-valgrind-tests], [enable using Valgrind for tests])],
+  [ENABLE_VALGRIND=$enableval],
+  [ENABLE_VALGRIND=no])
+
+AS_IF([test "x$ENABLE_VALGRIND" != xno], [
   AC_CHECK_PROG(HAVE_VALGRIND, valgrind, yes, no)
-  if test "$HAVE_VALGRIND" = "yes" ; then
+  AS_IF([test "x$HAVE_VALGRIND" = xyes], [
     VALGRIND_TESTS="1"
     AC_SUBST(VALGRIND_TESTS)
     VALGRIND_INFO="Test suite will be run under Valgrind"
-  else
+  ], [
     VALGRIND_INFO="Valgrind not found"
-  fi
-else
+  ])
+], [
   VALGRIND_INFO="Valgrind testing not enabled"
-fi
+])
 
 dnl
 dnl Find the compiler
-- 
2.1.3

Reply via email to