unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#25917: operating-system file-system with (check? #t) but (needed-for-boot #f) pauses boot until user interaction
@ 2017-03-01 16:38 Danny Milosavljevic
  2017-03-11 11:21 ` Ludovic Courtès
  2017-03-16 17:40 ` bug#25917: [PATCH] file-systems: Factorize file-system-packages Danny Milosavljevic
  0 siblings, 2 replies; 18+ messages in thread
From: Danny Milosavljevic @ 2017-03-01 16:38 UTC (permalink / raw)
  To: 25917

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: operating-system file-system with (check? #t) but (needed-for-boot #f) pauses boot until user interaction
  2017-03-01 16:38 bug#25917: operating-system file-system with (check? #t) but (needed-for-boot #f) pauses boot until user interaction Danny Milosavljevic
@ 2017-03-11 11:21 ` Ludovic Courtès
  2017-03-12 16:56   ` bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot Danny Milosavljevic
  2017-03-12 16:59   ` bug#25917: [PATCH v2] services: If a filesystem is not marked as needed for boot, don't check it even if told to check it Danny Milosavljevic
  2017-03-16 17:40 ` bug#25917: [PATCH] file-systems: Factorize file-system-packages Danny Milosavljevic
  1 sibling, 2 replies; 18+ messages in thread
From: Ludovic Courtès @ 2017-03-11 11:21 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 25917

Hi Danny,

Danny Milosavljevic <dannym@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’.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot.
  2017-03-11 11:21 ` Ludovic Courtès
@ 2017-03-12 16:56   ` Danny Milosavljevic
  2017-03-13  9:01     ` Ludovic Courtès
  2017-03-12 16:59   ` bug#25917: [PATCH v2] services: If a filesystem is not marked as needed for boot, don't check it even if told to check it Danny Milosavljevic
  1 sibling, 1 reply; 18+ messages in thread
From: Danny Milosavljevic @ 2017-03-12 16:56 UTC (permalink / raw)
  To: 25917

* 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)))

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH v2] services: If a filesystem is not marked as needed for boot, don't check it even if told to check it.
  2017-03-11 11:21 ` Ludovic Courtès
  2017-03-12 16:56   ` bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot Danny Milosavljevic
@ 2017-03-12 16:59   ` Danny Milosavljevic
  1 sibling, 0 replies; 18+ messages in thread
From: Danny Milosavljevic @ 2017-03-12 16:59 UTC (permalink / raw)
  To: 25917

* 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)))

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot.
  2017-03-12 16:56   ` bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot Danny Milosavljevic
@ 2017-03-13  9:01     ` Ludovic Courtès
  2017-03-13 19:55       ` Danny Milosavljevic
  0 siblings, 1 reply; 18+ messages in thread
From: Ludovic Courtès @ 2017-03-13  9:01 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 25917

Danny Milosavljevic <dannym@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’.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot.
  2017-03-13  9:01     ` Ludovic Courtès
@ 2017-03-13 19:55       ` Danny Milosavljevic
  2017-03-14  8:41         ` Ludovic Courtès
  0 siblings, 1 reply; 18+ messages in thread
From: Danny Milosavljevic @ 2017-03-13 19:55 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 25917

> 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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot.
  2017-03-13 19:55       ` Danny Milosavljevic
@ 2017-03-14  8:41         ` Ludovic Courtès
  2017-03-14 19:18           ` Danny Milosavljevic
  0 siblings, 1 reply; 18+ messages in thread
From: Ludovic Courtès @ 2017-03-14  8:41 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 25917

Danny Milosavljevic <dannym@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’.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot.
  2017-03-14  8:41         ` Ludovic Courtès
@ 2017-03-14 19:18           ` Danny Milosavljevic
  2017-03-14 20:18             ` Ludovic Courtès
  0 siblings, 1 reply; 18+ messages in thread
From: Danny Milosavljevic @ 2017-03-14 19:18 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 25917

Hi Ludo,

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

> Danny Milosavljevic <dannym@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?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot.
  2017-03-14 19:18           ` Danny Milosavljevic
@ 2017-03-14 20:18             ` Ludovic Courtès
  0 siblings, 0 replies; 18+ messages in thread
From: Ludovic Courtès @ 2017-03-14 20:18 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 25917

Hi Danny,

Danny Milosavljevic <dannym@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’.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] file-systems: Factorize file-system-packages.
  2017-03-01 16:38 bug#25917: operating-system file-system with (check? #t) but (needed-for-boot #f) pauses boot until user interaction Danny Milosavljevic
  2017-03-11 11:21 ` Ludovic Courtès
@ 2017-03-16 17:40 ` Danny Milosavljevic
  2017-03-17  9:03   ` Ludovic Courtès
  2017-03-18 14:06   ` bug#25917: [PATCH v2] services: file-system-shepherd-service: Make it find the fsck programs Danny Milosavljevic
  1 sibling, 2 replies; 18+ messages in thread
From: Danny Milosavljevic @ 2017-03-16 17:40 UTC (permalink / raw)
  To: 25917, ludo

* 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] file-systems: Factorize file-system-packages.
  2017-03-16 17:40 ` bug#25917: [PATCH] file-systems: Factorize file-system-packages Danny Milosavljevic
@ 2017-03-17  9:03   ` Ludovic Courtès
  2017-03-17 12:19     ` Danny Milosavljevic
  2017-03-18  9:42     ` Danny Milosavljevic
  2017-03-18 14:06   ` bug#25917: [PATCH v2] services: file-system-shepherd-service: Make it find the fsck programs Danny Milosavljevic
  1 sibling, 2 replies; 18+ messages in thread
