[bug#34195,v2] linux-modules: Add modules-soft-dependencies.

* gnu/build/linux-modules.scm (not-softdep-whitespace): New variable.
(module-soft-dependencies): New procedure.
---
 gnu/build/linux-modules.scm | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> * gnu/build/linux-modules.scm (not-softdep-whitespace): New variable.
> (module-soft-dependencies): New procedure.

That was fast!  :-)


[...]

> +(define (module-soft-dependencies file)
> +  "Return a list of (cons mode soft-dependency) of module FILE."
> +  ;; TEXT: "pre: baz blubb foo post: bax bar"
> +  (define (parse-softdep text)
> +    (let loop ((value '())
> +               (tokens (string-tokenize text not-softdep-whitespace))
> +               (section #f))
> +      (match tokens
> +       ((token _ ...)
> +        (if (string=? (string-take-right token 1) ":") ; section
> +            (loop value
> +                  (cdr tokens)
> +                  (string-trim-both token))

You can use the pattern (token rest ...) and then:

  (loop value rest (string-trim-both token))

instead of the not-so-nice ‘cdr’.  :-)

> +  (let ((info (modinfo-section-contents file)))
> +    (apply append
> +     (filter-map (match-lambda
> +                  (('softdep . value)
> +                   (parse-softdep value))
> +                  (_ #f))
> +                 (modinfo-section-contents file)))))

Replace ‘apply append’ with ‘concatenate’.

OK with these changes, thank you!

Ludo’.

Hi Ludo,

>That was fast!  :-)

Yeah, I dislike ticking boot time bombs ;-)

Thanks for the review!

Pushed as 1a5f46621b44aa1458ad7acd4eca5fe1d4574f92 
and 519be98c3536b5113cde368f9dc6db2e1ebe073e (tiny fix)
to guix master.

Note that it returns something like

(("pre" . "module-1") ("pre" . "module-2"))

So the user might want to

(1) map cdr (or match ;) ) it
(2) replace dashes by underscores if a filename is desired (normalize-module-name)
(although in practise nobody in the mainline Linux seems to use dashes there
right now, their example in include/linux/module.h has dashes :P)

Example result:

scheme> (module-soft-dependencies "/tmp/vfio.ko")
$2 = (("post" . "vfio_iommu_spapr_tce") ("post" . "vfio_iommu_type1"))

Hello,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> scheme> (module-soft-dependencies "/tmp/vfio.ko")
> $2 = (("post" . "vfio_iommu_spapr_tce") ("post" . "vfio_iommu_type1"))

That’s probably not the best interface.  :-)

Perhaps it should return two values: the list of modules to be loaded
before (“pre”), followed by the list of modules to be loaded after
(“post”).

WDYT?

Ludo’.

Hi Ludo,

On Sat, 26 Jan 2019 15:10:27 +0100
Ludovic Courtès <ludo@gnu.org> wrote:

> Danny Milosavljevic <dannym@scratchpost.org> skribis:
> 
> > scheme> (module-soft-dependencies "/tmp/vfio.ko")  
> > $2 = (("post" . "vfio_iommu_spapr_tce") ("post" . "vfio_iommu_type1"))  
> 
> That’s probably not the best interface.  :-)
> 
> Perhaps it should return two values: the list of modules to be loaded
> before (“pre”), followed by the list of modules to be loaded after
> (“post”).

I had thought about it - but for our use case it makes it slower and more
complicated.

I still have the previous version (see below).

Moreover, it did the wrong thing for multiple 'softdep sections.

I guess I can use an external grouper routine (something like a hypothetical
group-by-first - does it exist?) on the result of module-soft-dependencies.

But at that point the caller can call it himself :->

We could hard-code "pre" and "post" as the two only possible sections (and
kmod does), but that would make it break in the future even though we don't
care about the sections, just the module names.

Complicated version:

(define (module-soft-dependencies file)
  "Return the list of soft dependencies of module FILE."
  (define (add-to-first-acons value alist)
    (match alist
      (((k . v) . b)
       (cons (cons k (cons value v)) b))))

  (define (parse-softdep text)
    (let loop ((value '())
               (tokens (string-tokenize text not-softdep-whitespace))
               (section #f))
      (write tokens)
      (newline)
      (match tokens
       ((token _ ...)
        (if (string=? (string-take-right token 1) ":") ; section
            (loop (acons (string-drop-right token 1) '() value)
                  (cdr tokens)
                  (string-trim-both token))
            (loop (add-to-first-acons token value)
                  (cdr tokens) section)))
       (()
        value))))

  (let ((info (modinfo-section-contents file)))
    (filter-map (match-lambda
                 (('softdep . value)
                  (parse-softdep value))
                 (_ #f))
                (modinfo-section-contents file))))

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Sat, 26 Jan 2019 15:10:27 +0100
> Ludovic Courtès <ludo@gnu.org> wrote:
>
>> Danny Milosavljevic <dannym@scratchpost.org> skribis:
>> 
>> > scheme> (module-soft-dependencies "/tmp/vfio.ko")  
>> > $2 = (("post" . "vfio_iommu_spapr_tce") ("post" . "vfio_iommu_type1"))  
>> 
>> That’s probably not the best interface.  :-)
>> 
>> Perhaps it should return two values: the list of modules to be loaded
>> before (“pre”), followed by the list of modules to be loaded after
>> (“post”).
>
> I had thought about it - but for our use case it makes it slower and more
> complicated.

Once you have the result above, you can simply do:

  (partition (match-lambda
               (("pre" . _) #t)
               (("post" . _) #f))
             $2)

and then remove the cars.  Or you can fold over the elements instead of
constructing the alist in the first place.

Anyway it should be a few more lines at most, I think.

Ludo’.