guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#56677] [PATCH v2 1/2] environment: Add --emulate-fhs option.


From: John Kehayias
Subject: [bug#56677] [PATCH v2 1/2] environment: Add --emulate-fhs option.
Date: Tue, 04 Oct 2022 18:32:44 +0000

Hi Ludo’!

Apologies for the long delay! I've done some more rewriting than expected, so 
let me outline some changes in the v2 patch (attached, while path 1/2 for 
glibc-for-fhs remains unchanged).

On Thu, Sep 08, 2022 at 10:58 PM, Ludovic Courtès wrote:

> Howdy,
>
> John Kehayias <john.kehayias@protonmail.com> skribis:
>
[snip]
>>> 1. Can we make the implementation more orthogonal and less entangled
>>> in the already-long ‘launch-environment/container’?
>>>
>>> Maybe that can be accomplished by moving all the code conditional
>>> on ‘emulate-fhs?’ out of the way in a separate procedure, and
>>> possibly by adding a generic hook in ‘launch-environment/container’
>>> that would call said procedure.
>>>
>>
>> Sure, this sounds like a good idea. I can certainly separate out the FHS 
>> setup to a
>> separate function and call it. But I'm not sure what you mean by a "generic 
>> hook" here.
>> Do you mean that launch-environment/container would have as an argument say 
>> a list of
>> functions it would call?
>
> Yes, or an argument with a single procedure to call at a specific
> point.  That would default to a no-op.
>

I've done some rearranging to address this and along the way fixed the command 
passing to the container to work the same as without --emulate-fhs (previously 
I hacked together a startup script that didn't properly capture the command 
given by the user).

Unfortunately, since some of the needed directories are bind mounted and others 
can be linked after creating filesystems, not all of the FHS directory setup 
can be done in the same place. To be explicit, bind mounts for all filesystems 
are set up in the early mlet of launch-environment/container, before being able 
to call a procedure to do other setup needed here.

The end result:

launch-environment/container now takes a key 'setup-hook' which defaults to #f. 
For now this only handles a single list with the function name and arguments 
(could extend this to be more general, but didn't have any use cases for it off 
hand). For the FHS container this is set, by the entry point guix-environment*, 
to the new function setup-fhs which sets up symlinks and ld.so.conf. Handling 
arguments was necessary as setup-fhs needs the profile used in the container.

So then launch-environment also gets a new 'emulate-fhs?' option so it can run 
some additional setup before running the command given by the user. What was 
the (hacky) /tmp/fhs.sh is now a setenv and invoke, before the final execlp for 
the given command.

A little more complicated perhaps, but I think functionally separates the 
different steps (bind mounts, symlinks and file creation, and running in the 
container) at least.

>>> 2. Please add tests. You can probably augment
>>> ‘tests/guix-environment-container.sh’ for that. Let us know if
>>> you’re not sure how to do that.
>>>
>>
>> Thanks, definitely forgot about that. In looking at that, I've just ran it 
>> with
>> "./pre-inst-env sh tests/guix-environment-container.sh" and see that the 
>> exit code is 0.
>> Is that the correct way to run these?
>
> The correct way is:
>
>   make check TESTS=tests/guix-environment-container.sh
>
> Compared to what you wrote, it uses ./test-env (which spawns a daemon
> that uses the local store, not /gnu/sore) and sets a bunch of
> environment variables.
>
> See
> <https://guix.gnu.org/manual/devel/en/html_node/Running-the-Test-Suite.html>.
>

Thanks, missed that somehow.

>> Secondly, I'm trying to think of what tests to add. I could of course run 
>> the same tests
>> already, but with the --emulate-fhs option, to check that there are no 
>> regressions.
>> Other than that, maybe checking that e.g. there's /etc/ld.so.cache, /lib, 
>> and so on?
>
> Right, at least you’d want to check for these files/directories.
>
> Note that since the test relies on ‘glibc-for-fhs’, it cannot be done
> the “normal way” (that is, using the local store rather than /gnu/store)
> because it would end up building the world.
>
> The solution here is to use /gnu/store, if available, and to otherwise
> skip the test (return 77).  See ‘tests/guix-pack-relocatable.sh’ up to
> line 40 on how to do that.
>
> HTH!
>

Very helpful, thanks!

I added in two tests to the end of tests/guix-environment-container.sh. One 
checks for the FHS file structure in the container and the other tries to read 
the ld cache (using 'ldconfig -p').

If we wanted to run all the non-fhs tests with --emulate-fhs, then maybe we'd 
want to make it so the FHS specific tests live in a new file and 
guix-environment-container.sh can be called in a way to enable that option? (A 
quick guess would be to just set an alias so that guix environment is always 
called with --emulate-fhs, but not sure if that works in the test environment.)

Wasn't sure if all that is necessary so I just went with the FHS-specific tests 
for now. I checked that they pass for me.

The commit changelog has gotten a bit more complicated with these changes, 
hopefully I got everything in there.

Thanks for your help on this and I'll make sure any future revisions are more 
timely!
John

Attachment: 0002-environment-Add-emulate-fhs-v2.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]