Debian Bug report logs - #642922
regression: "sh -c" change causes FTBFS

Display info messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Stéphane Glondu <glondu@debian.org>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: regression: "sh -c" change causes FTBFS

Date: Sun, 25 Sep 2011 19:28:27 +0200

[Message part 1 (text/plain, inline)]

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  libc6        2.13-21 

dash recommends no packages.

dash suggests no packages.

-- debconf information:
* dash/sh: true

[dash.diff (text/x-diff, attachment)]

Message #10 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Stéphane Glondu <glondu@debian.org>

To: 642922@bugs.debian.org

Cc: Jonathan Nieder <jrnieder@gmail.com>, 642835@bugs.debian.org, 642706@bugs.debian.org

Subject: Re: Bug#642922: regression: "sh -c" change causes FTBFS

Date: Sun, 25 Sep 2011 19:41:30 +0200

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

Message #21 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Jonathan Nieder <jrnieder@gmail.com>

To: Stéphane Glondu <glondu@debian.org>

Cc: 642922@bugs.debian.org, 642835@bugs.debian.org, 642706@bugs.debian.org

Subject: Re: regression: "sh -c" change causes FTBFS

Date: Sun, 25 Sep 2011 13:39:44 -0500

severity 642922 important
quit

Stéphane Glondu wrote:
> 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
[...]
> Raising severity, since it blocks serious bugs.

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'll investigate further to see if the underlying problem is an
assumption in the relevant scripts that would be violated by ksh93,
too, or just a dash bug.

Thanks,
Jonathan

Message #32 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Stéphane Glondu <glondu@debian.org>

To: Jonathan Nieder <jrnieder@gmail.com>

Cc: 642922@bugs.debian.org

Subject: Re: regression: "sh -c" change causes FTBFS

Date: Mon, 26 Sep 2011 22:38:08 +0200

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

Message #37 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Jonathan Nieder <jrnieder@gmail.com>

To: Stéphane Glondu <glondu@debian.org>

Cc: 642922@bugs.debian.org

Subject: Re: regression: "sh -c" change causes FTBFS

Date: Mon, 26 Sep 2011 15:44:25 -0500

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.

Message #44 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Mehdi Dogguy <mehdi@dogguy.org>

To: Stéphane Glondu <glondu@debian.org>, 642922@bugs.debian.org

Cc: Jonathan Nieder <jrnieder@gmail.com>

Subject: Re: Bug#642922: regression: "sh -c" change causes FTBFS

Date: Mon, 26 Sep 2011 22:41:05 +0200

severity 642922 serious
thanks

On  0, Stéphane Glondu <glondu@debian.org> 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

Message #49 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Jonathan Nieder <jrnieder@gmail.com>

To: 642922@bugs.debian.org

Cc: Stéphane Glondu <glondu@debian.org>

Subject: proposed upload: dash/0.5.7-2

Date: Mon, 26 Sep 2011 17:09:54 -0500

Hi Gerrit,

I'd like to propose

 git://repo.or.cz/dash/debian/jrn.git proposed-updates

(commit efa06779) for upload to unstable.  It does very little new ---
the purpose is to fix

 - Bug#642922: packages using ocamlbuild fail to build from source
 - Bug#640334: "test -x" as root returns true for regular files on
   platforms with historical faccessat() semantics (like kfreebsd with
   libc versions before 2.13-19

and I haven't checked if it fixes #632494 (darcs testsuite regression
triggered by dash upgrade).

You can also find a source package at

 http://alioth.debian.org/~jrnieder-guest/temp/dash/dash_0.5.7-2.dsc

Thanks to Stéphane Glondu for the hints and prodding.  Thoughts of all
kinds welcome, as always.

Message #56 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Gerrit Pape <pape@smarden.org>

To: Jonathan Nieder <jrnieder@gmail.com>, 642922@bugs.debian.org

Cc: Stéphane Glondu <glondu@debian.org>

Subject: Re: Bug#642922: proposed upload: dash/0.5.7-2

Date: Tue, 27 Sep 2011 12:03:45 +0000

On Mon, Sep 26, 2011 at 05:09:54PM -0500, Jonathan Nieder wrote:
> Hi Gerrit,
> 
> I'd like to propose
> 
>  git://repo.or.cz/dash/debian/jrn.git proposed-updates
> 
> (commit efa06779) for upload to unstable.  It does very little new ---

Uploaded.

> Thanks to Stéphane Glondu for the hints and prodding.  Thoughts of all
> kinds welcome, as always.

My thoughts?  You do an awesome work on the dash and git packages ;),
thanks a lot for your valuable contributions to Debian.

Regards, Gerrit.

> 
> 
> 

Message #61 received at 642922-close@bugs.debian.org (full text, mbox, reply):

From: Jonathan Nieder <jrnieder@gmail.com>

To: 642922-close@bugs.debian.org

Subject: Bug#642922: fixed in dash 0.5.7-2

Date: Tue, 27 Sep 2011 12:17:10 +0000

Source: dash
Source-Version: 0.5.7-2

We believe that the bug you reported is fixed in the latest version of
dash, which is due to be installed in the Debian FTP archive:

ash_0.5.7-2_all.deb
  to main/d/dash/ash_0.5.7-2_all.deb
dash_0.5.7-2.diff.gz
  to main/d/dash/dash_0.5.7-2.diff.gz
