[PATCH v6 0/2] gnu: Add Arduino tools

[PATCH v6 0/2] gnu: Add Arduino tools

[Danny Milosavljevic]

Danny Milosavljevic (2):
  gnu: Add arduino-libraries.
  gnu: Add arduino-makefile.

 gnu/local.mk                                       |   2 +
 gnu/packages/arduino.scm                           | 172 +++++++++++++++++++++
 ...aries-hardware-remove-__cxa_guard_acquire.patch |  13 ++
 3 files changed, 187 insertions(+)
 create mode 100644 gnu/packages/arduino.scm
 create mode 100644 gnu/packages/patches/arduino-libraries-hardware-remove-__cxa_guard_acquire.patch

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

[Danny Milosavljevic]

* gnu/packages/arduino.scm (arduino-installer, arduino-libraries): 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.
---
 gnu/local.mk                                       |  2 +
 gnu/packages/arduino.scm                           | 91 ++++++++++++++++++++++
 ...aries-hardware-remove-__cxa_guard_acquire.patch | 13 ++++
 3 files changed, 106 insertions(+)
 create mode 100644 gnu/packages/arduino.scm
 create mode 100644 gnu/packages/patches/arduino-libraries-hardware-remove-__cxa_guard_acquire.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index d75ab54..66ef07b 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -40,6 +40,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/packages/anthy.scm			\
   %D%/packages/apl.scm				\
   %D%/packages/apr.scm				\
+  %D%/packages/arduino.scm			\
   %D%/packages/aspell.scm			\
   %D%/packages/assembly.scm			\
   %D%/packages/attr.scm				\
@@ -439,6 +440,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/antiword-CVE-2014-8123.patch			\
   %D%/packages/patches/apr-skip-getservbyname-test.patch	\
   %D%/packages/patches/arb-ldconfig.patch			\
+  %D%/packages/patches/arduino-hardware-patch-out-__cxa_guard_acquire.patch \
   %D%/packages/patches/ath9k-htc-firmware-binutils.patch	\
   %D%/packages/patches/ath9k-htc-firmware-gcc.patch		\
   %D%/packages/patches/ath9k-htc-firmware-objcopy.patch		\
diff --git a/gnu/packages/arduino.scm b/gnu/packages/arduino.scm
new file mode 100644
index 0000000..5309587
--- /dev/null
+++ b/gnu/packages/arduino.scm
@@ -0,0 +1,91 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Danny Milosavljevic <dannym@scratchpost.org>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(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)))))
+
+(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")
+    (description "arduino-libraries contains libraries and Arduino
+hardware spec files (boards.txt).")
+    ;; 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
+    ;;   libraries/SD: gpl3+
+    ;;   libraries/TFT: bsd-3
+    ;;   libraries/WiFi: bsd-3
+    (license (list license:lgpl2.1+
+                   license:asl2.0
+                   license:expat
+                   license:bsd-3))))
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
new file mode 100644
index 0000000..75d12cf
--- /dev/null
+++ b/gnu/packages/patches/arduino-libraries-hardware-remove-__cxa_guard_acquire.patch
@@ -0,0 +1,13 @@
+--- ./hardware/arduino/avr/cores/arduino/PluggableUSB.cpp.orig	2016-08-16 04:40:10.449580005 +0200
++++ ./hardware/arduino/avr/cores/arduino/PluggableUSB.cpp	2016-08-16 04:40:17.661634498 +0200
+@@ -97,9 +97,9 @@
+ 	// restart USB layer???
+ }
+ 
++static PluggableUSB_ obj;
+ PluggableUSB_& PluggableUSB()
+ {
+-	static PluggableUSB_ obj;
+ 	return obj;
+ }
+ 

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

[Ricardo Wurmus]

Danny Milosavljevic <dannym@scratchpost.org> 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

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

[Danny Milosavljevic]

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

I suppressed arduino-core in this patch series. It will be used by others.

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

What should I write?

Would "There are libraries for Ethernet, GSM, LCD, SD drives, servos, stepper motors, TFT displays and WiFi. " be OK?

> > +    ;;   libraries/Ethernet/src/utility/w5100*: lgpl2.1  
> 
> Only 2.1?  Or is a later version okay?

Only 2.1.

> > 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.

"Removes function-static variable. Our avr-gcc chokes on it otherwise." ?

[PATCH v6 2/2] gnu: Add arduino-makefile.

[Danny Milosavljevic]

