arduino / tooling-rfcs

RFCs related to Arduino tooling projects
Creative Commons Zero v1.0 Universal
5 stars 4 forks source link

Use of defines in reproducible builds #9

Open RobTillaart opened 3 years ago

RobTillaart commented 3 years ago

Often builds have command line defines

As these defines are part of the build they must be includable in the profile. e.g.

profiles:
  nanorp:
    fqbn: arduino:mbed_nano:nanorp2040connect
    platforms:
      - platform: arduino:mbed_nano (2.1.0)
    libraries:
      - ArduinoIoTCloud (1.0.2)
    defines:
      - SIZE: 128

  other:
    fqbn: ...
    platforms:
      - platform: ....
    libraries:
      - ....
    defines:
      - SIZE: 64

If the user defines the define on the command line, it overrules the value in the profile

arduino-cli compile [sketch] --DSIZE=256--profile myProject
cmaglie commented 3 years ago

Yep, this opens another can of worm :-) but we are definitely going to implement this one in a later revision of the profiles.

The main issue here is that there isn't a common pattern in the platforms.txt to add the extra -D properties, so we should probably create a new platofrm.txt "variable" for this specific purpose and wait for various 3rd party cores maintainers to adopt it (BTW they revealed to be pretty quick, so this may get through in a reasonable time).

DeeEmm commented 3 years ago

The main issue here is that there isn't a common pattern in the platforms.txt to add the extra -D properties,

Is this not already defined within the CLI as custom board options as a parameter of the FQBN?

All of these parameters follow typical key->value format so it should be trivial to allow these within the platforms.txt

i.e.

profiles:
  ESPduino:
    fqbn: esp32:esp32:esp32
        FlashMode qio
        UploadSpeed 115200
        PartitionScheme no_ota

With strict YAML conventions, indenting makes these parameters a subset of the fqbn, so we do not need to define additional variables to store them in unless we need to access these parameters publicly.

so we should probably create a new platofrm.txt "variable" for this specific purpose and wait for various 3rd party cores maintainers to adopt it (BTW they revealed to be pretty quick, so this may get through in a reasonable time).

using the above format is transparent to third party maintainers as it follows the current convention of adding parameter values to the fqbn. It also degrades nicely to the current system as using no platform.txt file just falls back to the current way of doing things.

In addition to this, it should be possible to do the following in addition to the above,

profiles:
  ESPduino:
    fqbn: esp32:esp32:esp32:FlashMode=qio:UploadSpeed=115200:PartitionScheme=no_ota

Which is essentially the same as we currently have. It also make an easy interim solution and should be dead simple to implement

egnor commented 4 months ago

This effort appears to be stalled? It's a quite commonly needed feature:

Major use cases include

Workarounds that are commonly offered, and why they're sad

(Notably, I have never met a build environment that doesn't let you set defines on a project level before. Admittedly Arduino is coming from a bit of a different perspective, prioritizing ease of getting started above all else. And people do find workarounds. But still it's pretty silly, feels like it should be as easy as adding an entry to sketch.yaml.)

And yes, this requires some way to add some consistent way for platform/build definitions to handle defines, with a consistent syntax. (https://github.com/arduino/arduino-cli/issues/846 is about attacking this subproblem.) Other proposals have suggested "prepend this include file" instead of "define these symbols", which sidesteps some issues but creates others.