GNU bug report logs - #25917
operating-system file-system with (check? #t) but (needed-for-boot #f) pauses boot until user interaction

Previous Next

Package: guix;

Reported by: Danny Milosavljevic <dannym <at> scratchpost.org>

Date: Wed, 1 Mar 2017 16:39:01 UTC

Severity: normal

Done: ludo <at> gnu.org (Ludovic Courtès)

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25917 in the body.
You can then email your comments to 25917 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Wed, 01 Mar 2017 16:39:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Danny Milosavljevic <dannym <at> scratchpost.org>:
New bug report received and forwarded. Copy sent to bug-guix <at> gnu.org. (Wed, 01 Mar 2017 16:39:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: <bug-guix <at> gnu.org>
Subject: operating-system file-system with (check? #t) but (needed-for-boot
 #f) pauses boot until user interaction
Date: Wed, 1 Mar 2017 17:38:38 +0100
If one has a operating-system file-system with (check? #t) but (needed-for-boot #f), the next boot after reconfiguring will boot into a Guile REPL because the initrd doesn't contain the fsck tool for that file system (because all the file-systems with (needed-for-boot #f) have been filtered out). Therefore, the normal boot process breaks.

Possible fixes:
- If (check? #t) but (needed-for-boot? #f), include the tool anyway, XOR
- If (check? #t) but (needed-for-boot? #f), don't check anyway

This can be reproduced for example by connecting an USB flash storage stick with a filesystem with a type on it that's not used in any other file-system declaration.

NB: If (check? #t) and (needed-for-boot? #t), everything works fine.

For all these, (mount? #t) (the default).

This is on Guix master from a few minutes ago.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Sat, 11 Mar 2017 11:22:02 GMT) Full text and rfc822 format available.

Message #8 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: bug#25917: operating-system file-system with (check? #t) but
 (needed-for-boot #f) pauses boot until user interaction
Date: Sat, 11 Mar 2017 12:21:28 +0100
Hi Danny,

Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> If one has a operating-system file-system with (check? #t) but (needed-for-boot #f), the next boot after reconfiguring will boot into a Guile REPL because the initrd doesn't contain the fsck tool for that file system (because all the file-systems with (needed-for-boot #f) have been filtered out). Therefore, the normal boot process breaks.
>
> Possible fixes:
> - If (check? #t) but (needed-for-boot? #f), include the tool anyway, XOR
> - If (check? #t) but (needed-for-boot? #f), don't check anyway

I think we should do the latter.  When needed-for-boot? is #f, the check
is performed by the Shepherd service that takes care of the file system:

  https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/services/base.scm#n291

Can you look into it?

Thanks for the report!

Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Sun, 12 Mar 2017 16:57:02 GMT) Full text and rfc822 format available.

Message #11 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: 25917 <at> debbugs.gnu.org
Cc: Danny Milosavljevic <dannym <at> scratchpost.org>
Subject: [PATCH] services: Don't check filesystem even if #:check? if not
 #:needed-for-boot.
Date: Sun, 12 Mar 2017 17:56:04 +0100
* gnu/services/base.scm (file-system-shepherd-service): If
not #:needed-for-boot, don't check filesystem even if #:check? .
---
 gnu/services/base.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 5298a11f6..2628b718f 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -274,6 +274,7 @@ FILE-SYSTEM."
         (options (file-system-options file-system))
         (check?  (file-system-check? file-system))
         (create? (file-system-create-mount-point? file-system))
+        (needed-for-boot? (file-system-needed-for-boot? file-system))
         (dependencies (file-system-dependencies file-system)))
     (and (file-system-mount? file-system)
          (with-imported-modules '((gnu build file-systems)
@@ -300,7 +301,7 @@ FILE-SYSTEM."
                            (lambda ()
                              (mount-file-system
                               `(#$device #$title #$target #$type #$flags
-                                         #$options #$check?)
+                                         #$options #$(and check? needed-for-boot?))
                               #:root "/"))
                            (lambda ()
                              (setenv "PATH" $PATH)))




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Sun, 12 Mar 2017 17:02:01 GMT) Full text and rfc822 format available.

Message #14 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: 25917 <at> debbugs.gnu.org
Cc: Danny Milosavljevic <dannym <at> scratchpost.org>
Subject: [PATCH v2] services: If a filesystem is not marked as needed for boot,
 don't check it even if told to check it.
Date: Sun, 12 Mar 2017 17:59:58 +0100
* gnu/services/base.scm (file-system-shepherd-service): If
not #:needed-for-boot, don't check filesystem even if #:check? .
---
 gnu/services/base.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 5298a11f6..2628b718f 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -274,6 +274,7 @@ FILE-SYSTEM."
         (options (file-system-options file-system))
         (check?  (file-system-check? file-system))
         (create? (file-system-create-mount-point? file-system))
+        (needed-for-boot? (file-system-needed-for-boot? file-system))
         (dependencies (file-system-dependencies file-system)))
     (and (file-system-mount? file-system)
          (with-imported-modules '((gnu build file-systems)
@@ -300,7 +301,7 @@ FILE-SYSTEM."
                            (lambda ()
                              (mount-file-system
                               `(#$device #$title #$target #$type #$flags
-                                         #$options #$check?)
+                                         #$options #$(and check? needed-for-boot?))
                               #:root "/"))
                            (lambda ()
                              (setenv "PATH" $PATH)))




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Mon, 13 Mar 2017 09:02:02 GMT) Full text and rfc822 format available.

Message #17 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: bug#25917: [PATCH] services: Don't check filesystem even if
 #:check? if not #:needed-for-boot.
Date: Mon, 13 Mar 2017 10:01:32 +0100
Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> * gnu/services/base.scm (file-system-shepherd-service): If
> not #:needed-for-boot, don't check filesystem even if #:check? .
> ---
>  gnu/services/base.scm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
> index 5298a11f6..2628b718f 100644
> --- a/gnu/services/base.scm
> +++ b/gnu/services/base.scm
> @@ -274,6 +274,7 @@ FILE-SYSTEM."
>          (options (file-system-options file-system))
>          (check?  (file-system-check? file-system))
>          (create? (file-system-create-mount-point? file-system))
> +        (needed-for-boot? (file-system-needed-for-boot? file-system))
>          (dependencies (file-system-dependencies file-system)))
>      (and (file-system-mount? file-system)
>           (with-imported-modules '((gnu build file-systems)
> @@ -300,7 +301,7 @@ FILE-SYSTEM."
>                             (lambda ()
>                               (mount-file-system
>                                `(#$device #$title #$target #$type #$flags
> -                                         #$options #$check?)
> +                                         #$options #$(and check? needed-for-boot?))
>                                #:root "/"))
>                             (lambda ()
>                               (setenv "PATH" $PATH)))

One thing I don’t get is that, if the file system is not
needed-for-boot?, then it doesn’t get a Shepherd service in the first
place.

In your original message, you wrote that the problem is that “the initrd
doesn't contain the fsck tool”, so it’s a problem in linux-initrd.scm,
no?

What am I missing?

Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Mon, 13 Mar 2017 19:56:02 GMT) Full text and rfc822 format available.

Message #20 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: bug#25917: [PATCH] services: Don't check filesystem even if
 #:check? if not #:needed-for-boot.
Date: Mon, 13 Mar 2017 20:55:04 +0100
> One thing I don’t get is that, if the file system is not
> needed-for-boot?, then it doesn’t get a Shepherd service in the first
> place.

But what is supposed to happen to filesystems where mount? but not needed-for-boot? if Shepherd doesn't mount it?  Does some other part mount it ?  Is it just put into fstab or something ?

> In your original message, you wrote that the problem is that “the initrd
> doesn't contain the fsck tool”, so it’s a problem in linux-initrd.scm,
> no?

I don't know where we should fix it.  If you want to reproduce it, I tested it with this config

  (file-systems (cons* ...
                       (file-system
                        (device "NO NAME")
                        (title 'label)
                        (mount-point "/mnt/tmp")
                        (type "vfat")
                        (needed-for-boot? #f)
                        (mount? #t)
                        (check? #t))
                       %base-file-systems))

and an USB flash memory stick with name "NO NAME" (the default of the stick :) ).

The effect is if not needed-for-boot? but check? , the boot breaks because it can't find fsck.vfat .

vfat is good for testing this since it's usually the only filesystem of that type on the system and so it can't be required by another filesystem entry and mask the problem.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Tue, 14 Mar 2017 08:42:01 GMT) Full text and rfc822 format available.

Message #23 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: bug#25917: [PATCH] services: Don't check filesystem even if
 #:check? if not #:needed-for-boot.
Date: Tue, 14 Mar 2017 09:41:32 +0100
Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

>> One thing I don’t get is that, if the file system is not
>> needed-for-boot?, then it doesn’t get a Shepherd service in the first
>> place.
>
> But what is supposed to happen to filesystems where mount? but not needed-for-boot? if Shepherd doesn't mount it?  Does some other part mount it ?  Is it just put into fstab or something ?

Yes, it’s just put in fstab.  The use case for this is that then you can
type just “mount /mnt/foo” and ‘mount’ looks it up in /etc/fstab and
does the right thing.

>> In your original message, you wrote that the problem is that “the initrd
>> doesn't contain the fsck tool”, so it’s a problem in linux-initrd.scm,
>> no?
>
> I don't know where we should fix it.  If you want to reproduce it, I tested it with this config
>
>   (file-systems (cons* ...
>                        (file-system
>                         (device "NO NAME")
>                         (title 'label)
>                         (mount-point "/mnt/tmp")
>                         (type "vfat")
>                         (needed-for-boot? #f)
>                         (mount? #t)
>                         (check? #t))
>                        %base-file-systems))
>
> and an USB flash memory stick with name "NO NAME" (the default of the stick :) ).
>
> The effect is if not needed-for-boot? but check? , the boot breaks because it can't find fsck.vfat .

Ooh, I think that’s another problem, then.  :-)

‘file-system-shepherd-service’ is bogus: it only handles ext2:

                             (setenv "PATH"
                                     (string-append
                                      #$e2fsprogs "/sbin:"
                                      "/run/current-system/profile/sbin:"
                                      $PATH)))

We should fix that to handle vfat, btrfs, etc., just like ‘base-initrd’
does.  In fact, we should extract the ‘helper-packages’ thing from
there and factorize it between linux-initrd.scm and services/base.scm.

How does that sound?

Thanks,
Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Tue, 14 Mar 2017 19:19:01 GMT) Full text and rfc822 format available.

Message #26 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: bug#25917: [PATCH] services: Don't check filesystem even if
 #:check? if not #:needed-for-boot.
Date: Tue, 14 Mar 2017 20:18:09 +0100
Hi Ludo,

On Tue, 14 Mar 2017 09:41:32 +0100
ludo <at> gnu.org (Ludovic Courtès) wrote:

> Danny Milosavljevic <dannym <at> scratchpost.org> skribis:
> 
> >> One thing I don’t get is that, if the file system is not
> >> needed-for-boot?, then it doesn’t get a Shepherd service in the first
> >> place.  
> >
> > But what is supposed to happen to filesystems where mount? but not needed-for-boot? if Shepherd doesn't mount it?  Does some other part mount it ?  Is it just put into fstab or something ?  
> 
> Yes, it’s just put in fstab.  The use case for this is that then you can
> type just “mount /mnt/foo” and ‘mount’ looks it up in /etc/fstab and
> does the right thing.
> 
> >> In your original message, you wrote that the problem is that “the initrd
> >> doesn't contain the fsck tool”, so it’s a problem in linux-initrd.scm,
> >> no?  
> >
> > I don't know where we should fix it.  If you want to reproduce it, I tested it with this config
> >
> >   (file-systems (cons* ...
> >                        (file-system
> >                         (device "NO NAME")
> >                         (title 'label)
> >                         (mount-point "/mnt/tmp")
> >                         (type "vfat")
> >                         (needed-for-boot? #f)
> >                         (mount? #t)
> >                         (check? #t))
> >                        %base-file-systems))
> >
> > and an USB flash memory stick with name "NO NAME" (the default of the stick :) ).
> >
> > The effect is if not needed-for-boot? but check? , the boot breaks because it can't find fsck.vfat .  
> 
> Ooh, I think that’s another problem, then.  :-)
> 
> ‘file-system-shepherd-service’ is bogus: it only handles ext2:
> 
>                              (setenv "PATH"
>                                      (string-append
>                                       #$e2fsprogs "/sbin:"
>                                       "/run/current-system/profile/sbin:"
>                                       $PATH)))

Oops...

> We should fix that to handle vfat, btrfs, etc., just like ‘base-initrd’
> does.  In fact, we should extract the ‘helper-packages’ thing from
> there and factorize it between linux-initrd.scm and services/base.scm.

Sounds good!

However, the above has (needed-for-boot? #f) and Guix still tries to check / mount it. Why?




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Tue, 14 Mar 2017 20:19:02 GMT) Full text and rfc822 format available.

Message #29 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: bug#25917: [PATCH] services: Don't check filesystem even if
 #:check? if not #:needed-for-boot.
Date: Tue, 14 Mar 2017 21:18:37 +0100
Hi Danny,

Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

>> >   (file-systems (cons* ...
>> >                        (file-system
>> >                         (device "NO NAME")
>> >                         (title 'label)
>> >                         (mount-point "/mnt/tmp")
>> >                         (type "vfat")
>> >                         (needed-for-boot? #f)
>> >                         (mount? #t)
>> >                         (check? #t))
>> >                        %base-file-systems))

[...]

> However, the above has (needed-for-boot? #f) and Guix still tries to check / mount it. Why?

It has (mount? #t), that’s why it’s mounting and checking it.

With (mount? #f), it wouldn’t do anything other than adding a line in
/etc/fstab.

Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Thu, 16 Mar 2017 17:41:01 GMT) Full text and rfc822 format available.

Message #32 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: 25917 <at> debbugs.gnu.org,
	ludo <at> gnu.org
Cc: Danny Milosavljevic <dannym <at> scratchpost.org>
Subject: [PATCH] file-systems: Factorize file-system-packages.
Date: Thu, 16 Mar 2017 18:40:29 +0100
* gnu/system/linux-initrd.scm (base-initrd): Move helper-packages body to ...
* gnu/system/file-systems.scm (file-system-packages): ... here.
Also export it.
* gnu/services/base.scm (file-system-shepherd-service): Set PATH by using
file-system-packages.
---
 gnu/services/base.scm       | 12 ++++++------
 gnu/system/file-systems.scm | 26 ++++++++++++++++++++++++++
 gnu/system/linux-initrd.scm | 18 +-----------------
 3 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 5298a11f6..f3224aae1 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -274,7 +274,9 @@ FILE-SYSTEM."
         (options (file-system-options file-system))
         (check?  (file-system-check? file-system))
         (create? (file-system-create-mount-point? file-system))
-        (dependencies (file-system-dependencies file-system)))
+        (needed-for-boot? (file-system-needed-for-boot? file-system))
+        (dependencies (file-system-dependencies file-system))
+        (packages (file-system-packages (list file-system))))
     (and (file-system-mount? file-system)
          (with-imported-modules '((gnu build file-systems)
                                   (guix build bournish))
@@ -292,11 +294,9 @@ FILE-SYSTEM."
                          ;; Make sure fsck.ext2 & co. can be found.
                          (dynamic-wind
                            (lambda ()
-                             (setenv "PATH"
-                                     (string-append
-                                      #$e2fsprogs "/sbin:"
-                                      "/run/current-system/profile/sbin:"
-                                      $PATH)))
+                             (set-path-environment-variable "PATH"
+                                                            '("bin" "sbin")
+                                                            '#$packages))
                            (lambda ()
                              (mount-file-system
                               `(#$device #$title #$target #$type #$flags
diff --git a/gnu/system/file-systems.scm b/gnu/system/file-systems.scm
index 7011a279d..8107722c7 100644
--- a/gnu/system/file-systems.scm
+++ b/gnu/system/file-systems.scm
@@ -22,6 +22,8 @@
   #:use-module (guix records)
   #:use-module ((gnu build file-systems)
                 #:select (string->uuid uuid->string))
+  #:use-module (gnu packages linux)
+  #:use-module (gnu packages disk)
   #:re-export (string->uuid
                uuid->string)
   #:export (<file-system>
@@ -65,6 +67,8 @@
 
             file-system-mapping->bind-mount
 
+            file-system-packages
+
             %store-mapping
             %network-configuration-files
             %network-file-mappings))
@@ -411,4 +415,26 @@ a bind mount."
                  (writable? (string=? file "/etc/resolv.conf"))))
               %network-configuration-files))
 
+(define (file-system-type-predicate type)
+  (lambda (fs)
+    (string=? (file-system-type fs) type)))
+
+(define* (file-system-packages file-systems #:key (volatile-root? #f))
+ `(,@(if (find (lambda (fs)
+                 (string-prefix? "ext" (file-system-type fs)))
+               file-systems)
+         (list e2fsck/static)
+         '())
+   ,@(if (find (lambda (fs)
+                 (string-suffix? "fat" (file-system-type fs)))
+               file-systems)
+         (list fatfsck/static)
+         '())
+   ,@(if (find (file-system-type-predicate "btrfs") file-systems)
+         (list btrfs-progs/static)
+         '())
+   ,@(if volatile-root?
+         (list unionfs-fuse/static)
+         '())))
+
 ;;; file-systems.scm ends here
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index 81c1278c0..1f1c30682 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -272,23 +272,7 @@ loaded at boot time in the order in which they appear."
       ,@extra-modules))
 
   (define helper-packages
-    ;; Packages to be copied on the initrd.
-    `(,@(if (find (lambda (fs)
-                    (string-prefix? "ext" (file-system-type fs)))
-                  file-systems)
-            (list e2fsck/static)
-            '())
-      ,@(if (find (lambda (fs)
-                    (string-suffix? "fat" (file-system-type fs)))
-                  file-systems)
-            (list fatfsck/static)
-            '())
-      ,@(if (find (file-system-type-predicate "btrfs") file-systems)
-            (list btrfs-progs/static)
-            '())
-      ,@(if volatile-root?
-            (list unionfs-fuse/static)
-            '())))
+    (file-system-packages file-systems #:volatile-root? volatile-root?))
 
   (raw-initrd file-systems
               #:linux linux




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Fri, 17 Mar 2017 09:05:02 GMT) Full text and rfc822 format available.

Message #35 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: [PATCH] file-systems: Factorize file-system-packages.
Date: Fri, 17 Mar 2017 10:03:52 +0100
Hi Danny,

Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> * gnu/system/linux-initrd.scm (base-initrd): Move helper-packages body to ...
> * gnu/system/file-systems.scm (file-system-packages): ... here.
> Also export it.
> * gnu/services/base.scm (file-system-shepherd-service): Set PATH by using
> file-system-packages.

Exactly what we need!

Could you just make it two patches:

  1. introduce ‘file-system-packages’;
  2. Use it in ‘file-system-shepherd-service’.

> +(define (file-system-type-predicate type)
> +  (lambda (fs)
> +    (string=? (file-system-type fs) type)))
> +
> +(define* (file-system-packages file-systems #:key (volatile-root? #f))

Please add a docstring to these procedures.

You can also remove the now-unused ‘file-system-type-predicate’
procedure that is in ‘base-initrd’.

OK with these changes.

Thank you!

Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Fri, 17 Mar 2017 12:21:01 GMT) Full text and rfc822 format available.

Message #38 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: [PATCH] file-systems: Factorize file-system-packages.
Date: Fri, 17 Mar 2017 13:19:58 +0100
Hi Ludo,

On Fri, 17 Mar 2017 10:03:52 +0100
ludo <at> gnu.org (Ludovic Courtès) wrote:

> You can also remove the now-unused ‘file-system-type-predicate’
> procedure that is in ‘base-initrd’.

It's still used there (in order to determine the Linux modules).  Should I also export 'file-system-type-predicate` from file-systems.scm and use that in 'base-initrd` ?  I thought it was too special-case to be a public function.

Also, it seems that the new version (which now uses `set-path-environment-variable') clears the old PATH whereas the previous version prepended to it.

The previous version has:

                             (setenv "PATH"
                                     (string-append
                                      #$e2fsprogs "/sbin:"
                                      "/run/current-system/profile/sbin:"
                                      $PATH)))

(What does "$" without "#" do?)

The new version would have:

                           (lambda ()
                             (set-path-environment-variable "PATH"
                                                            '("bin" "sbin")
                                                            '#$packages))

It works fine - however, I get a warning that PATH has been unset at bootup.

Should we replicate the previous behaviour?

What's up with the hard-coded "/run/current-system/profile/sbin" ?




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Fri, 17 Mar 2017 15:03:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Sat, 18 Mar 2017 09:43:02 GMT) Full text and rfc822 format available.

Message #44 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: [PATCH] file-systems: Factorize file-system-packages.
Date: Sat, 18 Mar 2017 10:42:33 +0100
> > * gnu/system/linux-initrd.scm (base-initrd): Move helper-packages body to ...
> > * gnu/system/file-systems.scm (file-system-packages): ... here.
> > Also export it.

Pushed those to master as 7208995426714c9fc3ad59cadc3cc0f52df0f018.

> > * gnu/services/base.scm (file-system-shepherd-service): Set PATH by using
> > file-system-packages.  

Will post these to the bugreport first.  It also requires a change in guix/build/utils.scm in order for it not to drop the existing search path.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Sat, 18 Mar 2017 11:01:02 GMT) Full text and rfc822 format available.

Message #47 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: [PATCH] file-systems: Factorize file-system-packages.
Date: Sat, 18 Mar 2017 12:00:47 +0100
Hello!

Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> On Fri, 17 Mar 2017 10:03:52 +0100
> ludo <at> gnu.org (Ludovic Courtès) wrote:
>
>> You can also remove the now-unused ‘file-system-type-predicate’
>> procedure that is in ‘base-initrd’.
>
> It's still used there (in order to determine the Linux modules).  Should I also export 'file-system-type-predicate` from file-systems.scm and use that in 'base-initrd` ?  I thought it was too special-case to be a public function.
>
> Also, it seems that the new version (which now uses `set-path-environment-variable') clears the old PATH whereas the previous version prepended to it.
>
> The previous version has:
>
>                              (setenv "PATH"
>                                      (string-append
>                                       #$e2fsprogs "/sbin:"
>                                       "/run/current-system/profile/sbin:"
>                                       $PATH)))
>
> (What does "$" without "#" do?)

#$ is synonymous for ‘ungexp’; $ alone has no special meaning, and $PATH
is a regular identifier.

> The new version would have:
>
>                            (lambda ()
>                              (set-path-environment-variable "PATH"
>                                                             '("bin" "sbin")
>                                                             '#$packages))
>
> It works fine - however, I get a warning that PATH has been unset at bootup.

You could shut it up like this:

  ;; Don’t display the PATH settings.
  (with-output-to-port (%make-void-port "w")
    (lambda ()
      (set-path-environment-variable …)))

> What's up with the hard-coded "/run/current-system/profile/sbin" ?

I traced it back to 1b09031f786238b21ab10ba4c3e384ab194735df.  I think
you can remove it (probably the idea was that we were “likely” to find
other fsck commands in /run/current-system/profile/sbin, not very
reliable…).

HTH!

Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Sat, 18 Mar 2017 11:05:01 GMT) Full text and rfc822 format available.

Message #50 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: [PATCH] file-systems: Factorize file-system-packages.
Date: Sat, 18 Mar 2017 12:04:40 +0100
Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> I'd like to make the following change to guix/build/utils.scm :
>
> diff --git a/guix/build/utils.scm b/guix/build/utils.scm
> index bc6f11415..ca6360ed4 100644
> --- a/guix/build/utils.scm
> +++ b/guix/build/utils.scm
> @@ -409,7 +409,8 @@ for under the directories designated by FILES.  For example:
>                                          #:key
>                                          (separator ":")
>                                          (type 'directory)
> -                                        pattern)
> +                                        pattern
> +                                        keep-previous-path?)
>    "Look for each of FILES of the given TYPE (a symbol as returned by
>  'stat:type') in INPUT-DIRS.  Set ENV-VAR to a SEPARATOR-separated path
>  accordingly.  Example:
> @@ -430,7 +431,13 @@ denoting file names to look for under the directories designated by FILES:
>    (let* ((path  (search-path-as-list files input-dirs
>                                       #:type type
>                                       #:pattern pattern))
> -         (value (list->search-path-as-string path separator)))
> +         (value (list->search-path-as-string path separator))
> +         (previous-path (getenv env-var))
> +         (value (if (and keep-previous-path?
> +                         previous-path
> +                         (not (string-null? previous-path)))
> +                  (string-append value ":" previous-path)
> +                  value)))
>      (if (string-null? value)
>          (begin
>            ;; Never set ENV-VAR to an empty string because often, the empty
>
> Would that rebuild the world, though?

Yep.  I don’t think that is needed though: we can use
‘set-path-environment-variables’ like your patch did to set PATH
precisely.

Thanks,
Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Sat, 18 Mar 2017 14:07:02 GMT) Full text and rfc822 format available.

Message #53 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: 25917 <at> debbugs.gnu.org,
	ludo <at> gnu.org
Cc: Danny Milosavljevic <dannym <at> scratchpost.org>
Subject: [PATCH v2] services: file-system-shepherd-service: Make it find the
 fsck programs.
Date: Sat, 18 Mar 2017 15:06:32 +0100
* gnu/services/base.scm (file-system-shepherd-service): Use
file-system-packages.
---
 gnu/services/base.scm | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 5298a11f6..ab5030146 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -274,7 +274,8 @@ FILE-SYSTEM."
         (options (file-system-options file-system))
         (check?  (file-system-check? file-system))
         (create? (file-system-create-mount-point? file-system))
-        (dependencies (file-system-dependencies file-system)))
+        (dependencies (file-system-dependencies file-system))
+        (packages (file-system-packages (list file-system))))
     (and (file-system-mount? file-system)
          (with-imported-modules '((gnu build file-systems)
                                   (guix build bournish))
@@ -292,11 +293,12 @@ FILE-SYSTEM."
                          ;; Make sure fsck.ext2 & co. can be found.
                          (dynamic-wind
                            (lambda ()
-                             (setenv "PATH"
-                                     (string-append
-                                      #$e2fsprogs "/sbin:"
-                                      "/run/current-system/profile/sbin:"
-                                      $PATH)))
+                             ;; Don’t display the PATH settings.
+                             (with-output-to-port (%make-void-port "w")
+                               (lambda ()
+                                 (set-path-environment-variable "PATH"
+                                                                '("bin" "sbin")
+                                                                '#$packages))))
                            (lambda ()
                              (mount-file-system
                               `(#$device #$title #$target #$type #$flags




Information forwarded to bug-guix <at> gnu.org:
bug#25917; Package guix. (Sun, 19 Mar 2017 15:45:02 GMT) Full text and rfc822 format available.

Message #56 received at 25917 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 25917 <at> debbugs.gnu.org
Subject: Re: [PATCH v2] services: file-system-shepherd-service: Make it find
 the fsck programs.
Date: Sun, 19 Mar 2017 16:44:18 +0100
Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> * gnu/services/base.scm (file-system-shepherd-service): Use
> file-system-packages.

Please add a “Fixes …” line in the commit message.  Other than that, if
“make check-system TESTS=basic” passes, please push!

Thank you,
Ludo’.




Reply sent to ludo <at> gnu.org (Ludovic Courtès):
You have taken responsibility. (Sat, 22 Apr 2017 23:35:02 GMT) Full text and rfc822 format available.

Notification sent to Danny Milosavljevic <dannym <at> scratchpost.org>:
bug acknowledged by developer. (Sat, 22 Apr 2017 23:35:02 GMT) Full text and rfc822 format available.

Message #61 received at 25917-done <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 25917-done <at> debbugs.gnu.org
Subject: Re: bug#25917: [PATCH v2] services: file-system-shepherd-service:
 Make it find the fsck programs.
Date: Sun, 23 Apr 2017 01:34:15 +0200
Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> * gnu/services/base.scm (file-system-shepherd-service): Use
> file-system-packages.

That was pushed as 26e34e1e1288d657e92372efb6edc95c0e299247.
Closing.

Ludo’.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 21 May 2017 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 333 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.