[bug#37868] [PATCH v6] system: Add kernel-module-packages to operating-system.

[Ludovic Courtès]

Hi Danny,

Danny Milosavljevic <address@hidden> skribis:

> On Sun, 15 Mar 2020 22:00:04 +0100
> Ludovic Courtès <address@hidden> wrote:
>
>> I don’t think #:allow-collisions?, #:locales? and #:relative-symlinks?
>> are needed, so I’d recommend removing them.
>
> Removing allow-collisions.
>
> Otherwise the defaults are different.
>
> I'm pretty sure that we don't need locales for Linux kernel modules,
> for example :)

#:locales? tells whether to install locales in the Guile process that
builds the profile so that it can handle non-ASCII file names, for
example.

> That said, I can do it--but it would increase build dependencies.

IMO it matters less than maintainability and conciseness in this case.
:-)

>> > +            (let* ((inputs '#$(manifest-inputs manifest))
>> > +                   (module-directories #$(input-files (manifest-inputs 
>> > manifest) "/lib/modules"))
>> > +                   (directory-entries
>> > +                    (lambda (directory-name)
>> > +                      (scandir directory-name (lambda (basename)
>> > +                                                (not (string-prefix? "." 
>> > basename))))))  
>> 
>> also one-word identifiers are preferred for local
>> variables.
>
> I'd like to do that but it would lose information here.
>
> "modules" would be too vague.  "directories" would be non-unique.
> (What "module-directories" means is "'/lib/modules'-directories", literally)
>
> "entries" would be too vague too.  Entries of what?
> (Especially since that's a procedure).
>
> I'll make it say "directory" instead of "directory-name" there.

Your call.  My point is: if we keep with the general guideline of
keeping functions small, then one-word identifiers are usually good
enough because in the context of the function it should be clear and
non-ambiguous.

> Note:
>
> The "existing-files" procedure exists only in order to allow us to
> build Linux kernels without any modules (neither in linux-libre nor anywhere
> else) and have the profile hook succeed.
>
> Maybe it's written in an overly general way for that?  What do you think?

Yeah, maybe.  It certainly looks weird to me to have a top-level
procedure for something that’s in fact quite specific to the problem at
hand (I realized when attempting to write a docstring that it’s a weird
interface, and that’s because it’s in fact very specific to what we’re
doing here.)

> (It's actually kinda bad that I ignore kernel-loadable-modules
> which have no "/lib/modules" in it (better would be an error)--but I wasn't
> sure whether manifest-inputs is guaranteed to keep the original order of
> the entries--which would be: linux-libre first)

Dunno, I guess it would be fine to error out when
‘kernel-loadable-modules’ is passed a package that doesn’t have any
modules.

Thanks,
Ludo’.