Re: [Bug-wget] Building and testing wget 1.16.1 on MinGW

[Tim Ruehsen]

Hi Eli,

thanks for your detailed report(s) !

See my answers inline.

Tim

On Friday 19 December 2014 13:12:22 Eli Zaretskii wrote:
> (I'm not subscribed to the list, so please CC me on any replies and
> discussions.  Thanks in advance.)
> 
> I've built wget 1.16.1 with mingw.org's toolchain, and encountered a
> few issues, described below.
> 
> First, the configure script's summary report, for the reference:
> 
>      configure: Summary of build options:
> 
>        Version:           1.16.1
>        Host OS:           mingw32
>        Install prefix:    d:/usr
>        Compiler:          gcc
>        CFlags:            -Id:/usr/include   -Id:/usr/include  
> -DHAVE_LIBGNUTLS -Id:/usr/include   -Id:/usr/include   -DNDEBUG -Og -g3
> -Id:/usr/include LDFlags:
>        Libs:              -Ld:/usr/lib -lpcre   -lnettle -Ld:/usr/lib
> -lgnutls   -Ld:/usr/lib -lz   -Ld:/usr/lib -lpsl    -lws2_32 -lidn SSL:    
>           gnutls
>        Zlib:              yes
>        PSL:               yes
>        Digest:            yes
>        NTLM:              yes
>        OPIE:              yes
>        Debugging:         yes
>        Assertions:        no
>        Valgrind:          Valgrind testing not enabled
> 
> As you see, this build uses GnuTLS, Libidn, Libpsl, and zlib, so most
> (all?) optional features are enabled.
> 
> Now the problems I bumped into:
> 
> 1. Compiler warning in warc.c:
> 
>      warc.c: In function 'warc_write_warcinfo_record':
>      warc.c:677:3: warning: passing argument 1 of 'strdup' makes pointer
> from integer without a cast [enabled by default] In file included from
> ../lib/string.h:27:0,
>                     from
> d:\usr\bin\../lib/gcc/mingw32/4.7.2/../../../../include/winnt.h:37, from
> d:\usr\bin\../lib/gcc/mingw32/4.7.2/../../../../include/windef.h:253, from
> d:\usr\bin\../lib/gcc/mingw32/4.7.2/../../../../include/windows.h:48, from
> mswindows.h:44,
>                     from sysdep.h:98,
>                     from wget.h:47,
>                     from warc.c:34:
>      d:\usr\bin\../lib/gcc/mingw32/4.7.2/../../../../include/string.h:92:39:
> note: expected 'const char *' but argument is of type 'int'
> 
>    This is because warc.c includes libgen.h only on non-Windows
>    systems, and the prototype for basename is declared on MinGW's libgen.h.
> 
>    Proposed solution:
> 
> --- src/warc.c~0      2014-12-02 09:49:37.000000000 +0200
> +++ src/warc.c        2014-12-19 12:16:25.827125000 +0200
> @@ -54,10 +54,11 @@ as that of the covered work.  */
>  #include <uuid.h>
>  #endif
> 
> -#ifndef WINDOWS
> -#include <libgen.h>
> -#else
> -#include <fcntl.h>
> +#if !defined WINDOWS || defined __MINGW32__
> +# include <libgen.h>
> +#endif
> +#ifdef WINDOWS
> +# include <fcntl.h>
>  #endif
> 
>  #include "warc.h"

