Stable 2.0 and popen.test on Debian Squeeze

Stable 2.0 and popen.test on Debian Squeeze

[Francis Southern]

Hello Guilers,

As you may know, several people have been having problems with
`popen.test' with 2.0 on Debian Squeeze.  After a suggestion from
Ludovic on IRC, I tried running it with my /bin/sh symlinked to Bash
rather than Dash and there were no unexpected failures.  Yay.

Upon further inspection, the problem is that the `open-output-pipe`
test, specifically the `(pass-if "no duplicate" ...' part, fails with
Dash but not Bash.
(I was checking this by running `./check-guile popen.test'.)

Incidentally, while trying to isolate the problem further, I
discovered that loading the file from the REPL gives me:
ERROR: open-output-pipe: no duplicate - arguments: ((wrong-type-arg
"string-append" "Wrong type (expecting ~A): ~S" ("string" #f) (#f)))

And all the other tests pass.  But I don't see how this error could be
related to the shell, and indeed it happens no matter what my system
shell is set to.

Unfortunately, I'm way out of my depth here and have no idea how to
proceed.  But if there's anything you think I can do to help I'm more
than willing to oblige.


I hope this helps!

Re: Stable 2.0 and popen.test on Debian Squeeze

[Andy Wingo]

On Fri 25 Feb 2011 06:01, Francis Southern <francis.southern@gmail.com> writes:

> As you may know, several people have been having problems with
> `popen.test' with 2.0 on Debian Squeeze.  After a suggestion from
> Ludovic on IRC, I tried running it with my /bin/sh symlinked to Bash
> rather than Dash and there were no unexpected failures.  Yay.

Whoa, that's a good one.

The difference seems to be the difference between:

  dash -c 'exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read'

and

  bash -c 'exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read'

Dash prints "closed" and exits immediately with error code 2.  Bash
prints "closed" and waits for input from the "read".

Are we relying on non-portable shell behavior here?

Andy
-- 
http://wingolog.org/

Re: Stable 2.0 and popen.test on Debian Squeeze

[Mark H Weaver]

Andy Wingo <wingo@pobox.com> writes:
> The difference seems to be the difference between:
>
>   dash -c 'exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read'
>
> and
>
>   bash -c 'exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read'
>
> Dash prints "closed" and exits immediately with error code 2.  Bash
> prints "closed" and waits for input from the "read".
>
> Are we relying on non-portable shell behavior here?

In dash, "read" requires at least one argument: the name of the variable
in which to put the string.  You don't see the error message because
stderr has been redirected to /dev/null.  In bash, the REPLY variable is
used by default.  So the "read" above ought to be changed to
"read REPLY".

    Best,
     Mark

Re: Stable 2.0 and popen.test on Debian Squeeze

[dsmich]

---- Mark H Weaver <mhw@netris.org> wrote: 
> Andy Wingo <wingo@pobox.com> writes:
> > The difference seems to be the difference between:
> >
> >   dash -c 'exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read'
> >
> > and
> >
> >   bash -c 'exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read'
> >
> > Dash prints "closed" and exits immediately with error code 2.  Bash
> > prints "closed" and waits for input from the "read".
> >
> > Are we relying on non-portable shell behavior here?
> 
> In dash, "read" requires at least one argument: the name of the variable
> in which to put the string.  You don't see the error message because
> stderr has been redirected to /dev/null.  In bash, the REPLY variable is
> used by default.  So the "read" above ought to be changed to
> "read REPLY".

On my Debian Squeeze system:


dsmith@stumpy:~/src/guile$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 Mar  1 20:26 /bin/sh -> dash


dsmith@stumpy:~/src/guile$ git diff
diff --git a/test-suite/tests/popen.test b/test-suite/tests/popen.test
index 6300c3b..9604c9c 100644
--- a/test-suite/tests/popen.test
+++ b/test-suite/tests/popen.test
@@ -104,7 +104,7 @@
                     (with-input-from-port (car p2c)
                       (lambda ()
                         (open-input-pipe
-                         "exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read")))))))
+                         "exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read REPLY")))))))
       (close-port (cdr c2p))   ;; write side
       (let ((result (eof-object? (read-char port))))
        (display "hello!\n" (cdr p2c))



dsmith@stumpy:~/src/guile$ ./check-guile popen.test
Testing /home/dsmith/src/guile/meta/guile ... popen.test
with GUILE_LOAD_PATH=/home/dsmith/src/guile/test-suite
Running popen.test
FAIL: popen.test: open-output-pipe: no duplicate

Totals for this test run:
passes:                 14
failures:               1
unexpected passes:      0
expected failures:      0
unresolved test cases:  0
untested test cases:    0
unsupported test cases: 0
errors:                 0



-Dale

Re: Stable 2.0 and popen.test on Debian Squeeze

[Mark H Weaver]

<dsmich@roadrunner.com> writes:
> dsmith@stumpy:~/src/guile$ ./check-guile popen.test
> Testing /home/dsmith/src/guile/meta/guile ... popen.test
> with GUILE_LOAD_PATH=/home/dsmith/src/guile/test-suite
> Running popen.test
> FAIL: popen.test: open-output-pipe: no duplicate

The problem here is that "dash -c COMMAND" runs COMMAND as a subprocess
of dash, whereas "bash -c COMMAND" which uses exec such that COMMAND
replaces the bash process.

dash's behavior breaks the "open-output-pipe: no duplicate" test, which
checks that the parent will receive a SIGPIPE if the child closes its
STDIN without terminating.  This depends on the child not keeping a
duplicate of that file descriptor around (as was done in Guile 1.6.4),
hence the name of the test.

Because "dash -c" creates a subprocess instead of exec'ing, the waiting
dash process is holding a duplicate of its STDIN open.  This prevents
the child from closing the end of the pipe, and thus the parent does not
receive a SIGPIPE and the test fails.

     Mark

Re: Stable 2.0 and popen.test on Debian Squeeze

[Mark H Weaver]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

<dsmich@roadrunner.com> writes:
> dsmith@stumpy:~/src/guile$ ./check-guile popen.test
> Testing /home/dsmith/src/guile/meta/guile ... popen.test
> with GUILE_LOAD_PATH=/home/dsmith/src/guile/test-suite
> Running popen.test
> FAIL: popen.test: open-output-pipe: no duplicate

Does the attached patch fix your problems with popen.test on Squeeze?

   Thanks,
     Mark



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Portability fixes for popen.test (for when /bin/sh is not bash) --]
[-- Type: text/x-diff, Size: 1809 bytes --]

