[bug#63527] [PATCH 0/3] Initial attempt at rumpdisk support for the Hurd.

[Janneke Nieuwenhuizen]

Ludovic Courtès writes:

> Janneke Nieuwenhuizen <janneke@gnu.org> skribis:
>
>> This uses the Debian Salsa rumpkernel package git as upstream as that is 
>> where
>> delopment happens.  Once things have stabalized upstream may change to the
>> NetBSD git from where Debian takes their snapshots.
>
> Could you make it a comment close to the ‘origin’ form?

That makes sense, so it doesn't get lost, done.

>> * gnu/packages/hurd.scm (rumpkernel): New variable.
>
> […]
>
>> +        #~(modify-phases %standard-phases
>
> You can tell from the phases that it took of lot of sweat to get there.
> :-)
>
> Could you add a few comments while it’s fresh on your mind to explain
> the various tweaks?  The ‘build’ phase is particularly intimidating.

I have added a general comment

        ;; As we are using the Debian package as upstream, we follow their
        ;; build:
        ;;   * apply patches in debian/patches taken from the
        ;;     debian/patches/series file
        ;;   * for the configure, make, and install stages, follow
        ;;     the code in debian/rules
        ;; The Debian patchset includes a cross build feature that we
        ;; use with two differences
        ;;   * Debian uses a multiarch toolchain
        ;;   * we use cross-mig
        #:phases
        #~(modify-phases %standard-phases

a comment before 'build

            ;; The build has three toplevel entry points
            ;;   * buildrump.sh/src/build.sh: create a NetBSD-compatible
            ;;     toolchain and supports cross-compiling
            ;;   * buildrump.sh/src/lib/librumpuser: the librump* libraries
            ;;   * pci-userspace/src-gnu: the librumpdev_pci* libraries
            (replace 'build

and a comment in install

                    ;; Some libraries are duplicated/copied around in the
                    ;; build system, do not fail trying to install one
                    ;; a second time.
                    (if (file-exists? dest)
                        (format (current-error-port)
                                "warning: skipping: ~a\n" file)

>> +      (native-inputs
>> +       (list
>> +        autoconf
>> +        automake
>> +        libgcrypt
>> +        (if (%current-target-system)
>> +            (let* ((cross-base (resolve-interface '(gnu packages 
>> cross-base)))
>> +                   (cross-mig (module-ref cross-base 'cross-mig)))
>> +              (cross-mig (%current-target-system)))
>> +            mig)
>
> I think you could #:use-module (gnu packages cross-base) and refer to
> ‘cross-mig’ directly.

Right.  Weird, other packages do it like this and I copied without
thinking.  Possibly there was a circular dependency before?

> Also, watch out the indentation.  ;-)
>
>   (native-inputs
>    (list autoconf
>          automake
>          …))

Ah, changed.  I need to remember this, I'm a sucker for M-x sort-lines.

>> +      (license (list
>> +                asl2.0
>> +                boost1.0

> Could you add a comment above briefly stating what this list means?  (I
> suppose each license applies to a specific part of the source?)

Changed to

      (license
       ;; The NetBSD rumpkernel code is a big hodgepodge of softwares many of
       ;; which have their own different licensing terms, see also
       ;; 
https://salsa.debian.org/hurd-team/rumpkernel/-/blob/master/debian/copyright
       (list asl2.0
             boost1.0
             ...

> Anyhow, well done!

Yay!

(A v5 patch series with this to follow once we have figured out the
glibc/hurd patching problem.)

Greetings,

-- 
Janneke Nieuwenhuizen <janneke@gnu.org>  | GNU LilyPond https://LilyPond.org
Freelance IT https://www.JoyOfSource.com | Avatar® https://AvatarAcademy.com