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

[Danny Milosavljevic]

Hi Ricardo,

On Wed, 31 Aug 2016 22:02:55 +0200
Ricardo Wurmus <address@hidden> 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