Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-19 Thread Eli Zaretskii 
> From: "Wajda, Piotr" 
> Date: Wed, 19 Oct 2016 11:57:06 +0200
> 
> My only confusion was that during testing on windows, when sending 
> CTRL+C or CTRL+Break it immediately terminates, which is basically what 
> I think it should do for CTRL+C. Not sure about CTRL+Break.

What else is reasonable for CTRL+Break?  We can arrange for them to
produce different effects, if there are two alternative behaviors that
would make sense.

Thanks.

Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-19 Thread Wajda, Piotr 



Sorry, I had the impression there was some discussion going on !?


My only confusion was that during testing on windows, when sending 
CTRL+C or CTRL+Break it immediately terminates, which is basically what 
I think it should do for CTRL+C. Not sure about CTRL+Break.
But the same behaviour was with current wget version (without my 
patches), so I think it's correct.



But apart from that:
- please add a space after if statements.
- please add a space between function name and (.
- please make local commits with appropriate commit messages (see git log for
examples), generate your patches with 'git format-patch -3' (-3 for your last
3 commits) and send them as attachments.


Corrected and sent.

Thanks
Piotr

Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-18 Thread Tim Rühsen 
On Dienstag, 18. Oktober 2016 08:30:34 CEST Wajda, Piotr wrote:
> Can you please review my 3 patches?

Sorry, I had the impression there was some discussion going on !?

But apart from that:
- please add a space after if statements.
- please add a space between function name and (.
- please make local commits with appropriate commit messages (see git log for 
examples), generate your patches with 'git format-patch -3' (-3 for your last 
3 commits) and send them as attachments.

Regards, Tim


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

Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-18 Thread Wajda, Piotr 

Can you please review my 3 patches?

Piotr

On 07/10/16 10:10, Wajda, Piotr wrote:

From: losgrandes 
Date: Thu,  6 Oct 2016 09:47:01 +0200

Fortunately I tested wget.exe in normal mode and background mode
(-b). Was ok.
Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it
really works on windows?).


Yes, CTRL-C/CTRL-BREAK should work on Windows.  What didn't work in
your case?



I've tested ctrl+c/ctrl+break with current wget and my patched version.
In all cases process is terminated.

I think patch is ready to be reviewed.

Piotr

Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-07 Thread Tim Ruehsen 
On Friday, October 7, 2016 10:08:08 AM CEST Eli Zaretskii wrote:
> The Windows runtime doesn't have 'pipe', it has '_pipe' (with a
> slightly different argument list).  I believe we need the Gnulib pipe
> module to get this to compile.  However, just as a quick hack, replace
> the call to 'pipe' with a corresponding call to '_pipe', you can find
> its documentation here:
> 
>   https://msdn.microsoft.com/en-us/library/edze9h7e.aspx
> 
> (This problem is unrelated to your changes, the call to 'pipe' is
> already in the repository.)

I just pushed a commit to add pipe-posix to bootstrap.conf.
That makes pipe() available on Windows.

