Re: [Bug-wget] Configure cleanup

[Darshit Shah]

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

On Tue, Nov 18, 2014 at 3:55 PM, Darshit Shah <address@hidden> 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

0001-Autotoolize-configure.ac.patch
Description: Text Data