dash_0.5.7-2.dsc
  to main/d/dash/dash_0.5.7-2.dsc



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 642922@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Jonathan Nieder <jrnieder@gmail.com> (supplier of updated dash package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.8
Date: Mon, 26 Sep 2011 16:39:38 -0500
Source: dash
Binary: dash ash
Architecture: all source
Version: 0.5.7-2
Distribution: unstable
Urgency: high
Maintainer: Gerrit Pape <pape@smarden.org>
Changed-By: Jonathan Nieder <jrnieder@gmail.com>
Description: 
 ash        - compatibility package for dash
 dash       - POSIX-compliant shell
Closes: 640334 642922
Changes: 
 dash (0.5.7-2) unstable; urgency=high
 .
   * debian/diff/:
     - 0004-BUILTIN-Fix-test-x-as-root...diff: new; teach "test -x"
       as root not to return true for nonexecutable files on
       platforms where faccessat() does return true in this case,
       such as glibc-bsd in squeeze (thx Christoph Egger, closes:
       #640334).
     - 0005-SHELL-Disable-sh-c-...diff: back out the optimization
       that makes dash -c "script" avoid a fork().  It caused
       several packages to fail to build from source (thx Stéphane
       Glondu, closes: #642922).
Checksums-Sha1: 
 2f66d96f0c6cc4dc89f6ff6c4f802a3d43603ea0 1083 dash_0.5.7-2.dsc
 87d77d2de11dc911938cd93fc17bf1fac2a302e3 40587 dash_0.5.7-2.diff.gz
 8cb4c5f4072a1626720d6f87a673fd2321e4cf73 28938 ash_0.5.7-2_all.deb
Checksums-Sha256: 
 987a50f6f79f854a8caa485d5d4f4b9f5205ce1eba6bc6cda92096ebec2e8107 1083 dash_0.5.7-2.dsc
 7ae4ad61bea0849e022dba92b16d46e562ae2dadc7d335a7bce13d5d436d1129 40587 dash_0.5.7-2.diff.gz
 1cbbaa64e2b6cbfbf0c3803e63e1d9a4799bf9a5a5d1b7438adb3008816236fd 28938 ash_0.5.7-2_all.deb
Files: 
 c9062bd3bfd8d1b765758552ffec24db 1083 shells optional dash_0.5.7-2.dsc
 5918091a338e94a4ec7014f0da024d4c 40587 shells optional dash_0.5.7-2.diff.gz
 7a5436d676c382e6b66cda82c7b97490 28938 shells optional ash_0.5.7-2_all.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iEYEARECAAYFAk6BGjAACgkQGJoyQbxwpv+BogCePsAZJQ9ZYrSx2UKUwc2aNGaX
MbAAnRRVLIwtyHHQQbVAb26L4Ql5kNnH
=ozSf
-----END PGP SIGNATURE-----

Message #66 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Stéphane Glondu <glondu@debian.org>

To: 642922@bugs.debian.org

Cc: Jonathan Nieder <jrnieder@gmail.com>, Debian Ocaml Maint ML <debian-ocaml-maint@lists.debian.org>

Subject: Re: regression: "sh -c" change causes FTBFS

Date: Wed, 05 Oct 2011 23:51:07 +0200

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

Message #71 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Stéphane Glondu <glondu@debian.org>

To: 642922@bugs.debian.org

Cc: Jonathan Nieder <jrnieder@gmail.com>, Debian Ocaml Maint ML <debian-ocaml-maint@lists.debian.org>

Subject: Re: regression: "sh -c" change causes FTBFS

Date: Thu, 06 Oct 2011 15:58:51 +0200

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

Message #76 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Jonathan Nieder <jrnieder@gmail.com>

To: Stéphane Glondu <glondu@debian.org>

Cc: 642922@bugs.debian.org, Debian Ocaml Maint ML <debian-ocaml-maint@lists.debian.org>

Subject: Re: regression: "sh -c" change causes FTBFS

Date: Thu, 6 Oct 2011 09:07:55 -0500

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.

Message #81 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Stéphane Glondu <glondu@debian.org>

To: Jonathan Nieder <jrnieder@gmail.com>

Cc: 642922@bugs.debian.org, Debian Ocaml Maint ML <debian-ocaml-maint@lists.debian.org>, debian-devel@lists.debian.org

Subject: Re: regression: "sh -c" change causes FTBFS

Date: Thu, 06 Oct 2011 16:35:45 +0200

[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

Message #86 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Jonathan Nieder <jrnieder@gmail.com>

To: Stéphane Glondu <glondu@debian.org>

Cc: 642922@bugs.debian.org, Debian Ocaml Maint ML <debian-ocaml-maint@lists.debian.org>

Subject: Re: regression: "sh -c" change causes FTBFS

Date: Thu, 6 Oct 2011 11:09:43 -0500

(-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/

Message #91 received at 642922@bugs.debian.org (full text, mbox, reply):

From: Jilles Tjoelker <jilles@stack.nl>

To: Jonathan Nieder <jrnieder@gmail.com>, Stéphane Glondu <glondu@debian.org>

Cc: 642922@bugs.debian.org

Subject: Re: regression: "sh -c" change causes FTBFS

Date: Sun, 9 Oct 2011 22:45:37 +0200

> > 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

Send a report that this bug log contains spam.