> >   url.o:url.c:(.text+0x1e78): undefined reference to `libiconv_open'
> >   url.o:url.c:(.text+0x1f25): undefined reference to `libiconv'
> >   url.o:url.c:(.text+0x1f57): undefined reference to `libiconv'
> >   url.o:url.c:(.text+0x1f7e): undefined reference to `libiconv_close'
> >   url.o:url.c:(.text+0x20ee): undefined reference to `libiconv_close'
> >   collect2: error: ld returned 1 exit status
> > 
> > This was generated by:
> > ./configure --host=i686-w64-mingw32 --without-ssl --without-libidn
> > --without-metalink --with-gpgme-prefix=/dev/null
> > CFLAGS=-I$BUILDDIR/tmp/include LDFLAGS=-L$BUILDDIR/tmp/lib
> > --with-libiconv-prefix=$BUILDDIR/tmp CFLAGS=-liconv
> I think you need to add -liconv to LIBS, not to CFLAGS.  GNU ld is a
> one-pass linker, so it needs to see -liconv _after_ all the object
> files, not before.

Cross-compilation (Linux -> Windows) and testing needs a recipe that works for 
other developers as well.

I just pushed 3 changes that makes it easier
- make pipe() available
- properly include iconv.h
- not using /dev/null for WGETRC in tests/Makefile.am

Here is my current recipe, just hacked together on Debian.
Maybe we can work on it together, there are some obvious flaws.


# for iconv support:
#   apt-get install win-iconv-mingw-w64-dev
#
# for zlib support:
#   apt-get install libz-mingw-w64 libz-mingw-w64-dev

PREFIX=i686-w64-mingw32
export CC=$PREFIX-gcc
export CPP=$PREFIX-cpp
export RANLIB=$PREFIX-ranlib
export PKG_CONFIG=$PREFIX-pkg-config
./bootstrap
./configure --host=i686-w64-mingw32 --without-ssl --without-libidn --without-
metalink
make

# copy DLLs to your library folder or where your .exe is
cp -p /usr/$PREFIX/bin/iconv.dll src
cp -p /usr/$PREFIX/lib/zlib1.dll src

# run wget.exe
wine src/wget.exe --help

# for testing we need a link wget.exe->wget
ln -s src/wget.exe src/wget
# and we need the DLLs for unit-tests.exe
cp -p /usr/$PREFIX/bin/iconv.dll tests
cp -p /usr/$PREFIX/lib/zlib1.dll tests

make check
# This only works if you can run .exe files on your system (kernel feature)
# The test suite should be amended to make use of $(EXEEXT)


- Copying those DLLs is ugly. Any idea ?
- In WgetTests.pm the executable is hard-coded to '../src/wget'. That should 
check $(EXEEXT) and use e.g. 'wine ../src/wget.$(EXEEXT)'. Any idea that works 
with wine 32-bit and 64-bit (depending on $PREFIX) ?

Regards, Tim


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

Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-07 Thread Wajda, Piotr 

From: losgrandes 
Date: Thu,  6 Oct 2016 09:47:01 +0200

Fortunately I tested wget.exe in normal mode and background mode (-b). Was ok.
Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it really 
works on windows?).


Yes, CTRL-C/CTRL-BREAK should work on Windows.  What didn't work in
your case?



I've tested ctrl+c/ctrl+break with current wget and my patched version. 
In all cases process is terminated.


I think patch is ready to be reviewed.

Piotr

Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-07 Thread Wajda, Piotr 

Eli, you're awesome! :)
I tried many combinations (I think so), including LIBS=-liconv, but 
without success. I've tried now and it works!


Now I'll test on my own.

Thanks
Piotr

On 07/10/16 09:08, Eli Zaretskii wrote:

From: losgrandes 
Date: Thu,  6 Oct 2016 09:47:01 +0200

Fortunately I tested wget.exe in normal mode and background mode (-b). Was ok.
Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it really 
works on windows?).


Yes, CTRL-C/CTRL-BREAK should work on Windows.  What didn't work in
your case?


1. Could you be so kind and test my wget.exe with CTRL+Break?


Send your test instructions, and I will try to build and test it here.


2. Advise me with error I get while compiling:
  main.o:main.c:(.text+0x579): undefined reference to `pipe'


The Windows runtime doesn't have 'pipe', it has '_pipe' (with a
slightly different argument list).  I believe we need the Gnulib pipe
module to get this to compile.  However, just as a quick hack, replace
the call to 'pipe' with a corresponding call to '_pipe', you can find
its documentation here:

  https://msdn.microsoft.com/en-us/library/edze9h7e.aspx

(This problem is unrelated to your changes, the call to 'pipe' is
already in the repository.)


  url.o:url.c:(.text+0x1e78): undefined reference to `libiconv_open'
  url.o:url.c:(.text+0x1f25): undefined reference to `libiconv'
  url.o:url.c:(.text+0x1f57): undefined reference to `libiconv'
  url.o:url.c:(.text+0x1f7e): undefined reference to `libiconv_close'
  url.o:url.c:(.text+0x20ee): undefined reference to `libiconv_close'
  collect2: error: ld returned 1 exit status

This was generated by:
./configure --host=i686-w64-mingw32 --without-ssl --without-libidn 
--without-metalink --with-gpgme-prefix=/dev/null CFLAGS=-I$BUILDDIR/tmp/include 
LDFLAGS=-L$BUILDDIR/tmp/lib --with-libiconv-prefix=$BUILDDIR/tmp CFLAGS=-liconv


I think you need to add -liconv to LIBS, not to CFLAGS.  GNU ld is a
one-pass linker, so it needs to see -liconv _after_ all the object
files, not before.

Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-07 Thread Eli Zaretskii 
> From: losgrandes 
> Date: Thu,  6 Oct 2016 09:47:01 +0200
> 
> Fortunately I tested wget.exe in normal mode and background mode (-b). Was ok.
> Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it really 
> works on windows?).

Yes, CTRL-C/CTRL-BREAK should work on Windows.  What didn't work in
your case?

> 1. Could you be so kind and test my wget.exe with CTRL+Break?

Send your test instructions, and I will try to build and test it here.

> 2. Advise me with error I get while compiling:
>   main.o:main.c:(.text+0x579): undefined reference to `pipe'

