Re: [PATCH] scripts: environment: Properly handle SIGINT.

[Thompson, David]

On Sat, Mar 26, 2016 at 2:23 PM, Ludovic Courtès <address@hidden> wrote:
> David Thompson <address@hidden> skribis:
>
>> Has anyone ever been really annoyed that C-c doesn't work in a bash
>> shell spawned by 'guix environment'?  Me too!  And I finally got around
>> to fixing it.  I would like to get this in before 0.10.0 is released.
>
> Indeed, that’s annoyed me a few times.  :-)  (C-z does work though.)
>
>> From ec7994eec73d322386abbcd901da1b1d2f6f7733 Mon Sep 17 00:00:00 2001
>> From: David Thompson <address@hidden>
>> Date: Sat, 26 Mar 2016 08:45:08 -0400
>> Subject: [PATCH] scripts: environment: Properly handle SIGINT.
>>
>> Switching to execlp means that the process spawned in a container is PID
>> 1, which obsoleted one of the 'guix environment --container' tests
>> because the init process can't be killed in the usual manner.
>>
>> * guix/scripts/environment.scm (launch-environment/fork): New procedure.
>> (launch-environment): Switch from system* to execlp.  Add handler for
>> SIGINT.
>> (guix-environment): Use launch-environment/fork.
>
> Isn’t it enough to add the ‘sigaction’ call to fix the C-c issue?

No, because system(3) states that "SIGINT and SIGQUIT will be ignored."

> Now, it’s nice to be PID 1 instead of PID 2, but that seems to be a
> separate issue, no?

Ideally, it would be a separate issue, if it weren't for the
above-mentioned issue that makes 'system' not usable for our purposes.

>> * tests/guix-environment-container.sh: Remove obsolete test.
>
> [...]
>
>> -if guix environment --bootstrap --container \
>> -     --ad-hoc bootstrap-binaries -- kill -SEGV 2
>> -then false;
>> -else
>> -    test $? -gt 127
>> -fi
>
> This test was added in light of <http://bugs.gnu.org/21958>.  We want to
> make sure we don’t lose that property.
>
> What happens exactly when a signal is sent to PID 1?  I would expect
> that its parent process, which is outside the container in a waitpid
> call, would simply get its exit value in the normal way, and thus,
> changing “2” to “1” in this test should do the trick.  Am I naïve?  :-)

The problem is that a process within the container cannot just kill
PID 1 since its the init process and the kernel protects it, so
changing "2" to "1" doesn't work.  The exit status of the environment
command is 0 in that case because PID 1 never received the signal and
thus exits normally.

I'll try to come up with a replacement test case, thanks for giving me
the context in which it was added.  (I should've used 'git blame'
first.)

- Dave