[bug#65335,v2,5/5] gnu: home: symlink-manager: Allow dedicated device skip.

---
 gnu/home/services/symlink-manager.scm | 5 +++++
 1 file changed, 5 insertions(+)
  

Nicolas Graves <ngraves@ngraves.fr> skribis:

> ---
>  gnu/home/services/symlink-manager.scm | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gnu/home/services/symlink-manager.scm b/gnu/home/services/symlink-manager.scm
> index e4c931fbee..d3c4d01db4 100644
> --- a/gnu/home/services/symlink-manager.scm
> +++ b/gnu/home/services/symlink-manager.scm
> @@ -157,6 +157,11 @@ (define (update-symlinks-script)
>                             #t
>                             (G_ "Skipping ~a (not an empty directory)... done\n")
>                             directory))
> +                         ((= EBUSY errno)
> +                          (format
> +                           #t
> +                           (G_ "Skipping ~a (dedicated device)... done\n")
> +                           directory))

How does that relate to the rest of the patch series?  What does
“dedicate device” mean here?

Maybe add a comment giving some context.

Ludo’.
  

Hi,

(You forgot to Cc the bug in your reply, so our conversation went off
the record…)

Ludovic Courtès <ludo@gnu.org> skribis:

> Nicolas Graves <ngraves@ngraves.fr> skribis:
>
>> ---
>>  gnu/home/services/symlink-manager.scm | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/gnu/home/services/symlink-manager.scm b/gnu/home/services/symlink-manager.scm
>> index e4c931fbee..d3c4d01db4 100644
>> --- a/gnu/home/services/symlink-manager.scm
>> +++ b/gnu/home/services/symlink-manager.scm
>> @@ -157,6 +157,11 @@ (define (update-symlinks-script)
>>                             #t
>>                             (G_ "Skipping ~a (not an empty directory)... done\n")
>>                             directory))
>> +                         ((= EBUSY errno)
>> +                          (format
>> +                           #t
>> +                           (G_ "Skipping ~a (dedicated device)... done\n")
>> +                           directory))
>
> How does that relate to the rest of the patch series?  What does
> “dedicate device” mean here?

Your use case is for when ~/.local (for instance) is a separate mount
point (an uncommon use case because mounting file systems on Linux
usually requires root privileges).

I’d suggested “dedicated device” → “underlying device is busy”, and a
short comment above explaining the kind of situation where this can
occur.

Thanks,
Ludo’.
  

Hi Nicolas,

Did you have a chance to work on a revised version of this patch set?

  https://issues.guix.gnu.org/65335

Seems to me that little was missing.

Thanks in advance!

Ludo’.

Ludovic Courtès <ludo@gnu.org> skribis:

> Hi,
>
> (You forgot to Cc the bug in your reply, so our conversation went off
> the record…)
>
> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> Nicolas Graves <ngraves@ngraves.fr> skribis:
>>
>>> ---
>>>  gnu/home/services/symlink-manager.scm | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/gnu/home/services/symlink-manager.scm b/gnu/home/services/symlink-manager.scm
>>> index e4c931fbee..d3c4d01db4 100644
>>> --- a/gnu/home/services/symlink-manager.scm
>>> +++ b/gnu/home/services/symlink-manager.scm
>>> @@ -157,6 +157,11 @@ (define (update-symlinks-script)
>>>                             #t
>>>                             (G_ "Skipping ~a (not an empty directory)... done\n")
>>>                             directory))
>>> +                         ((= EBUSY errno)
>>> +                          (format
>>> +                           #t
>>> +                           (G_ "Skipping ~a (dedicated device)... done\n")
>>> +                           directory))
>>
>> How does that relate to the rest of the patch series?  What does
>> “dedicate device” mean here?
>
> Your use case is for when ~/.local (for instance) is a separate mount
> point (an uncommon use case because mounting file systems on Linux
> usually requires root privileges).
>
> I’d suggested “dedicated device” → “underlying device is busy”, and a
> short comment above explaining the kind of situation where this can
> occur.
>
> Thanks,
> Ludo’.
  

On 2023-10-05 15:07, Ludovic Courtès wrote:

> Hi Nicolas,
>
> Did you have a chance to work on a revised version of this patch set?
>
>   https://issues.guix.gnu.org/65335
>
> Seems to me that little was missing.

Indeed, I have to finish this among other things. I still use it locally
and it works well for system impermanence. 

I couldn't answer your following question :

>   Should /var/run be removed (in the same commit) from ‘directives’ in
>   (gnu build install)?

Maybe you can help a bit if you have worked on this, so that I
understand correctly if something should be done here or not. 