From 11195909b25347ae710d6423165eb123cf0aa996 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Wed, 2 Mar 2011 06:02:58 -0500
Subject: [PATCH] Portability fixes for popen.test (for when /bin/sh is not bash)

* test-suite/tests/popen.test (open-input-pipe no-duplicate): Pass
  "read REPLY" command instead of "read" to the subshell, for improved
  portability.  In particular, it is needed when /bin/sh is dash.

  (open-output-pipe no-duplicate): Pass "exec guile [...]" instead of
  "guile [...]" to the subshell, to ensure that the subshell will not
  run guile as a subprocess while holding a duplicate of STDIN, which
  would cause this test to fail.  This is needed when /bin/sh is dash.
---
 test-suite/tests/popen.test |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test-suite/tests/popen.test b/test-suite/tests/popen.test
index 6300c3b..bfd7da7 100644
--- a/test-suite/tests/popen.test
+++ b/test-suite/tests/popen.test
@@ -104,7 +104,7 @@
 		     (with-input-from-port (car p2c)
 		       (lambda ()
 			 (open-input-pipe
-			  "exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read")))))))
+			  "exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read REPLY")))))))
       (close-port (cdr c2p))   ;; write side
       (let ((result (eof-object? (read-char port))))
 	(display "hello!\n" (cdr p2c))
@@ -168,7 +168,7 @@
 	   (port (with-error-to-port (cdr c2p)
 		   (lambda ()
 		     (open-output-pipe
-                      (string-append "guile --no-auto-compile -s \""
+                      (string-append "exec guile --no-auto-compile -s \""
                                      (getenv "TEST_SUITE_DIR")
                                      "/tests/popen-child.scm\""))))))
       (close-port (cdr c2p))   ;; write side
-- 
1.7.1

Re: Stable 2.0 and popen.test on Debian Squeeze

[dsmich]

---- Mark H Weaver <mhw@netris.org> wrote: 
> 
> Does the attached patch fix your problems with popen.test on Squeeze?

Indeed it does.   Very much thanks!


dsmith@stumpy:~/src/guile$ ./check-guile popen.test
Testing /home/dsmith/src/guile/meta/guile ... popen.test
with GUILE_LOAD_PATH=/home/dsmith/src/guile/test-suite
Running popen.test

Totals for this test run:
passes:                 15
failures:               0
unexpected passes:      0
expected failures:      0
unresolved test cases:  0
untested test cases:    0
unsupported test cases: 0
errors:                 0


-Dale

Re: Stable 2.0 and popen.test on Debian Squeeze

[dsmich]

---- Mark H Weaver <mhw@netris.org> wrote: 
> 
> Does the attached patch fix your problems with popen.test on Squeeze?

Indeed it does.   Very much thanks!


dsmith@stumpy:~/src/guile$ ./check-guile popen.test
Testing /home/dsmith/src/guile/meta/guile ... popen.test
with GUILE_LOAD_PATH=/home/dsmith/src/guile/test-suite
Running popen.test

Totals for this test run:
passes:                 15
failures:               0
unexpected passes:      0
expected failures:      0
unresolved test cases:  0
untested test cases:    0
unsupported test cases: 0
errors:                 0


-Dale