* gnu/packages/arduino.scm (arduino-makefile): New variable.
---
 gnu/packages/arduino.scm | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/gnu/packages/arduino.scm b/gnu/packages/arduino.scm
index 5309587..08ed1cf 100644
--- a/gnu/packages/arduino.scm
+++ b/gnu/packages/arduino.scm
@@ -89,3 +89,84 @@ hardware spec files (boards.txt).")
                    license:asl2.0
                    license:expat
                    license:bsd-3))))
+
+(define-public arduino-makefile
+  (package
+    (name "arduino-makefile")
+    (version "1.5.1")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "https://github.com/sudar/Arduino-Makefile/"
+                                  "archive/" version ".tar.gz"))
+              (sha256
+               (base32
+                "1gqmcg2jg62b915akbkivnqf8sx76gv719vx7azm47szd0w1i94i"))
+              (file-name (string-append name "-" version ".tar.gz"))))
+    (build-system python-build-system)
+    (arguments
+     `(#:tests? #f ; no tests exist
+       #:phases
+        (modify-phases %standard-phases
+          (delete 'configure)
+          (add-after 'unpack 'patch-paths
+            (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let ((avr-gcc (assoc-ref inputs "avr-gcc-5"))
+                   (avr-binutils (assoc-ref inputs "avr-binutils")))
+              (substitute* "bin/ard-reset-arduino"
+                (("#!/usr/bin/env python") "#!/usr/bin/python3"))
+              (substitute* "Arduino.mk"
+                (("#    => ARDUINO_DIR.*")
+                   (string-append "ARDUINO_DIR = "
+                                  (assoc-ref %build-inputs "arduino-libraries")
+                                  "/share/arduino\n"))
+                ; ; defaults to "hardware/tools/avr"
+                (("#    => AVR_TOOLS_DIR.*")
+                   (string-append "AVR_TOOLS_DIR = /\n"
+                                  "AVR_TOOLS_PATH = /\n"
+                                  "AVRDUDE = " (assoc-ref %build-inputs "avrdude") "\n"))
+                (("CC_NAME[ ]*=.*")
+                   (string-append "CC_NAME = " avr-gcc "/bin/avr-gcc\n"))
+                (("CXX_NAME[ ]*=.*")
+                   (string-append "CXX_NAME = " avr-gcc "/bin/avr-g++\n"))
+                (("OBJCOPY_NAME[ ]*=.*")
+                   (string-append "OBJCOPY_NAME = " avr-binutils "/bin/avr-objcopy\n"))
+                (("OBJDUMP_NAME[ ]*=.*")
+                   (string-append "OBJDUMP_NAME = " avr-binutils "/bin/avr-objdump\n"))
+                (("AR_NAME[ ]*=.*")
+                   (string-append "AR_NAME = " avr-binutils "/bin/avr-ar\n"))
+                (("SIZE_NAME[ ]*=.*")
+                   (string-append "SIZE_NAME = " avr-binutils "/bin/avr-size\n"))
+                (("NM_NAME[ ]*=.*")
+                   (string-append "NM_NAME = " avr-binutils "/bin/avr-nm\n"))
+                ; What about ld ?
+                (("#    => ARDMK_DIR.*")
+                   (string-append "ARDMK_DIR = "
+                                  (assoc-ref %outputs "out")
+                                  "/share/arduino\n"))))))
+          (delete 'build)
+          (replace 'install
+            (lambda* (#:key outputs #:allow-other-keys)
+              (let* ((out (assoc-ref outputs "out"))
+                     (out-mk (string-append out "/share/arduino"))
+                     (out-doc (string-append out "/share/doc"))
+                     (out-bin (string-append out "/bin"))
+                     (out-man (string-append out "/share/man/man1")))
+                    (for-each (lambda (name)
+                                (install-file name out-mk))
+                              '("Arduino.mk" "arduino-mk-vars.md" "chipKIT.mk" "Common.mk"))
+                    (mkdir-p out-doc)
+                    (copy-recursively "examples" out-doc)
+                    (install-file "bin/ard-reset-arduino" out-bin)
+                    (install-file "ard-reset-arduino.1" out-man)))))))
+    (inputs
+     `(("python" ,python)
+       ("python-pyserial" ,python-pyserial)
+       ("arduino-libraries" ,arduino-libraries)
+       ("avrdude" ,avrdude)
+       ("avr-gcc-5" ,avr-gcc-5)
+       ("avr-binutils" ,avr-binutils)))
+    (synopsis "Arduino Makefile Include file")
+    (description "@code{arduino-makefile} allows you to build Arduino sketches using a very tiny Makefile.")
+    (home-page "https://github.com/sudar/Arduino-Makefile")
+    ;(supported-systems '("avr"))
+    (license license:lgpl2.1)))

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

