| Message ID | 108a67c0ef10e00162d81bcf7bc6243d6895c49b.1692569807.git.ngraves@ngraves.fr |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#65335,v2,1/5] file-systems: canonicalize-device-name: Ignore the "none" case. | expand |
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’.
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)) ((= ENOENT errno) #t) ((= ENOTDIR errno) #t) (else