Re: [Bug-wget] Configure cleanup

[Tim Ruehsen]

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

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.

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.)])
  ])
]) 

Tim

signature.asc
Description: This is a digitally signed message part.