>
> Thanks in advance!
>
> Ludo’.
>
> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> Hi,
>>
>> (You forgot to Cc the bug in your reply, so our conversation went off
>> the record…)
>>
>> Ludovic Courtès <ludo@gnu.org> skribis:
>>
>>> Nicolas Graves <ngraves@ngraves.fr> skribis:
>>>
>>>> ---
>>>>  gnu/home/services/symlink-manager.scm | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/gnu/home/services/symlink-manager.scm b/gnu/home/services/symlink-manager.scm
>>>> index e4c931fbee..d3c4d01db4 100644
>>>> --- a/gnu/home/services/symlink-manager.scm
>>>> +++ b/gnu/home/services/symlink-manager.scm
>>>> @@ -157,6 +157,11 @@ (define (update-symlinks-script)
>>>>                             #t
>>>>                             (G_ "Skipping ~a (not an empty directory)... done\n")
>>>>                             directory))
>>>> +                         ((= EBUSY errno)
>>>> +                          (format
>>>> +                           #t
>>>> +                           (G_ "Skipping ~a (dedicated device)... done\n")
>>>> +                           directory))
>>>
>>> How does that relate to the rest of the patch series?  What does
>>> “dedicate device” mean here?
>>
>> Your use case is for when ~/.local (for instance) is a separate mount
>> point (an uncommon use case because mounting file systems on Linux
>> usually requires root privileges).
>>
>> I’d suggested “dedicated device” → “underlying device is busy”, and a
>> short comment above explaining the kind of situation where this can
>> occur.
>>
>> Thanks,
>> Ludo’.
  

Hi,

Nicolas Graves <ngraves@ngraves.fr> skribis:

> On 2023-10-05 15:07, Ludovic Courtès wrote:

[...]

>>   Should /var/run be removed (in the same commit) from ‘directives’ in
>>   (gnu build install)?
>
> Maybe you can help a bit if you have worked on this, so that I
> understand correctly if something should be done here or not. 

The directives in (gnu build install) specify files and directories that
should be created upfront when initializing a system—meaning when
running ‘guix system init’, or when populating a VM image.  The
boot/activation code assumes that these files already exist when the
system boots.

If we’re now creating /var/run in an activation snippet, then it no
longer needs to be here.

When doing such changes, I would suggest to at least run:

  make check-system TESTS=basic

to detect obvious mistakes, and perhaps run the “installed-os” test as
well (it’s much more expensive, so run it once you’re essentially done).

HTH!

Ludo’.
  

Hi Ludo,

Just to tell that the v3 I sent a few weeks ago is indeed fully tested,
and that commit messages are properly written.

Should we also think about a blog post on impermanence like nix did?

Nicolas

On 2023-10-12 08:55, Ludovic Courtès wrote:

> Hi,
>
> Nicolas Graves <ngraves@ngraves.fr> skribis:
>
>> On 2023-10-05 15:07, Ludovic Courtès wrote:
>
> [...]
>
>>>   Should /var/run be removed (in the same commit) from ‘directives’ in
>>>   (gnu build install)?
>>
>> Maybe you can help a bit if you have worked on this, so that I
>> understand correctly if something should be done here or not. 
>
> The directives in (gnu build install) specify files and directories that
> should be created upfront when initializing a system—meaning when
> running ‘guix system init’, or when populating a VM image.  The
> boot/activation code assumes that these files already exist when the
> system boots.
>
> If we’re now creating /var/run in an activation snippet, then it no
> longer needs to be here.
>
> When doing such changes, I would suggest to at least run:
>
>   make check-system TESTS=basic
>
> to detect obvious mistakes, and perhaps run the “installed-os” test as
> well (it’s much more expensive, so run it once you’re essentially done).
>
> HTH!
>
> Ludo’.
>
>
>
  

Hi Nicolas,

Nicolas Graves <ngraves@ngraves.fr> skribis:

> Just to tell that the v3 I sent a few weeks ago is indeed fully tested,
> and that commit messages are properly written.

Just pushed v3 as b1ec85533a63c10616d9260f90411ca6f362de00!

I did tweak the commit messages but that’s because I’m too picky :-) and
the changes were minor.

> Should we also think about a blog post on impermanence like nix did?

Yes, definitely!

There could also be a section in the manual with a complete OS
definition, to show how one can set up a volatile image.

Last, I realize there’s no system tests.  I’m not sure how to do that
(because system tests already run on a tmpfs overlay) but it’d be nice
to make sure “root=none” handling keeps working.

Thoughts?

Thanks for all the work!

Ludo’.
  

On 2024-02-19 18:48, Ludovic Courtès wrote:

> Hi Nicolas,
>
> Nicolas Graves <ngraves@ngraves.fr> skribis:
>
>> Just to tell that the v3 I sent a few weeks ago is indeed fully tested,
>> and that commit messages are properly written.
>
> Just pushed v3 as b1ec85533a63c10616d9260f90411ca6f362de00!
>
> I did tweak the commit messages but that’s because I’m too picky :-) and
> the changes were minor.
>
>> Should we also think about a blog post on impermanence like nix did?
>
> Yes, definitely!
>
> There could also be a section in the manual with a complete OS
> definition, to show how one can set up a volatile image.

On my todo list, though my priority right now is node-build-system to
try and package zotero properly.

>
> Last, I realize there’s no system tests.  I’m not sure how to do that
> (because system tests already run on a tmpfs overlay) but it’d be nice
> to make sure “root=none” handling keeps working.
>
> Thoughts?

I don't have a better view on this than you have. Basically I mostly
test it directly since for most changes I can always roll-back. Indeed
here it doesn't seem straightforward to check it fully, we can still
test the function bootable-kernel-arguments, but it's not that useful.
>
> Thanks for all the work!
>
> Ludo’.