bug-wget
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-wget] Configure cleanup


From: Darshit Shah
Subject: Re: [Bug-wget] Configure cleanup
Date: Tue, 18 Nov 2014 15:55:27 +0530
User-agent: Mutt/1.5.23 (2014-03-12)

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]