Bug#642922: regression: "sh -c" change causes FTBFS
> > POSIX's Shell and Utilities (XCU) 2.12 [1] does say that "[the] > > environment of the shell process shall not be changed by the utility", > > and that environment includes open files. My understanding is that > > dash's new behaviour (and incidentally, ksh93's one) is incorrect. > As I understand it, the intent of that section was not to prohibit > changes in process state due to a command like "exec foo" replacing > the shell process. The shell execution environment is not changed but > simply no longer exists once execve() has been called. The word "process" seems a bit awkward here, yes. For the most part, POSIX is very careful in writing "shell environment" instead of "shell process", so that creating a new process can often be avoided by clever implementations. Anyway, if the shell has found that it can safely exec a simple command without forking (or run a subshell without forking), there is no possibility of any execution in the original shell environment, so there seems no need to preserve any open files or other resource needed for such execution. Note that 2.7 Redirection does not say anything explicit about preserving the previous content of file descriptors, only that a redirection on a command (except 'exec') shall be in effect during that command only. Therefore, the shell only needs to save file descriptors at high numbers if it may need to execute subsequent commands in the same process ("may" because a trap for EXIT might be set in the command, if it is not external). Also note that 2.9.1 Simple Commands does not explicitly call for creating a new process. If there are subsequent commands, the shell will obviously need to create a new process, but otherwise that is not needed. > More to the point, POSIX is very easy to change[1], so if this > behavior is causing a problem in otherwise reasonable application > code, methinks it would be better to discuss that instead... The behaviour you are complaining about exists in almost all shells (tried: FreeBSD /bin/sh, dash, bash, zsh, mksh, ksh93, Heirloom Bourne shell, kBuild kmk_ash (much like NetBSD /bin/sh)) in some contexts, so it is unlikely to change. To demonstrate, where ${SH} is the shell to be tested: ${SH} -c '/bin/sleep 10 >/dev/null &' | { cat; echo EOF; } The "EOF" line appears immediately in all tested shells, showing that there is nothing holding onto the pipe in the subshell environment created by '&'. On the other hand, if the redirection is 2>/dev/null instead, it takes ten seconds before "EOF" appears, as in this case the pipe file descriptor is passed on. If braces or parentheses are added around the sleep command (including or not including the redirection), behaviour varies per shell. Likewise, behaviour for { ${SH} -c '/bin/sleep 10 >/dev/null' & } | { cat; echo EOF; } varies per shell, and I do not see a reason why this should keep the pipe file descriptor open while shells clearly agree that it should not be kept open in the first example. (Note that I write /bin/sleep explicitly, because some shells have a sleep builtin.) -- Jilles Tjoelker -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
(-cc: debian-devel) Stéphane Glondu wrote: > [CC-ing debian-devel to get more opinions] Sorry for the tone of my last response. But really, more opinions are not needed, though the increased review of the ramifications of the change is welcome. Facts should suffice. ;-) > POSIX's Shell and Utilities (XCU) 2.12 [1] does say that "[the] > environment of the shell process shall not be changed by the utility", > and that environment includes open files. My understanding is that > dash's new behaviour (and incidentally, ksh93's one) is incorrect. As I understand it, the intent of that section was not to prohibit changes in process state due to a command like "exec foo" replacing the shell process. The shell execution environment is not changed but simply no longer exists once execve() has been called. More to the point, POSIX is very easy to change[1], so if this behavior is causing a problem in otherwise reasonable application code, methinks it would be better to discuss that instead... From a release management point of view, I'm of course happy with delaying reinstatement of the change in sid until the fallout is better understood. Thanks, Jonathan [1] http://austingroupbugs.net/ -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
[CC-ing debian-devel to get more opinions] On 10/06/2011 04:07 PM, Jonathan Nieder wrote: >> ocamlbuild's logic is definitively incorrect, but I'm not sure if dash's >> new behaviour is correct. "bash -c" doesn't skip fork() when a >> redirection is set up, I guess for a reason. "dash -c" should probably >> do the same for the same reason. > > Hold on a second. Dash is not supposed to be a bash emulator. :) > > ksh93 -c "/bin/sleep 100 >dev/null" does skip a fork(). I suspect > bash does not skip a fork in this case for the same reason that > > bash -c 'echo hi; /bin/sleep 100' > > does not skip a fork. POSIX's Shell and Utilities (XCU) 2.12 [1] does say that "[the] environment of the shell process shall not be changed by the utility", and that environment includes open files. My understanding is that dash's new behaviour (and incidentally, ksh93's one) is incorrect. [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_12 Cheers, -- Stéphane -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
Stéphane Glondu wrote: > ocamlbuild's logic is definitively incorrect, but I'm not sure if dash's > new behaviour is correct. "bash -c" doesn't skip fork() when a > redirection is set up, I guess for a reason. "dash -c" should probably > do the same for the same reason. Hold on a second. Dash is not supposed to be a bash emulator. :) ksh93 -c "/bin/sleep 100 >dev/null" does skip a fork(). I suspect bash does not skip a fork in this case for the same reason that bash -c 'echo hi; /bin/sleep 100' does not skip a fork. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
On 10/05/2011 11:51 PM, Stéphane Glondu wrote: > It seems that darcs failure was unrelated. I digged a bit more, and > found some suspicious code in ocamlbuild: > > http://caml.inria.fr/mantis/view.php?id=5371 > > I still don't understand why this corner case is triggered with this > patched dash, and I'd rather wait for more feedback before unreverting > the patch in dash. I got it: the command started with "sh -c" (ocamldep) redirects (with shell's ">") its output to a file, therefore closing first its current standard output, which is the writing end of a pipe. When fork() is skipped, the parent process (ocamlbuild) gets an EOF on the reading end of the pipe, which is then misinterpreted. When fork() is used, the writing end of the pipe is closed only when the shell exits, i.e. when ocamldep finishes. ocamlbuild's logic is definitively incorrect, but I'm not sure if dash's new behaviour is correct. "bash -c" doesn't skip fork() when a redirection is set up, I guess for a reason. "dash -c" should probably do the same for the same reason. Cheers, -- Stéphane -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
Le 26/09/2011 22:44, Jonathan Nieder a écrit : >> I beg to differ. ocamlbuild uses libc's system(), it would be insane to >> change that. It seems that darcs is also affected. There might be also >> more packages affected... > > Got it --- I'll prepare an upload reverting the "sh -c" patch. > > More understanding of what is actually happening would be welcome, of > course. It seems that darcs failure was unrelated. I digged a bit more, and found some suspicious code in ocamlbuild: http://caml.inria.fr/mantis/view.php?id=5371 I still don't understand why this corner case is triggered with this patched dash, and I'd rather wait for more feedback before unreverting the patch in dash. Cheers, -- Stéphane -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
severity 642922 serious thanks On 0, Stéphane Glondu wrote: > Le 25/09/2011 20:39, Jonathan Nieder a écrit : > > Thanks for letting me know. This is important (and a regression), but > > it should be possible to work around in ocamlbuild as easily as using > > "bash", so I don't see why that would make it release-critical. > > I beg to differ. ocamlbuild uses libc's system(), it would be insane to > change that. ... and for that reason, this bug should be release critical. It is a severe regression, and led many packages to fail to build (in unexpected ways). Regards, with my Release Team hat on -- Mehdi Dogguy -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
tags 642922 + upstream severity 642922 serious quit Stéphane Glondu wrote: > Le 25/09/2011 20:39, Jonathan Nieder a écrit : >> Thanks for letting me know. This is important (and a regression), but >> it should be possible to work around in ocamlbuild as easily as using >> "bash", so I don't see why that would make it release-critical. > > I beg to differ. ocamlbuild uses libc's system(), it would be insane to > change that. It seems that darcs is also affected. There might be also > more packages affected... Got it --- I'll prepare an upload reverting the "sh -c" patch. More understanding of what is actually happening would be welcome, of course. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
Le 25/09/2011 20:39, Jonathan Nieder a écrit : > Thanks for letting me know. This is important (and a regression), but > it should be possible to work around in ocamlbuild as easily as using > "bash", so I don't see why that would make it release-critical. I beg to differ. ocamlbuild uses libc's system(), it would be insane to change that. It seems that darcs is also affected. There might be also more packages affected... -- Stéphane -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
block 642835 by 642922 block 642706 by 642922 severity 642922 serious thanks Le 25/09/2011 19:28, Stéphane Glondu a écrit : > Bugs #642706 (bin-prot FTBFS) and #642835 (sexplib310 FTBFS) can be > fixed by reverting the patch submitted at [1]. I don't understand why. > > [1] http://thread.gmane.org/gmane.comp.shells.dash/556 > > While investigating #642706, in the failing case, I observed that a > cpp process called with "sh -c" gets SIGPIPE while writing to > stderr. In the succeeding case, the write is successful, and is read > by the ocamlbuild process that started "sh -c cpp ...". Raising severity, since it blocks serious bugs. Cheers, -- Stéphane -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#642922: regression: "sh -c" change causes FTBFS
Package: dash Version: 0.5.7-1 Severity: important Tags: patch Hello, Bugs #642706 (bin-prot FTBFS) and #642835 (sexplib310 FTBFS) can be fixed by reverting the patch submitted at [1]. I don't understand why. [1] http://thread.gmane.org/gmane.comp.shells.dash/556 While investigating #642706, in the failing case, I observed that a cpp process called with "sh -c" gets SIGPIPE while writing to stderr. In the succeeding case, the write is successful, and is read by the ocamlbuild process that started "sh -c cpp ...". Cheers, -- Stéphane -- System Information: Debian Release: wheezy/sid APT prefers testing APT policy: (990, 'testing'), (500, 'unstable'), (1, 'experimental') Architecture: amd64 (x86_64) Kernel: Linux 3.0.0-1-amd64 (SMP w/4 CPU cores) Locale: LANG=fr_FR.utf8, LC_CTYPE=fr_FR.utf8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Versions of packages dash depends on: ii debianutils 4.0.2 ii dpkg 1.16.0.3 ii libc62.13-21 dash recommends no packages. dash suggests no packages. -- debconf information: * dash/sh: true diff -u dash-0.5.7/debian/changelog dash-0.5.7/debian/changelog --- dash-0.5.7/debian/changelog +++ dash-0.5.7/debian/changelog @@ -1,3 +1,10 @@ +dash (0.5.7-1.1) UNRELEASED; urgency=low + + * Non-maintainer upload. + * Revert http://thread.gmane.org/gmane.comp.shells.dash/556 + + -- Stéphane Glondu Sun, 25 Sep 2011 19:05:10 +0200 + dash (0.5.7-1) unstable; urgency=low * new upstream release. only in patch2: unchanged: --- dash-0.5.7.orig/src/main.c +++ dash-0.5.7/src/main.c @@ -171,7 +171,7 @@ state3: state = 4; if (minusc) - evalstring(minusc, sflag ? 0 : EV_EXIT); + evalstring(minusc, 0); if (sflag || minusc == NULL) { state4: /* XXX ??? - why isn't this before the "if" statement */