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