From: Ludovic Courtès @ 2017-03-17  9:03 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 25917

Hi Danny,

Danny Milosavljevic <dannym@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’.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] file-systems: Factorize file-system-packages.
  2017-03-17  9:03   ` Ludovic Courtès
@ 2017-03-17 12:19     ` Danny Milosavljevic
  2017-03-18 11:00       ` Ludovic Courtès
  2017-03-18 11:04       ` Ludovic Courtès
  2017-03-18  9:42     ` Danny Milosavljevic
  1 sibling, 2 replies; 18+ messages in thread
From: Danny Milosavljevic @ 2017-03-17 12:19 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 25917

Hi Ludo,

On Fri, 17 Mar 2017 10:03:52 +0100
ludo@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" ?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] file-systems: Factorize file-system-packages.
  2017-03-17  9:03   ` Ludovic Courtès
  2017-03-17 12:19     ` Danny Milosavljevic
@ 2017-03-18  9:42     ` Danny Milosavljevic
  1 sibling, 0 replies; 18+ messages in thread
From: Danny Milosavljevic @ 2017-03-18  9:42 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 25917

> > * 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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] file-systems: Factorize file-system-packages.
  2017-03-17 12:19     ` Danny Milosavljevic
@ 2017-03-18 11:00       ` Ludovic Courtès
  2017-03-18 11:04       ` Ludovic Courtès
  1 sibling, 0 replies; 18+ messages in thread
From: Ludovic Courtès @ 2017-03-18 11:00 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 25917

Hello!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Fri, 17 Mar 2017 10:03:52 +0100
> ludo@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’.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH] file-systems: Factorize file-system-packages.
  2017-03-17 12:19     ` Danny Milosavljevic
  2017-03-18 11:00       ` Ludovic Courtès
@ 2017-03-18 11:04       ` Ludovic Courtès
  1 sibling, 0 replies; 18+ messages in thread
From: Ludovic Courtès @ 2017-03-18 11:04 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 25917

Danny Milosavljevic <dannym@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’.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH v2] services: file-system-shepherd-service: Make it find the fsck programs.
  2017-03-16 17:40 ` bug#25917: [PATCH] file-systems: Factorize file-system-packages Danny Milosavljevic
  2017-03-17  9:03   ` Ludovic Courtès
@ 2017-03-18 14:06   ` Danny Milosavljevic
  2017-03-19 15:44     ` Ludovic Courtès
  2017-04-22 23:34     ` Ludovic Courtès
  1 sibling, 2 replies; 18+ messages in thread
From: Danny Milosavljevic @ 2017-03-18 14:06 UTC (permalink / raw)
  To: 25917, ludo

* 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH v2] services: file-system-shepherd-service: Make it find the fsck programs.
  2017-03-18 14:06   ` bug#25917: [PATCH v2] services: file-system-shepherd-service: Make it find the fsck programs Danny Milosavljevic
@ 2017-03-19 15:44     ` Ludovic Courtès
  2017-04-22 23:34     ` Ludovic Courtès
  1 sibling, 0 replies; 18+ messages in thread
From: Ludovic Courtès @ 2017-03-19 15:44 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 25917

Danny Milosavljevic <dannym@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’.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#25917: [PATCH v2] services: file-system-shepherd-service: Make it find the fsck programs.
  2017-03-18 14:06   ` bug#25917: [PATCH v2] services: file-system-shepherd-service: Make it find the fsck programs Danny Milosavljevic
  2017-03-19 15:44     ` Ludovic Courtès
@ 2017-04-22 23:34     ` Ludovic Courtès
  1 sibling, 0 replies; 18+ messages in thread
From: Ludovic Courtès @ 2017-04-22 23:34 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 25917-done

Danny Milosavljevic <dannym@scratchpost.org> skribis:

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

That was pushed as 26e34e1e1288d657e92372efb6edc95c0e299247.
Closing.

Ludo’.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-04-22 23:35 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-01 16:38 bug#25917: operating-system file-system with (check? #t) but (needed-for-boot #f) pauses boot until user interaction Danny Milosavljevic
2017-03-11 11:21 ` Ludovic Courtès
2017-03-12 16:56   ` bug#25917: [PATCH] services: Don't check filesystem even if #:check? if not #:needed-for-boot Danny Milosavljevic
2017-03-13  9:01     ` Ludovic Courtès
2017-03-13 19:55       ` Danny Milosavljevic
2017-03-14  8:41         ` Ludovic Courtès
2017-03-14 19:18           ` Danny Milosavljevic
2017-03-14 20:18             ` Ludovic Courtès
2017-03-12 16:59   ` bug#25917: [PATCH v2] services: If a filesystem is not marked as needed for boot, don't check it even if told to check it Danny Milosavljevic
2017-03-16 17:40 ` bug#25917: [PATCH] file-systems: Factorize file-system-packages Danny Milosavljevic
2017-03-17  9:03   ` Ludovic Courtès
2017-03-17 12:19     ` Danny Milosavljevic
2017-03-18 11:00       ` Ludovic Courtès
2017-03-18 11:04       ` Ludovic Courtès
2017-03-18  9:42     ` Danny Milosavljevic
2017-03-18 14:06   ` bug#25917: [PATCH v2] services: file-system-shepherd-service: Make it find the fsck programs Danny Milosavljevic
2017-03-19 15:44     ` Ludovic Courtès
2017-04-22 23:34     ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).