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