The Windows runtime doesn't have 'pipe', it has '_pipe' (with a
slightly different argument list).  I believe we need the Gnulib pipe
module to get this to compile.  However, just as a quick hack, replace
the call to 'pipe' with a corresponding call to '_pipe', you can find
its documentation here:

  https://msdn.microsoft.com/en-us/library/edze9h7e.aspx

(This problem is unrelated to your changes, the call to 'pipe' is
already in the repository.)

>   url.o:url.c:(.text+0x1e78): undefined reference to `libiconv_open'
>   url.o:url.c:(.text+0x1f25): undefined reference to `libiconv'
>   url.o:url.c:(.text+0x1f57): undefined reference to `libiconv'
>   url.o:url.c:(.text+0x1f7e): undefined reference to `libiconv_close'
>   url.o:url.c:(.text+0x20ee): undefined reference to `libiconv_close'
>   collect2: error: ld returned 1 exit status
> 
> This was generated by:
> ./configure --host=i686-w64-mingw32 --without-ssl --without-libidn 
> --without-metalink --with-gpgme-prefix=/dev/null 
> CFLAGS=-I$BUILDDIR/tmp/include LDFLAGS=-L$BUILDDIR/tmp/lib 
> --with-libiconv-prefix=$BUILDDIR/tmp CFLAGS=-liconv

I think you need to add -liconv to LIBS, not to CFLAGS.  GNU ld is a
one-pass linker, so it needs to see -liconv _after_ all the object
files, not before.

Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-06 Thread Wajda, Piotr 
Sorry for incorrect Date email header in that email, I've send it from 
previously suspended virtual machine that I forgot to ntpupdate.


Piotr

On 06/10/16 09:47, losgrandes wrote:

I've updated patch to not run tcgetpgrp, getpgrp and isatty (for 
shell_is_interactive test) on windows.
I have compiled and tested on linux and (mostly*) on windows. Should be ok.

* - I was able to create working mingw env on linux, but cross-compilation 
worked for me only few times (I know, silly, I broke it somehow).
Fortunately I tested wget.exe in normal mode and background mode (-b). Was ok.
Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it really 
works on windows?).
I'm struggling with mingw, but no luck so far.

So here're my asks for you:
1. Could you be so kind and test my wget.exe with CTRL+Break?
and/or
2. Advise me with error I get while compiling:
  main.o:main.c:(.text+0x579): undefined reference to `pipe'
  url.o:url.c:(.text+0x1e78): undefined reference to `libiconv_open'
  url.o:url.c:(.text+0x1f25): undefined reference to `libiconv'
  url.o:url.c:(.text+0x1f57): undefined reference to `libiconv'
  url.o:url.c:(.text+0x1f7e): undefined reference to `libiconv_close'
  url.o:url.c:(.text+0x20ee): undefined reference to `libiconv_close'
  collect2: error: ld returned 1 exit status

This was generated by:
./configure --host=i686-w64-mingw32 --without-ssl --without-libidn 
--without-metalink --with-gpgme-prefix=/dev/null CFLAGS=-I$BUILDDIR/tmp/include 
LDFLAGS=-L$BUILDDIR/tmp/lib --with-libiconv-prefix=$BUILDDIR/tmp CFLAGS=-liconv
make

Thanks
Piotr

---
 src/log.c   | 125 +---
 src/log.h   |   1 +
 src/main.c  |   2 +-
 src/mswindows.c |   5 +--
 4 files changed, 85 insertions(+), 48 deletions(-)

diff --git a/src/log.c b/src/log.c
index a1338ca..9a84c85 100644
--- a/src/log.c
+++ b/src/log.c
@@ -80,6 +80,18 @@ as that of the covered work.  */
logging is inhibited, logfp is set back to NULL. */
 static FILE *logfp;

+/* Descriptor of the stdout|stderr */
+static FILE *stdlogfp;
+
+/* Descriptor of the wget.log* file (if created) */
+static FILE *filelogfp;
+
+/* Name of log file */
+static char *logfile;
+
+/* Is interactive shell ? */
+static int shell_is_interactive;
+
 /* A second file descriptor pointing to the temporary log file for the
WARC writer.  If WARC writing is disabled, this is NULL.  */
 static FILE *warclogfp;
@@ -611,16 +623,18 @@ log_init (const char *file, bool appendp)
 {
   if (HYPHENP (file))
 {
-  logfp = stdout;
+  stdlogfp = stdout;
+  logfp = stdlogfp;
 }
   else
 {
-  logfp = fopen (file, appendp ? "a" : "w");
-  if (!logfp)
+  filelogfp = fopen (file, appendp ? "a" : "w");
+  if (!filelogfp)
 {
   fprintf (stderr, "%s: %s: %s\n", exec_name, file, strerror 
(errno));
   exit (WGET_EXIT_GENERIC_ERROR);
 }
+  logfp = filelogfp;
 }
 }
   else
@@ -631,7 +645,8 @@ log_init (const char *file, bool appendp)
  stderr only if the user actually specifies `-O -'.  He says
  this inconsistency is harder to document, but is overall
  easier on the user.  */
