bug-wget
[Top][All Lists]
Advanced

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

Re: [Bug-wget] [PATCH] Script to check Wget status (for developers)


From: Tim Ruehsen
Subject: Re: [Bug-wget] [PATCH] Script to check Wget status (for developers)
Date: Thu, 11 Dec 2014 16:27:17 +0100
User-agent: KMail/4.14.2 (Linux/3.16.0-4-amd64; KDE/4.14.2; x86_64; ; )

> >+export CFLAGS="-stdÈ9 -pedantic -O2 -g -Wall -Wextra -Wstrict-prototypes
> >-Wold-style-definition -Wwrite-strings -Wshadow -Wformat -Wformat-security
> >-Wunreachable-code -Wstrict-prototypes -Wmissing-prototypes
> >-Wold-style-definition"
> Are we still going for C89 compatibility? I've been setting the -std=gnu99
> flag till now. Maybe that's why I see fewer errors than you.

Yes, we made some effort before the release to have everything C89 compliant.

> Except for Windows, for what other systems do we need to maintain strict C89
> compliance?

Who knows ? But Windows alone is a hard-to-beat argument.
Just some error messages on the list popped up regarding Solaris 10... well,
that system will be maintained until end of 2024... and will exist in the real
world at least 10 years longer. Solaris 8 and 9 is tagged as "Sustaining
Support Indefinite".

You know, I personally prefer C99+Posix. But let's keep C89 for Wget 1 and
change it for Wget 2.

> >+  for options in "" "--with-ssl=openssl"; do
>
> We probably want to add a couple more such variations of options here.
> Specifically, check both openssl and gnutls backends.

"" checks GnuTLS (it is the default). So maybe --without-ssl would be more
appropriate !?
Maybe --disable-nls ?

We definitely can't check all combinations... we should determine some
reasonable combinations.

> >+    export DISTCHECK_CONFIGURE_FLAGS="-C --cache-file=$CACHEFILE $options"
> >+    echo "  ./configure $DISTCHECK_CONFIGURE_FLAGS"
> >+    ./configure $DISTCHECK_CONFIGURE_FLAGS >/dev/null
> >+
> >+    for xVALGRIND in 0 1; do
> >+      for xLCALL in C tr_TR.utf8; do
>
> This fails when the user doesn't already have the tr_TR.UTF-8 locale set up.
> The script should not handle doing that, but it should atleast check and
> report to the user if the locale isn't available.

This script is for developers only. 'Users' should only use it, when they know
what they are doing. Since the turkish locale has special issues (maybe also
the greek), we should IMHO include it here.

> >+        export TESTS_ENVIRONMENT="LC_ALL=$xLCALL
> >VALGRIND_TESTS=$xVALGRIND" +        echo "
> >TESTS_ENVIRONMENT=\"$TESTS_ENVIRONMENT\"" make check +        make check
> >-j5 >/dev/null
>
> If we're pushing this to the repo, let's remove the -j5 option. Not everyone
> uses a 6/8 core machine for development. I tried running this script on my
> dual core and a -j5 caused it to slow down considerably while taking up way
> too much memory.

Well yes, I amended the script to determine the number of cores.

> We could handle reading this as a run time option.
>
> >+      done
> >+    done
> >+
> >+    unset TESTS_ENVIRONMENT
> >+    export TESTS_ENVIRONMENT
> >+    echo "  make distcheck"
> >+    make distcheck -j5
> >+  done
> >+done
> >+
> >+END=$(date +%s.%N)
> >+echo "Duration: "$(echo "$END - $START" | bc)
>
> Also, this script seems to have a whitespace issue on line 28. My git
> complained about it before fixing it and merging with the local tree.
>
> Else, this looks good.

Here is my current version.

Tim

Attachment: 0001-Added-script-contrib-check-hard-to-check-Wget-status.patch
Description: Text Data

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


reply via email to

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