bug#35640: Make USB modems just work on Guix

["Ludovic Courtès" <ludo@gnu.org>]

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> I am uncertain if having paragraphs like this in
> modem-manager-service-type and usb-modeswitch-service-type is a good
> idea:
>
>      This service is part of ‘%desktop-services’ (*note Desktop
>      Services::).

I think it’s OK.

Looks like we’re almost there!  A couple of comments and suggestions:

>From db6e3a937f89811017abf06c7dffc494b71b9127 Mon Sep 17 00:00:00 2001
> From: Florian Pelz <pelzflorian@pelzflorian.de>
> Date: Tue, 11 Jun 2019 16:56:46 +0200
> Subject: [PATCH 2/3] Add usb-modeswitch-service-type.
>
> * gnu/services/networking.scm (usb-modeswitch-service-type): New variable.
> (usb-modeswitch-configuration): New variable.
> (usb-modeswitch-configuration->udev-rules): New procedure.
> * doc/guix.texi (Networking Services): Document it.

[...]

> +When plugged in, some USB modems (and other USB devices) initially present
> +themselves as a read-only storage medium and not as a modem.  They need to be
> +@dfn{modeswitched} before they are usable.  The USB_ModeSwitch service type
> +installs UDEV rules to automatically modeswitch these devices when they are

s/UDEV/udev/ (here and elsewhere).

> +(define (usb-modeswitch-configuration->udev-rules config)
> +  "Build a rules file for extending udev-service-type from the rules in the
> +usb-modeswitch package.  The rules file will invoke usb_modeswitch.sh from the
> +usb-modeswitch package, modified to pass the right config file."
> +  (define usb-modeswitch-sh
> +    (match-lambda
> +      (($ <usb-modeswitch-configuration> usb-modeswitch data config-file)
> +       (let ((orig #~(string-append #$usb-modeswitch:dispatcher
> +                                    "/lib/udev/usb_modeswitch")))
> +         (computed-file
> +          "usb_modeswitch-sh"

(computed-file …) should probably be moved to a separate procedure, to
keep the main procedure concise.

> +          (with-imported-modules '((guix build utils))
> +            #~(begin
> +                (use-modules (guix build utils))
> +                (let ((cfg-param
> +                       (case #$config-file
> +                         ((#f) "")
> +                         (else
> +                          (string-append " --config-file=" #$config-file)))))

Instead of (case …), maybe this would be slightly clearer:

  #$(if config-file
        #~(string-append … #$config-file)
        "")

> +                  (mkdir #$output)
> +                  (install-file #$orig #$output)
> +                  (substitute* (string-append #$output "/usb_modeswitch")
> +                    (("(exec usb_modeswitch_dispatcher .*)( 2>>)" _ left right)
> +                     (string-append left cfg-param right))
> +                    (("(exec usb_modeswitch_dispatcher .*)( &)" _ left right)
> +                     (string-append left cfg-param right)))

Perhaps add comments to explain what’s happening here.

> +                  ;; wrap-program needs bash in PATH:
> +                  (putenv (string-append "PATH=" #$bash "/bin"))

Why do you think so?  ‘wrap-program’ does not read $PATH, AFAICS.

>From dc50f979f77d9298e8c81e6943076c612e5f459a Mon Sep 17 00:00:00 2001
> From: Florian Pelz <pelzflorian@pelzflorian.de>
> Date: Wed, 12 Jun 2019 11:16:57 +0200
> Subject: [PATCH 3/3] services: Include USB_ModeSwitch in %desktop-services.
>
> Fixes <https://bugs.gnu.org/35640>.
>
> * gnu/services/desktop.scm (%desktop-services): Add usb-modeswitch-service-type.
> * doc/guix.texi (Networking Services): Document it.

OK!

Thank you!

Ludo’.