The man page for basename says that there is a POSIX-2001 and a GNU version.
The POSIX version does not allow string literals - the function writes into 
the argument string :-(
So I guess we should take basename from gnulib to have a consistent (and 
graceful) behavior. 


> 2. Compilation error in utils.c:
> 
>      utils.c:68:21: fatal error: termios.h: No such file or directory
>      compilation terminated.
>      Makefile:1542: recipe for target `utils.o' failed
>      make[3]: *** [utils.o] Error 1
> 
>    This is because termios.h is included unconditionally, although a
>    workaround for Windows is already in the code.
> 
>    Proposed solution: include termios.h conditionally:
> 
> --- src/utils.c~0     2014-11-23 18:49:06.000000000 +0200
> +++ src/utils.c       2014-12-19 12:13:57.217750000 +0200
> @@ -64,8 +64,10 @@ as that of the covered work.  */
>  #include <sys/stat.h>
> 
>  /* For TIOCGWINSZ and friends: */
> -#include <sys/ioctl.h>
> -#include <termios.h>
> +#ifndef WINDOWS
> +# include <sys/ioctl.h>
> +# include <termios.h>
> +#endif
> 
>  /* Needed for Unix version of run_with_timeout. */
>  #include <signal.h>

Looks good to me.


> 3. Compilation warning and link error due to lack of strcasestr:
> 
>      iri.c: In function 'parse_charset':
>      iri.c:59:7: warning: assignment makes pointer from integer without a
> cast [enabled by default] str = strcasestr (str, "charset=");
>           ^
>      iri.o: In function `parse_charset':
>      d:\gnu\wget-1.16.1\src/iri.c:59: undefined reference to `strcasestr'
>      collect2.exe: error: ld returned 1 exit status
>      Makefile:1465: recipe for target `wget.exe' failed
>      make[3]: *** [wget.exe] Error 1
> 
>    This is because strcasestr isn't compiled into libgnu.a, even
>    though it seems to be probed by the configure script:
> 
>      checking whether strcasestr is declared without a macro... no
> 
>    I kludged around this, but the real solution should be to force
>    gnulib to compile strcasestr.c int libgnu.a on MinGW.  Sorry, I
>    don't know enough about gnulib to suggest a patch.

This issue should already been solved. Meanwhile we use c_strcasestr in iri.c 
(from gnulib). Please try the latest git version of Wget.


> 4. "make check" fails to link test programs, such as unit-tests.exe,
>    because it doesn't link against libintl.  Again, not sure how best
>    to fix that (wget itself does get linked against libintl and
>    libiconv).

Dunno.


> 5. Example wgetrc has many incorrect commands.  My humble attempt to
>    fix that:
> 
> --- doc/sample.wgetrc~        2014-12-15 09:48:17.000000000 +0200
> +++ doc/sample.wgetrc 2014-12-15 12:45:16.778000300 +0200
> @@ -52,7 +52,7 @@
>  # downloads, set waitretry to maximum number of seconds to wait (Wget
>  # will use "linear backoff", waiting 1 second after the first failure
>  # on a file, 2 seconds after the second failure, etc. up to this max).
> -#waitretry = 10
> +#wait_retry = 10
> 
> 
>  ##
> @@ -116,19 +116,19 @@
>  #follow_ftp = off
> 
>  # To try ipv6 addresses first:
> -#prefer-family = IPv6
> +#prefer_family = IPv6
> 
>  # Set default IRI support state
>  #iri = off
> 
>  # Force the default system encoding
> -#locale = UTF-8
> +local_encoding = CP1255
> 
>  # Force the default remote server encoding
> -#remoteencoding = UTF-8
> +#remote_encoding = UTF-8
> 
>  # Turn on to prevent following non-HTTPS links when in recursive mode
> -#httpsonly = off
> +#httpsonly = off  ??? doesn't seem to exist
> 
>  # Tune HTTPS security (auto, SSLv2, SSLv3, TLSv1, PFS)
> -#secureprotocol = auto
> +#secure_protocol = auto

Commands work either way:
remoteencoding = UTF-8
remote-encoding = UTF-8
remote_encoding = UTF-8

> +#httpsonly = off  ??? doesn't seem to exist
Only exists when HAVE_SSL is defined.

> +#locale = ...
Yes, this is not correct and should be fixed.


> 6. Some tests fail.  I'm still looking into the failures, trying to
>    figure out which ones are real, but here are a couple of
>    conclusions I already made:
> 
>    . Test-N-* tests fail because the test suite sets the timestamp of
>      the file to a date in Oct 2004, when the DST rules were not what
>      the MS runtime implements.  My solution was to "export TZ=IST-2"
>      before running the tests, but I wonder whether using a date close
>      to DST change is really necessary here.
> 
>    . Test-k fails because ':' is encoded as %3A on Windows, so this is
>      not a real failure.  Again, is using ':' really necessary here?

I'll have to look into these tests with more time.

>    Also, I'd appreciate any guidance as to how to enable debugging
>    output from the test scripts/programs, or any other debugging
>    tricks for the test suite.  I'm having hard time debugging each
>    failure, and I wonder whether I missed some facilities that could
>    help.

# to run a single test
cd tests
./Test-N-old.px
or 
make check TESTS="Test-N-old"

If you want debugging output, I add -d directly into the .px file (my $cmdline 
= ...).

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