Danny Milosavljevic <dan...@scratchpost.org> writes:
> Hi Ricardo,
>
> On Wed, 31 Aug 2016 22:02:55 +0200
> Ricardo Wurmus <rek...@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
