[bug#33329] [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[bug#33329] [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[swedebugia]

[-- Attachment #1.1: Type: text/plain, Size: 1686 bytes --]

Hi

Ricardo, you mentioned that this needs to be fixed.

I made a patch to fix this. I did not test the patch as I have no idea 
how to do that.

I have not run the indent script on it.

Se attachment


-------- Forwarded Message --------
Subject: 	Re: GuixSD system reconfigure error
Date: 	Thu, 30 Aug 2018 17:22:53 +0200
From: 	Ricardo Wurmus <rekado@elephly.net>
To: 	Hebi Li <lihebi.com@gmail.com>
CC: 	help-guix@gnu.org




Hi,

>      https://github.com/lihebi/dothebi/blob/master/config.scm

I see you use the “shpchp” kernel module. This no longer exists in the
latest version of the kernel Linux. That’s what this really bad error
message tries to tell you:

> Backtrace:
>             8 (primitive-load "/gnu/store/vz7ci9rh483f2zps2cl174rd91b?")
> In ice-9/eval.scm:
>      619:8  7 (_ #f)
>     626:19  6 (_ #<directory (guile-user) 7cf140>)
>      159:9  5 (_ _)
> In gnu/build/linux-modules.scm:
>     184:47  4 (recursive-module-dependencies _ #:lookup-module _)
>      98:14  3 (module-dependencies _)
>      85:18  2 (modinfo-section-contents _)
> In ice-9/ports.scm:
>     439:11  1 (call-with-input-file #f #<procedure get-bytevector-al?> ?)
> In unknown file:
>             0 (open-file #f "r" #:encoding #f #:guess-encoding #f)
>
> ERROR: In procedure open-file:
> Wrong type (expecting string): #f
> builder for `/gnu/store/0ahsvp7wx52zzh1rywbdbq78llcwb7id-linux-modules.drv' failed with exit code 1

(I only know this because I had the same problem.) Obviously, this
error message should be changed.

As a workaround you could switch to the LTS kernel where that module
still exists.

--
Ricardo



[-- Attachment #1.2: Type: text/html, Size: 3496 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Deprecate-linux-module-shpchp-and-tell-user-to-r.patch --]
[-- Type: text/x-patch; name="0001-gnu-Deprecate-linux-module-shpchp-and-tell-user-to-r.patch", Size: 5108 bytes --]

From d67d90b9ab0aa6b7a9532ccb3788a34edde7bc52 Mon Sep 17 00:00:00 2001
From: swedebugia <swedebugia@riseup.net>
Date: Fri, 9 Nov 2018 22:52:12 +0100
Subject: [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove
 it.

 * gnu/build/linux-modules.scm (load-linux-module*): Add argument
 'remove-deprecated-modules'.
 * gnu/build/linux-modules.scm (load-dependencies): Add keyword
 '#:remove-deprecate-modules'.
 * gnu/system/mapped-devices.scm (check-device-initrd-modules): New if
 statement raising a condition if shpchp is found.
---
 gnu/build/linux-modules.scm   | 13 +++++++++++--
 gnu/system/mapped-devices.scm | 28 +++++++++++++++++++---------
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/gnu/build/linux-modules.scm b/gnu/build/linux-modules.scm
index 2d8117504..999ab4fc6 100644
--- a/gnu/build/linux-modules.scm
+++ b/gnu/build/linux-modules.scm
@@ -1,6 +1,8 @@
+
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2016, 2018 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2018 swedebugia <swedebugia@riseup.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -227,7 +229,13 @@ not a file name."
                              #:key
                              (recursive? #t)
                              (lookup-module dot-ko)
-                             (black-list (module-black-list)))
+                             (black-list (module-black-list))
+                             ;; Deprecate modules included in linux-libre but
+                             ;; which might still be present in the users
+                             ;; config.scm.
+                             ;; We tell the users to remove it from their
+                             ;; config.scm in gnu/system/mapped-devices.scm
+                             (remove-deprecated-modules 'shpchp))
   "Load Linux module from FILE, the name of a '.ko' file; return true on
 success, false otherwise.  When RECURSIVE? is true, load its dependencies
 first (à la 'modprobe'.)  The actual files containing modules depended on are
@@ -245,7 +253,8 @@ appears in BLACK-LIST are not loaded."
     (let ((dependencies (module-dependencies file)))
       (every (cut load-linux-module* <>
                   #:lookup-module lookup-module
-                  #:black-list black-list)
+                  #:black-list black-list
+                  #:remove-deprecate-modules remove-deprecate-modules)
              (map lookup-module dependencies))))
 
   (and (not (black-listed? (file-name->module-name file)))
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index a87466646..d30372c25 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2014, 2015, 2016, 2017, 2018 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 Andreas Enge <andreas@enge.fr>
 ;;; Copyright © 2017, 2018 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2018 swedebugia <swedebugia@riseup.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -142,13 +143,22 @@ DEVICE must be a \"/dev\" file name."
         ;; "usb_storage"), not file names (e.g., "usb-storage.ko").  This is
         ;; OK because we have machinery that accepts both the hyphen and the
         ;; underscore version.
-        (raise (condition
-                (&message
-                 (message (format #f (G_ "you may need these modules \
+        (if (eqv (missing 'shpchp)
+                 ;; Tell user to remove shpchp from config.scm
+                 ;; True
+                 (raise (condition
+                         (&message
+                          (message (format #f (G_ "shpchp is no longer \
+needed because it has been included in linux-libre. Please remove it from \
+your config.scm to continue"))))))
+                 ;; Else
+                 (raise (condition
+                         (&message
+                          (message (format #f (G_ "you may need these modules \
 in the initrd for ~a:~{ ~a~}")
-                                  device missing)))
-                (&fix-hint
-                 (hint (format #f (G_ "Try adding them to the
+                                           device missing)))
+                         (&fix-hint
+                          (hint (format #f (G_ "Try adding them to the
 @code{initrd-modules} field of your @code{operating-system} declaration, along
 these lines:
 
@@ -161,9 +171,9 @@ these lines:
 
 If you think this diagnostic is inaccurate, use the @option{--skip-checks}
 option of @command{guix system}.\n")
-                               missing)))
-                (&error-location
-                 (location (source-properties->location location)))))))))
+                                        missing)))
+                         (&error-location
+                          (location (source-properties->location location)))))))))
 
 \f
 ;;;
-- 
2.18.0

[bug#33329] [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[swedebugia]

[-- Attachment #1.1: Type: text/plain, Size: 1906 bytes --]

Hi

Updated patch attached with a fix of parens and removed my changes to 
linux-modules.scm.

I look forward to the review.

Cheers

On 2018-11-09 23:47, swedebugia wrote:
>
> Hi
>
> Ricardo, you mentioned that this needs to be fixed.
>
> I made a patch to fix this. I did not test the patch as I have no idea 
> how to do that.
>
> I have not run the indent script on it.
>
> Se attachment
>
>
> -------- Forwarded Message --------
> Subject: 	Re: GuixSD system reconfigure error
> Date: 	Thu, 30 Aug 2018 17:22:53 +0200
> From: 	Ricardo Wurmus <rekado@elephly.net>
> To: 	Hebi Li <lihebi.com@gmail.com>
> CC: 	help-guix@gnu.org
>
>
>
>
> Hi,
>
>>      https://github.com/lihebi/dothebi/blob/master/config.scm
>
> I see you use the “shpchp” kernel module. This no longer exists in the
> latest version of the kernel Linux. That’s what this really bad error
> message tries to tell you:
>
>> Backtrace:
>>             8 (primitive-load "/gnu/store/vz7ci9rh483f2zps2cl174rd91b?")
>> In ice-9/eval.scm:
>>      619:8  7 (_ #f)
>>     626:19  6 (_ #<directory (guile-user) 7cf140>)
>>      159:9  5 (_ _)
>> In gnu/build/linux-modules.scm:
>>     184:47  4 (recursive-module-dependencies _ #:lookup-module _)
>>      98:14  3 (module-dependencies _)
>>      85:18  2 (modinfo-section-contents _)
>> In ice-9/ports.scm:
>>     439:11  1 (call-with-input-file #f #<procedure get-bytevector-al?> ?)
>> In unknown file:
>>             0 (open-file #f "r" #:encoding #f #:guess-encoding #f)
>>
>> ERROR: In procedure open-file:
>> Wrong type (expecting string): #f
>> builder for `/gnu/store/0ahsvp7wx52zzh1rywbdbq78llcwb7id-linux-modules.drv' failed with exit code 1
>
> (I only know this because I had the same problem.) Obviously, this
> error message should be changed.
>
> As a workaround you could switch to the LTS kernel where that module
> still exists.
>
> --
> Ricardo
>
>
-- 
Cheers
Swedebugia


[-- Attachment #1.2: Type: text/html, Size: 4292 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-V2-gnu-Check-for-linux-module-shpchp-and-tell-user-t.patch --]
[-- Type: text/x-patch; name="0001-V2-gnu-Check-for-linux-module-shpchp-and-tell-user-t.patch", Size: 3062 bytes --]

From 4d70dda8c2f119fc6ff9d221eae6f060ff1fcd98 Mon Sep 17 00:00:00 2001
From: swedebugia <swedebugia@riseup.net>
Date: Fri, 9 Nov 2018 22:52:12 +0100
Subject: [PATCH] [V2] gnu: Check for linux-module shpchp and tell user to
 remove it.

 * gnu/system/mapped-devices.scm (check-device-initrd-modules): New if
 statement raising a condition if shpchp is found.
---
 gnu/packages/cran.go.3Q3wbP   |  0
 gnu/system/mapped-devices.scm | 28 +++++++++++++++++++---------
 2 files changed, 19 insertions(+), 9 deletions(-)
 create mode 100644 gnu/packages/cran.go.3Q3wbP

diff --git a/gnu/packages/cran.go.3Q3wbP b/gnu/packages/cran.go.3Q3wbP
new file mode 100644
index 000000000..e69de29bb
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index a87466646..483f952c6 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2014, 2015, 2016, 2017, 2018 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 Andreas Enge <andreas@enge.fr>
 ;;; Copyright © 2017, 2018 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2018 swedebugia <swedebugia@riseup.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -142,13 +143,22 @@ DEVICE must be a \"/dev\" file name."
         ;; "usb_storage"), not file names (e.g., "usb-storage.ko").  This is
         ;; OK because we have machinery that accepts both the hyphen and the
         ;; underscore version.
-        (raise (condition
-                (&message
-                 (message (format #f (G_ "you may need these modules \
+        (if (eqv? (missing 'shpchp))
+                 ;; Tell user to remove shpchp from config.scm
+                 ;; True
+                 (raise (condition
+                         (&message
+                          (message (format #f (G_ "shpchp is no longer \
+needed because it has been included in linux-libre. Please remove it from \
+your config.scm to continue"))))))
+                 ;; Else
+                 (raise (condition
+                         (&message
+                          (message (format #f (G_ "you may need these modules \
 in the initrd for ~a:~{ ~a~}")
-                                  device missing)))
-                (&fix-hint
-                 (hint (format #f (G_ "Try adding them to the
+                                           device missing)))
+                         (&fix-hint
+                          (hint (format #f (G_ "Try adding them to the
 @code{initrd-modules} field of your @code{operating-system} declaration, along
 these lines:
 
@@ -161,9 +171,9 @@ these lines:
 
 If you think this diagnostic is inaccurate, use the @option{--skip-checks}
 option of @command{guix system}.\n")
-                               missing)))
-                (&error-location
-                 (location (source-properties->location location)))))))))
+                                        missing)))
+                         (&error-location
+                          (location (source-properties->location location))))))))))
 
 \f
 ;;;
-- 
2.18.0

[bug#33329] [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[Ludovic Courtès]

Hello,

swedebugia <swedebugia@riseup.net> skribis:

> From 4d70dda8c2f119fc6ff9d221eae6f060ff1fcd98 Mon Sep 17 00:00:00 2001
> From: swedebugia <swedebugia@riseup.net>
> Date: Fri, 9 Nov 2018 22:52:12 +0100
> Subject: [PATCH] [V2] gnu: Check for linux-module shpchp and tell user to
>  remove it.
>
>  * gnu/system/mapped-devices.scm (check-device-initrd-modules): New if
>  statement raising a condition if shpchp is found.

I don’t think we should hard-code things like this: they would
accumulate over the years and become unmanageable.

If anything, what should be improved IMO is the error message you get
when specifying a module that is unavailable.  That’s not easily done
though since that happens at build time.

Thoughts?

Ludo’.

[bug#33329] [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[Brett Gilio]

Ludovic Courtès writes:

> If anything, what should be improved IMO is the error message you get
> when specifying a module that is unavailable.  That’s not easily done
> though since that happens at build time.

I think Ludo is correct in this, an error message seems to be the only
option we have to ensure that maintainability and reproducibility are
respected. To add onto that, I was thinking that maybe it could be part
of the configuration process that we could modify to ensure that all of
the specified modules that are needed are available else it throws an
error?

Brett Gilio

[bug#33329] [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[swedebugia]

On 2018-11-11 01:15, Brett Gilio wrote:
> 
> Ludovic Courtès writes:
> 
>> If anything, what should be improved IMO is the error message you get
>> when specifying a module that is unavailable.  That’s not easily done
>> though since that happens at build time.
> 
> I think Ludo is correct in this, an error message seems to be the only
> option we have to ensure that maintainability and reproducibility are
> respected. To add onto that, I was thinking that maybe it could be part
> of the configuration process that we could modify to ensure that all of
> the specified modules that are needed are available else it throws an
> error?

Ok. Would my patch have worked anyway?

So where would that error message be produced?
In gnu/build/linux-modules? load-linux-modules?

-- 
Cheers
Swedebugia

[bug#33329] [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[Ludovic Courtès]

swedebugia <swedebugia@riseup.net> skribis:

> On 2018-11-11 01:15, Brett Gilio wrote:
>>
>> Ludovic Courtès writes:
>>
>>> If anything, what should be improved IMO is the error message you get
>>> when specifying a module that is unavailable.  That’s not easily done
>>> though since that happens at build time.
>>
>> I think Ludo is correct in this, an error message seems to be the only
>> option we have to ensure that maintainability and reproducibility are
>> respected. To add onto that, I was thinking that maybe it could be part
>> of the configuration process that we could modify to ensure that all of
>> the specified modules that are needed are available else it throws an
>> error?
>
> Ok. Would my patch have worked anyway?

I spotted a typo: (eqv? (missing 'shpchp)) is wrong because ‘eqv?’
should take two arguments and ‘missing’ is not a procedure.

Apart from this the patch could have worked I guess.

> So where would that error message be produced?
> In gnu/build/linux-modules? load-linux-modules?

An error message is produced while building the initrd; see commit
4db7a9dc663c5b26e45ec35538bf68ff87acdf7b.

For now, what about closing this issue and opening a new one when we
have an idea on how to improve on this?

Thanks,
Ludo’.

[bug#33329] [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[swedebugia]

On 2018-11-11 12:32, Ludovic Courtès wrote:
>> swedebugia <swedebugia@riseup.net> skribis:

snip

>> Ok. Would my patch have worked anyway?
> 
> I spotted a typo: (eqv? (missing 'shpchp)) is wrong because ‘eqv?’
> should take two arguments and ‘missing’ is not a procedure.

Ok. Thanks for taking your time.

> 
> Apart from this the patch could have worked I guess.
> 
>> So where would that error message be produced?
>> In gnu/build/linux-modules? load-linux-modules?
> 
> An error message is produced while building the initrd; see commit
> 4db7a9dc663c5b26e45ec35538bf68ff87acdf7b.

Good, I had not seen that.

> 
> For now, what about closing this issue and opening a new one when we
> have an idea on how to improve on this?

Fine with me :)

-- 
Cheers
Swedebugia

bug#33329: [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[Ludovic Courtès]

swedebugia <swedebugia@riseup.net> skribis:

> On 2018-11-11 12:32, Ludovic Courtès wrote:

[...]

>>> So where would that error message be produced?
>>> In gnu/build/linux-modules? load-linux-modules?
>>
>> An error message is produced while building the initrd; see commit
>> 4db7a9dc663c5b26e45ec35538bf68ff87acdf7b.
>
> Good, I had not seen that.
>
>>
>> For now, what about closing this issue and opening a new one when we
>> have an idea on how to improve on this?
>
> Fine with me :)

Alright, done!

Ludo’.

[bug#33329] [PATCH] gnu: Deprecate linux-module shpchp and tell user to remove it.

[Ludovic Courtès]

Brett Gilio <brettg@posteo.net> skribis:

> Ludovic Courtès writes:
>
>> If anything, what should be improved IMO is the error message you get
>> when specifying a module that is unavailable.  That’s not easily done
>> though since that happens at build time.
>
> I think Ludo is correct in this, an error message seems to be the only
> option we have to ensure that maintainability and reproducibility are
> respected. To add onto that, I was thinking that maybe it could be part
> of the configuration process that we could modify to ensure that all of
> the specified modules that are needed are available else it throws an
> error?

That already happens, but the check cannot be 100% accurate because it
relies on information from the currently running kernel, which is why
there’s the ‘--skip-checks’ option.

Ludo’.