Re: [PATCH v6 1/2] gnu: Add arduino-libraries.

[Ricardo Wurmus]

Danny Milosavljevic <address@hidden> writes:

> * gnu/packages/arduino.scm (arduino-installer, arduino-libraries): New 
> variables.

I think we’d just write “New file.” without mentioning the new variables.

> * 
> gnu/packages/patches/arduino-libraries-hardware-remove-__cxa_guard_acquire.patch:
>  New file.
> * gnu/local.mk (dist_patch_DATA, GNU_SYSTEM_MODULES): Add them.
> ---

[…]

> +(define-module (gnu packages arduino)
> +  #:use-module ((guix licenses) #:prefix license:)
> +  #:use-module (guix utils)
> +  #:use-module (guix download)
> +  #:use-module (guix packages)
> +  #:use-module (guix build-system gnu)
> +  #:use-module (guix build-system python)
> +  #:use-module (guix build-system ant)
> +  #:use-module (gnu packages)
> +  #:use-module (gnu packages avr)
> +  #:use-module (gnu packages flashing-tools)
> +  #:use-module (gnu packages java)
> +  #:use-module (gnu packages python))
> +
> +(define (arduino-installer filenames)
> +  `(lambda* (#:key outputs #:allow-other-keys)
> +    (let* ((out (assoc-ref outputs "out"))
> +           (out-share (string-append out "/share/arduino")))
> +      (mkdir-p out-share)
> +      (for-each
> +        (lambda (filename)
> +          (let* ((out-share-part (string-append out-share "/" filename)))
> +            (copy-recursively filename out-share-part)))
> +        (list ,@filenames)))))

As this is only used by “arduino-libraries” and probably no other
package I’d move this into the definition of “arduino-libraries”.

> +(define-public arduino-libraries
> +  (package
> +    (name "arduino-libraries")
> +    (version "1.6.10")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/arduino/Arduino/";
> +                                  "archive/" version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "15gbg64i2ac6d0mlnbla567sn26494cvqwf5q53xzf4b5v2rb0jc"))
> +              (file-name (string-append name "-" version ".tar.gz"))
> +              (modules '((guix build utils)))
> +              (snippet
> +                 '(begin
> +                    ;; Delete bundled jar archives.
> +                    (for-each delete-file (find-files "." "\\.jar$"))
> +                    ;; Delete non-Free Software
> +                    (delete-file-recursively "libraries/WiFi/extras/wifiHD")
> +                    (delete-file-recursively 
> "libraries/WiFi/extras/wifi_dnld")
> +                    #t))
> +              (patches (search-patches
> +                        
> "arduino-libraries-hardware-remove-__cxa_guard_acquire.patch"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f ; no tests exist
> +       #:phases
> +        (modify-phases %standard-phases
> +          (delete 'configure)
> +          (delete 'build)
> +          (replace 'install ,(arduino-installer '("libraries" 
> "hardware"))))))
> +    (home-page "https://www.arduino.cc/";)
> +    (synopsis "Arduino Libraries and Hardware Spec Files")

Please use lower case.

> +    (description "arduino-libraries contains libraries and Arduino
> +hardware spec files (boards.txt).")

s/arduino-libraries/This package/

Could you expand the description a little?  Maybe one more sentence.

> +    ;; gpl covers the main body ("app", "core").
> +    ;; lgpl2.1+ covers the remainder mostly.
> +    ;; Exceptions are:
> +    ;;   libraries/Ethernet/src/Dns*: asl2.0
> +    ;;   libraries/Ethernet/src/EthernetUdp*: expat
> +    ;;   libraries/Ethernet/src/utility/w5100*: lgpl2.1

Only 2.1?  Or is a later version okay?

> +    ;;   libraries/SD: gpl3+
> +    ;;   libraries/TFT: bsd-3
> +    ;;   libraries/WiFi: bsd-3

Nitpick: the license names should be capitalised.

> +    (license (list license:lgpl2.1+
> +                   license:asl2.0
> +                   license:expat
> +                   license:bsd-3))))

The list should also include “gpl3+”.

> diff --git 
> a/gnu/packages/patches/arduino-libraries-hardware-remove-__cxa_guard_acquire.patch
>  
> b/gnu/packages/patches/arduino-libraries-hardware-remove-__cxa_guard_acquire.patch

Please add a comment at the very top of this patch to explain what its
purpose is.

Thanks!

~~ Ricardo