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

[Eli Zaretskii]

> Cc: Eli Zaretskii 
> From: "pwa...@gmail.net.pl" 
> Date: Sun, 2 Oct 2016 21:54:58 +0200
> 
> Is there a instruction on how to compile current wget version on windows?

Nothing special, just "./configure && make", as you'd do on a Posix
system.

The trick is to have a development environment that supports the
above.  I use MSYS and MinGW from mingw.org:

  https://sourceforge.net/projects/mingw/files/

If you don't have the dependency libraries, you need to install them
first.  They are also built as above, but you can find precompiled
32-bit binaries and the corresponding header files here:

  https://sourceforge.net/projects/ezwinports/files

If you want to build a 64-bit version of Wget, I suggest to get the
dependencies from the MSYS2 project, starting with this page's
instructions:

  https://msys2.github.io/

The MSYS2/MinGW64 development environment can be installed from that
page as well (if you don't have it).

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

[pwa...@gmail.net.pl]

Is there a instruction on how to compile current wget version on windows?

Piotr

W dniu 30.09.2016 o 10:53, Eli Zaretskii pisze:

From: Piotr Wajda 
Date: Fri, 30 Sep 2016 09:51:37 +0200

Hi, Reworked recent patch to behave correctly on fg and bg. Now user can switch 
from fg to bg and vice versa and wget will select fd accordingly.

Thanks.


+  /* Initialize this values so we don't have to ask every time we print line */
+  shell_is_interactive = isatty (STDIN_FILENO);

The MS-Windows version of isatty returns non-zero when its argument
file descriptor is open on any character device.  Notably, this
includes the null device, which is definitely not what we want in this
case, I think.

So I think using this logic will need to import isatty from Gnulib, or
provide an alternative implementation in mswindows.c.


  static void
  check_redirect_output (void)
  {
-  if (redirect_request == RR_REQUESTED)
+  /* If it was redirected already to log file by SIGHUP or SIGUSR1, it was 
permanent */
+  if(!redirect_request_signal_name && shell_is_interactive)
  {
-  redirect_request = RR_DONE;
-  redirect_output ();
+  if(tcgetpgrp(STDIN_FILENO) != getpgrp())

Neither tcgetpgrp nor getpgrp exist on MS-Windows.

AFAIU, this test is intended to check whether wget was backgrounded.
Since AFAIK that's not possible on MS-Windows, this test should always
return zero on Windows, so I suggest a separate predicate function
with 2 implementations: one on Windows, the other on Posix platforms.

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

[Eli Zaretskii]

> From: Piotr Wajda 
> Date: Fri, 30 Sep 2016 09:51:37 +0200
> 
> Hi, Reworked recent patch to behave correctly on fg and bg. Now user can 
> switch from fg to bg and vice versa and wget will select fd accordingly.

Thanks.

> +  /* Initialize this values so we don't have to ask every time we print line 
> */
> +  shell_is_interactive = isatty (STDIN_FILENO);

The MS-Windows version of isatty returns non-zero when its argument
file descriptor is open on any character device.  Notably, this
includes the null device, which is definitely not what we want in this
case, I think.

So I think using this logic will need to import isatty from Gnulib, or
provide an alternative implementation in mswindows.c.

>  static void
>  check_redirect_output (void)
>  {
> -  if (redirect_request == RR_REQUESTED)
> +  /* If it was redirected already to log file by SIGHUP or SIGUSR1, it was 
> permanent */
> +  if(!redirect_request_signal_name && shell_is_interactive)
>  {
> -  redirect_request = RR_DONE;
> -  redirect_output ();
> +  if(tcgetpgrp(STDIN_FILENO) != getpgrp()) 

Neither tcgetpgrp nor getpgrp exist on MS-Windows.

AFAIU, this test is intended to check whether wget was backgrounded.
Since AFAIK that's not possible on MS-Windows, this test should always
return zero on Windows, so I suggest a separate predicate function
with 2 implementations: one on Windows, the other on Posix platforms.

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

[Piotr Wajda]

Hi, Reworked recent patch to behave correctly on fg and bg. Now user can switch 
from fg to bg and vice versa and wget will select fd accordingly.

Please review.

Thanks
Piotr

---
 src/log.c  | 116 +++--
 src/log.h  |   1 +
 src/main.c |   2 +-
 3 files changed, 76 insertions(+), 43 deletions(-)

diff --git a/src/log.c b/src/log.c
index a1338ca..f93b02c 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");
+  filelogfp = fopen (file, appendp ? "a" : "w");
   if (!logfp)
 {
   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,9 @@ log_init (const char *file, bool appendp)
   save_context_p = true;
 }
 }
+
+  /* Initialize this values so we don't have to ask every time we print line */
+  shell_is_interactive = isatty (STDIN_FILENO);
 }
 
 /* Close LOGFP (only if we opened it, not if it's stderr), inhibit
@@ -880,59 +898,73 @@ 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;
+  /* Dump the context output to the newly opened log.  */
+  log_dump_context ();
+}
+  else
+{
+  /* Eek!  Opening the alternate log file has failed.  Nothing we
+can do but disable printing completely. */
+  fprintf (stderr, _("%s: %s; disabling logging.\n"),
+  (logfile) ? logfile : DEFAULT_LOGFILE, strerror (errno));
+  inhibit_logging = true;
+}
+}
+  else 
+{
+  fprintf (stderr, _("\nRedirecting output to %s.\n"),
+  quote (logfile));
+  logfp = filelogfp;
+  log_dump_context ();
+}
 }
-  else
+  else if (!to_file && logfp != stdlogfp)
 {
-  /* Eek!  Opening the alternate log file has failed.  Nothing we
- can do but disable printing completely. */
-  fprintf (stderr, _("\n%s received.\n"), redirect_request_signal_name);
-  fprintf (stderr, _("%s: %s; disabling logging.\n"),
-   (logfile) ? logfile : DEFAULT_LOGFILE, strerror (errno));
-  inhibit_logging = true;
+  logfp = stdlogfp;
+  log_dump_context ();
 }
-  save_context_p = false;
 }
 
-/* Check whether a signal handler requested the output to be
-   redirected. */
+/* Check whether there's a