[Ricardo Wurmus]

Hi Danny,

Thank you for your contributions!  I haven’t followed the discussion on
the previous versions, so I hope my comments below are not annoying.

> +(define-public arduino-makefile
> +  (package
> +    (name "arduino-makefile")
> +    (version "1.5.1")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/sudar/Arduino-Makefile/"
> +                                  "archive/" version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "1gqmcg2jg62b915akbkivnqf8sx76gv719vx7azm47szd0w1i94i"))
> +              (file-name (string-append name "-" version ".tar.gz"))))
> +    (build-system python-build-system)
> +    (arguments
> +     `(#:tests? #f ; no tests exist
> +       #:phases
> +        (modify-phases %standard-phases
> +          (delete 'configure)
> +          (add-after 'unpack 'patch-paths
> +            (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (let ((avr-gcc (assoc-ref inputs "avr-gcc-5"))
> +                   (avr-binutils (assoc-ref inputs "avr-binutils")))
> +              (substitute* "bin/ard-reset-arduino"
> +                (("#!/usr/bin/env python") "#!/usr/bin/python3"))

This looks unnecessary.  When “python-wrapper” is among the inputs the
shebang would be replaced automatically.

> +              (substitute* "Arduino.mk"
> +                (("#    => ARDUINO_DIR.*")
> +                   (string-append "ARDUINO_DIR = "
> +                                  (assoc-ref %build-inputs "arduino-libraries")

Could you use “inputs” instead of “%build-inputs” here?

> +                                  "/share/arduino\n"))
> +                ; ; defaults to "hardware/tools/avr"

; ; –> ;;

> +                (("#    => AVR_TOOLS_DIR.*")
> +                   (string-append "AVR_TOOLS_DIR = /\n"
> +                                  "AVR_TOOLS_PATH = /\n"
> +                                  "AVRDUDE = " (assoc-ref %build-inputs "avrdude") "\n"))
> +                (("CC_NAME[ ]*=.*")
> +                   (string-append "CC_NAME = " avr-gcc "/bin/avr-gcc\n"))
> +                (("CXX_NAME[ ]*=.*")
> +                   (string-append "CXX_NAME = " avr-gcc "/bin/avr-g++\n"))
> +                (("OBJCOPY_NAME[ ]*=.*")
> +                   (string-append "OBJCOPY_NAME = " avr-binutils "/bin/avr-objcopy\n"))
> +                (("OBJDUMP_NAME[ ]*=.*")
> +                   (string-append "OBJDUMP_NAME = " avr-binutils "/bin/avr-objdump\n"))
> +                (("AR_NAME[ ]*=.*")
> +                   (string-append "AR_NAME = " avr-binutils "/bin/avr-ar\n"))
> +                (("SIZE_NAME[ ]*=.*")
> +                   (string-append "SIZE_NAME = " avr-binutils "/bin/avr-size\n"))
> +                (("NM_NAME[ ]*=.*")
> +                   (string-append "NM_NAME = " avr-binutils "/bin/avr-nm\n"))
> +                ; What about ld ?

What about it?  :)

> +                (("#    => ARDMK_DIR.*")
> +                   (string-append "ARDMK_DIR = "
> +                                  (assoc-ref %outputs "out")
> +                                  "/share/arduino\n"))))))

The indentation of this whole block looks a bit off.  Don’t worry about
it, though.  We can fix this before pushing the patch.

> +          (delete 'build)
> +          (replace 'install
> +            (lambda* (#:key outputs #:allow-other-keys)
> +              (let* ((out (assoc-ref outputs "out"))
> +                     (out-mk (string-append out "/share/arduino"))
> +                     (out-doc (string-append out "/share/doc"))
> +                     (out-bin (string-append out "/bin"))
> +                     (out-man (string-append out "/share/man/man1")))
> +                    (for-each (lambda (name)
> +                                (install-file name out-mk))
> +                              '("Arduino.mk" "arduino-mk-vars.md" "chipKIT.mk" "Common.mk"))
> +                    (mkdir-p out-doc)
> +                    (copy-recursively "examples" out-doc)
> +                    (install-file "bin/ard-reset-arduino" out-bin)
> +                    (install-file "ard-reset-arduino.1" out-man)))))))

The phase should end with #t because “install-file” has no useful return value.

> +    (inputs
> +     `(("python" ,python)
> +       ("python-pyserial" ,python-pyserial)
> +       ("arduino-libraries" ,arduino-libraries)
> +       ("avrdude" ,avrdude)
> +       ("avr-gcc-5" ,avr-gcc-5)
> +       ("avr-binutils" ,avr-binutils)))