-  logfp = stderr;
+  stdlogfp = stderr;
+  logfp = stdlogfp;

   if (1
 #ifdef HAVE_ISATTY
@@ -646,6 +661,11 @@ log_init (const char *file, bool appendp)
   save_context_p = true;
 }
 }
+
+#ifndef WINDOWS
+  /* Initialize this values so we don't have to ask every time we print line */
+  shell_is_interactive = isatty (STDIN_FILENO);
+#endif
 }

 /* Close LOGFP (only if we opened it, not if it's stderr), inhibit
@@ -880,59 +900,78 @@ log_cleanup (void)

 /* When SIGHUP or SIGUSR1 are received, the output is redirected
elsewhere.  Such redirection is only allowed once. */
-static enum { RR_NONE, RR_REQUESTED, RR_DONE } redirect_request = RR_NONE;
 static const char *redirect_request_signal_name;

-/* Redirect output to `wget-log'.  */
+/* Redirect output to `wget-log' or back to stdout/stderr.  */

-static void
-redirect_output (void)
+void
+redirect_output (bool to_file, const char *signal_name)
 {
-  char *logfile;
-  logfp = unique_create (DEFAULT_LOGFILE, false, );
-  if (logfp)
+  if(to_file && logfp != filelogfp)
 {
-  fprintf (stderr, _("\n%s received, redirecting output to %s.\n"),
-   redirect_request_signal_name, quote (logfile));
-  xfree (logfile);
-  /* Dump the context output to the newly opened log.  */
-  log_dump_context ();
+  if (signal_name)
+{
+  fprintf(stderr, "\n%s received.", signal_name);
+}
+  if (!filelogfp)
+{
+  filelogfp = unique_create (DEFAULT_LOGFILE, false, );
+  if (filelogfp)
+{
+  fprintf (stderr, _("\nRedirecting output to %s.\n"),
+  quote (logfile));
+

[Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background

2016-10-06 Thread losgrandes 
I've updated patch to not run tcgetpgrp, getpgrp and isatty (for 
shell_is_interactive test) on windows.
I have compiled and tested on linux and (mostly*) on windows. Should be ok.

* - I was able to create working mingw env on linux, but cross-compilation 
worked for me only few times (I know, silly, I broke it somehow).
Fortunately I tested wget.exe in normal mode and background mode (-b). Was ok.
Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it really 
works on windows?).
I'm struggling with mingw, but no luck so far.

So here're my asks for you:
1. Could you be so kind and test my wget.exe with CTRL+Break?
and/or
2. Advise me with error I get while compiling:
  main.o:main.c:(.text+0x579): undefined reference to `pipe'
  url.o:url.c:(.text+0x1e78): undefined reference to `libiconv_open'
  url.o:url.c:(.text+0x1f25): undefined reference to `libiconv'
  url.o:url.c:(.text+0x1f57): undefined reference to `libiconv'
  url.o:url.c:(.text+0x1f7e): undefined reference to `libiconv_close'
  url.o:url.c:(.text+0x20ee): undefined reference to `libiconv_close'
  collect2: error: ld returned 1 exit status

This was generated by:
./configure --host=i686-w64-mingw32 --without-ssl --without-libidn 
--without-metalink --with-gpgme-prefix=/dev/null CFLAGS=-I$BUILDDIR/tmp/include 
LDFLAGS=-L$BUILDDIR/tmp/lib --with-libiconv-prefix=$BUILDDIR/tmp CFLAGS=-liconv
make

Thanks
Piotr

---
 src/log.c   | 125 +---
 src/log.h   |   1 +
 src/main.c  |   2 +-
 src/mswindows.c |   5 +--
 4 files changed, 85 insertions(+), 48 deletions(-)

diff --git a/src/log.c b/src/log.c
index a1338ca..9a84c85 100644
--- a/src/log.c
+++ b/src/log.c
@@ -80,6 +80,18 @@ as that of the covered work.  */
logging is inhibited, logfp is set back to NULL. */
 static FILE *logfp;
 
+/* Descriptor of the stdout|stderr */
+static FILE *stdlogfp;
+
+/* Descriptor of the wget.log* file (if created) */
+static FILE *filelogfp;
+
+/* Name of log file */
+static char *logfile;
+
+/* Is interactive shell ? */
+static int shell_is_interactive;
+
 /* A second file descriptor pointing to the temporary log file for the
WARC writer.  If WARC writing is disabled, this is NULL.  */
 static FILE *warclogfp;
@@ -611,16 +623,18 @@ log_init (const char *file, bool appendp)
 {
   if (HYPHENP (file))
 {
-  logfp = stdout;
+  stdlogfp = stdout;
+  logfp = stdlogfp;
 }
   else
 {
-  logfp = fopen (file, appendp ? "a" : "w");
-  if (!logfp)
+  filelogfp = fopen (file, appendp ? "a" : "w");
+  if (!filelogfp)
 {
   fprintf (stderr, "%s: %s: %s\n", exec_name, file, strerror 
(errno));
   exit (WGET_EXIT_GENERIC_ERROR);
 }
+  logfp = filelogfp;
 }
 }
   else
@@ -631,7 +645,8 @@ log_init (const char *file, bool appendp)
  stderr only if the user actually specifies `-O -'.  He says
  this inconsistency is harder to document, but is overall
  easier on the user.  */
-  logfp = stderr;
+  stdlogfp = stderr;
+  logfp = stdlogfp;
 
   if (1
 #ifdef HAVE_ISATTY
@@ -646,6 +661,11 @@ log_init (const char *file, bool appendp)
   save_context_p = true;
 }
 }
+
+#ifndef WINDOWS
+  /* Initialize this values so we don't have to ask every time we print line */
+  shell_is_interactive = isatty (STDIN_FILENO);
+#endif
 }
 
 /* Close LOGFP (only if we opened it, not if it's stderr), inhibit
@@ -880,59 +900,78 @@ log_cleanup (void)
 
 /* When SIGHUP or SIGUSR1 are received, the output is redirected
elsewhere.  Such redirection is only allowed once. */
-static enum { RR_NONE, RR_REQUESTED, RR_DONE } redirect_request = RR_NONE;
 static const char *redirect_request_signal_name;
 
-/* Redirect output to `wget-log'.  */
+/* Redirect output to `wget-log' or back to stdout/stderr.  */
 
-static void
-redirect_output (void)
+void
+redirect_output (bool to_file, const char *signal_name)
 {
-  char *logfile;
-  logfp = unique_create (DEFAULT_LOGFILE, false, );
-  if (logfp)
+  if(to_file && logfp != filelogfp)
 {
-  fprintf (stderr, _("\n%s received, redirecting output to %s.\n"),
-   redirect_request_signal_name, quote (logfile));
-  xfree (logfile);
-  /* Dump the context output to the newly opened log.  */
-  log_dump_context ();
+  if (signal_name)
+{
+  fprintf(stderr, "\n%s received.", signal_name);
+}
+  if (!filelogfp) 
+{
+  filelogfp = unique_create (DEFAULT_LOGFILE, false, );
+  if (filelogfp)
+{
+  fprintf (stderr, _("\nRedirecting output to %s.\n"),
+  quote (logfile));
+  /* Store signal name to tell wget it's permanent redirect to log 
file */
+  redirect_request_signal_name = signal_name;
+  logfp = filelogfp;
+