I’m a little confused about this.  Why is avrdude among the inputs?  Why
python-pyserial?  Nothing is built here.  You just copy the files.

If “bin/ard-reset-arduino” or any other script uses pyserial you will
probably have to wrap the executables such that they set the PYTHONPATH
to include the output of python-pyserial.  Otherwise they won’t be able
to find anything.

> +    (synopsis "Arduino Makefile Include file")

Please use lower case.

> +    (description "@code{arduino-makefile} allows you to build Arduino sketches using a very tiny Makefile.")

Please break this line.  Is this really just a Makefile?  What else is
there?

> +    (home-page "https://github.com/sudar/Arduino-Makefile")
> +    ;(supported-systems '("avr"))

This should be removed.

> +    (license license:lgpl2.1)))

Only version 2.1, or does it have the “or later” clause?

~~ Ricardo

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

[Danny Milosavljevic]

Hi Ricardo,

On Wed, 31 Aug 2016 22:02:55 +0200
Ricardo Wurmus <rekado@elephly.net> wrote:
> > +             (let ((avr-gcc (assoc-ref inputs "avr-gcc-5"))
> > +                   (avr-binutils (assoc-ref inputs "avr-binutils")))
> > +              (substitute* "bin/ard-reset-arduino"
> > +                (("#!/usr/bin/env python") "#!/usr/bin/python3"))  
> 
> This looks unnecessary.  When “python-wrapper” is among the inputs the
> shebang would be replaced automatically.

It's impossible for anyone to reliably detect the major Python version that
the script expects.

Therefore, this clarifies that we need Python3 for it.
The shebang will then be automatically updated correctly.

> > +              (substitute* "Arduino.mk"
> > +                (("#    => ARDUINO_DIR.*")
> > +                   (string-append "ARDUINO_DIR = "
> > +                                  (assoc-ref %build-inputs "arduino-libraries")  
> 
> Could you use “inputs” instead of “%build-inputs” here?

Yes. What's the difference?

> > +                ; What about ld ?  
> 
> What about it?  :)

I don't know. Seems to work without it but it just irks me.
There's no variable for ld's name in the makefile.

> I’m a little confused about this.  Why is avrdude among the inputs?  Why
> python-pyserial?  Nothing is built here.  You just copy the files.

It's supposed to be something like an arduino-toolchain. You just install it
and it will pull the compiler, flasher etc and use it for your projects
automagically as long as they "include Arduino.mk".

> If “bin/ard-reset-arduino” or any other script uses pyserial you will
> probably have to wrap the executables such that they set the PYTHONPATH
> to include the output of python-pyserial.  Otherwise they won’t be able
> to find anything.

Probably.

> Please break this line.  Is this really just a Makefile?  What else is
> there?

Yeah, huge makefile. Allows you to build and flash the stuff.

> > +    ;(supported-systems '("avr"))  
> 
> This should be removed.

As I said I'm not a fan of obfuscating what system this is for.
It is *good* for maintenance to have little hints of what to expect.

If it worked usefully, I'd even make the (supported-system '("avr")) active!

> > +    (license license:lgpl2.1)))  
> 
> Only version 2.1, or does it have the “or later” clause?

lgpl2.1+

Thanks,
    Danny

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

[Ricardo Wurmus]

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Ricardo,
>
> On Wed, 31 Aug 2016 22:02:55 +0200
> Ricardo Wurmus <rekado@elephly.net> wrote:
>> > +             (let ((avr-gcc (assoc-ref inputs "avr-gcc-5"))
>> > +                   (avr-binutils (assoc-ref inputs "avr-binutils")))
>> > +              (substitute* "bin/ard-reset-arduino"
>> > +                (("#!/usr/bin/env python") "#!/usr/bin/python3"))  
>> 
>> This looks unnecessary.  When “python-wrapper” is among the inputs the
>> shebang would be replaced automatically.
>
> It's impossible for anyone to reliably detect the major Python version that
> the script expects.
>
> Therefore, this clarifies that we need Python3 for it.
> The shebang will then be automatically updated correctly.

I don’t understand this.  When the “python-wrapper” package is an input
it will be the only package providing a “python” executable, so the
shebang patcher will replace “#!/usr/bin/env python” with
“#!/gnu/store/…-python-3…/bin/python”.  There’s nothing ambiguous about
this.

>> > +              (substitute* "Arduino.mk"
>> > +                (("#    => ARDUINO_DIR.*")
>> > +                   (string-append "ARDUINO_DIR = "
>> > +                                  (assoc-ref %build-inputs "arduino-libraries")  
>> 
>> Could you use “inputs” instead of “%build-inputs” here?
>
> Yes. What's the difference?

We prefer to use a properly bound “inputs” over the magical
“%build-inputs”.  “%build-inputs” can be used in the specification of
#:make-flags where “inputs” cannot be bound.

>> > +                ; What about ld ?  
>> 
>> What about it?  :)
>
> I don't know. Seems to work without it but it just irks me.
> There's no variable for ld's name in the makefile.
>
>> I’m a little confused about this.  Why is avrdude among the inputs?  Why
>> python-pyserial?  Nothing is built here.  You just copy the files.
>
> It's supposed to be something like an arduino-toolchain. You just install it
> and it will pull the compiler, flasher etc and use it for your projects
> automagically as long as they "include Arduino.mk".

Then shouldn’t the inputs be propagated?  As the build artifacts of this
package retain no reference to these regular inputs it’s almost the same
as not specifying them in the first place.

>> > +    ;(supported-systems '("avr"))  
>> 
>> This should be removed.
>
> As I said I'm not a fan of obfuscating what system this is for.
> It is *good* for maintenance to have little hints of what to expect.
>
> If it worked usefully, I'd even make the (supported-system '("avr")) active!

This is not what “supported-systems” is used for.  This field is used to
tell the build machines in the build farm not to bother building
packages that are not supported on the current architecture.

The software provided by this package does not “run” on AVRs.  (It
doesn’t run at all.)  It is used on any host system to build software
for AVRs.  That’s very different from what the “supported-systems” field
describes.

~~ Ricardo

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

[Danny Milosavljevic]

Hi Ricardo,

On Thu, 01 Sep 2016 16:57:32 +0200
Ricardo Wurmus <rekado@elephly.net> wrote:

>> > +              (substitute* "bin/ard-reset-arduino"
>> > +                (("#!/usr/bin/env python") "#!/usr/bin/python3"))    
>> 
> >> This looks unnecessary.  When “python-wrapper” is among the inputs the
> >> shebang would be replaced automatically.  

... with Python 3's name. So okay, I'll remove this part. (I still don't get what Guix does (not do) with major versions - but fine)

ard-reset-arduino source code should have specified which Python version it expects in the shebang to begin with, I was just bugfixing it... 

> “#!/gnu/store/…-python-3…/bin/python”.  There’s nothing ambiguous about
                         ^ yeah, I've checked out python-wrapper now and as long as it does that now and in the future, okay :)

> We prefer to use a properly bound “inputs” over the magical
> “%build-inputs”.  “%build-inputs” can be used in the specification of
> #:make-flags where “inputs” cannot be bound.

Ok.

> Then shouldn’t the inputs be propagated?  As the build artifacts of this
> package retain no reference to these regular inputs it’s almost the same
> as not specifying them in the first place.

I don't know about this. I thought propagated meant that if I installed arduino-makefile into the profile it would also install avrdude into the profile. That is not what I wanted. The Makefile refers to the /gnu/store/....avrdude... - that's it. But we can also propagate - I'm just not sure whether that's needed and useful.

> This is not what “supported-systems” is used for.  This field is used to
> tell the build machines in the build farm not to bother building
> packages that are not supported on the current architecture.

Ah okay. I'll remove it then.

Cheers,
   Danny

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

[Danny Milosavljevic]

On Thu, 1 Sep 2016 20:44:20 +0200
Danny Milosavljevic <dannym@scratchpost.org> wrote:

> I don't know about this. I thought propagated meant that if I installed arduino-makefile into the profile it would also install avrdude into the profile. That is not what I wanted. The Makefile refers to the /gnu/store/....avrdude... - that's it. But we can also propagate - I'm just not sure whether that's needed and useful.

On second thought, gcc-toolchain propagates ld etc too. So if you want, we can propagate avr-gcc-5, avr-binutils, arduino-libraries and avrdude.

Propagating Python would probably be technically necessary but actually not desireable because Arduino has very little to do with Python.

So I'm fine with it either way.