Refined the definition of `build_cache.path` and the `--build-path` behaviour #2673

cmaglie · 2024-07-25T13:27:23Z

Please check if the PR fulfills these requirements

See how to contribute

The PR has no duplicates (please search among the Pull Requests
before creating one)
The PR follows
our contributing guidelines
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)
UPGRADING.md has been updated with a migration guide (for breaking changes)
configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

Based on feedback from build_cache.path not honored for "sketches" #2668, this PR refines the definition of build_cache.path (and --build-cache-path).
It also changes the builder's behavior when --build-path is set by the user: in this case, the build cache is no longer used and the build is based only on the object files found in the build path

UPDATES
After a bit more discussion in this PR, we agreed to also:

Deprecate the --build-cache-path parameter from the compile command (it's now hidden and now useless, but we'll keep it working for backward compatibility)
We added the --build-path flag to upload --build-path and debug --build-path commands to pair with the compile --build-path command.
Move the default build-cache dir from the Temp folder $TMP/arduino to the User cache folder (as defined by each OS, for example on Linux appears to be $HOME/.cache/arduino).

What is the current behavior?

Previously the build_cache.path setting (and consequently the --build-cache-path flag) specified the path of the platforms' cores cache (it will be in {build_cache.path}/cores). The default {build_cache.path} is /tmp/arduino, so the default core cache is in /tmp/arduino/cores. Instead, the temporary sketch cache was always in the default /tmp/arduino/sketches regardless of the setting.
Previously, if a user specify a --build-path, an already-built core found in the core cache path could be used to speed up the build.

Let's see some examples to clarify:

~/Arduino/Blink$ rm -r /tmp/arduino ../cache
~/Arduino/Blink$ arduino-cli compile -b arduino:mbed_nano:nanorp2040connect \
                                     --build-cache-path ../cache
~/Arduino/Blink$ tree /tmp/arduino/ ../cache/ .
/tmp/arduino/
└── sketches
    └── 002050EAA7EFB9A4FC451CDFBC0FA2D3
        ├── Blink.ino.bin
        ├── Blink.ino.elf
        ├── Blink.ino.hex
        ├── Blink.ino.map
        ├── Blink.ino.uf2
        ├── build.options.json
        ├── compile_commands.json
        ├── core
        │   ├── abi.cpp.d
        │   ├── abi.cpp.o
        │   ├── api
        │   │   ├── CanMsg.cpp.d
        │   │   ├── CanMsg.cpp.o
        │   │   ├── CanMsgRingbuffer.cpp.d
        │   │   ├── CanMsgRingbuffer.cpp.o
        │   │   ├── Common.cpp.d
        │   │   ├── Common.cpp.o
        │   │   ├── IPAddress.cpp.d
        │   │   ├── IPAddress.cpp.o
        │   │   ├── PluggableUSB.cpp.d
        │   │   ├── PluggableUSB.cpp.o
        │   │   ├── Print.cpp.d
        │   │   ├── Print.cpp.o
        │   │   ├── Stream.cpp.d
        │   │   ├── Stream.cpp.o
        │   │   ├── String.cpp.d
        │   │   └── String.cpp.o
        │   ├── arm_hal_random.c.d
        │   ├── arm_hal_random.c.o
        │   ├── as_mbed_library
        │   │   ├── variant.cpp.d
        │   │   └── variant.cpp.o
        │   ├── core.a
        │   ├── double_tap_usb_boot.cpp.d
        │   ├── double_tap_usb_boot.cpp.o
        │   ├── Interrupts.cpp.d
        │   ├── Interrupts.cpp.o
        │   ├── itoa.c.d
        │   ├── itoa.c.o
        │   ├── main.cpp.d
        │   ├── main.cpp.o
        │   ├── mbed
        │   │   └── platform
        │   │       └── cxxsupport
        │   │           ├── mstd_mutex.cpp.d
        │   │           └── mstd_mutex.cpp.o
        │   ├── nina_pins.cpp.d
        │   ├── nina_pins.cpp.o
        │   ├── pinToIndex.cpp.d
        │   ├── pinToIndex.cpp.o
        │   ├── random_seed.cpp.d
        │   ├── random_seed.cpp.o
        │   ├── Serial.cpp.d
        │   ├── Serial.cpp.o
        │   ├── timer.cpp.d
        │   ├── timer.cpp.o
        │   ├── Tone.cpp.d
        │   ├── Tone.cpp.o
        │   ├── USB
        │   │   ├── PluggableUSBDevice.cpp.d
        │   │   ├── PluggableUSBDevice.cpp.o
        │   │   ├── USBCDC.cpp.d
        │   │   ├── USBCDC.cpp.o
        │   │   ├── USBSerial.cpp.d
        │   │   └── USBSerial.cpp.o
        │   ├── variant.cpp.d
        │   ├── variant.cpp.o
        │   ├── wiring_analog.cpp.d
        │   ├── wiring_analog.cpp.o
        │   ├── wiring.cpp.d
        │   ├── wiring.cpp.o
        │   ├── wiring_digital.cpp.d
        │   ├── wiring_digital.cpp.o
        │   ├── wiring_pulse.cpp.d
        │   ├── wiring_pulse.cpp.o
        │   ├── wiring_shift.cpp.d
        │   ├── wiring_shift.cpp.o
        │   ├── WMath.cpp.d
        │   └── WMath.cpp.o
        ├── includes.cache
        ├── libraries
        ├── libraries.cache
        ├── linker_script.ld
        └── sketch
            ├── Blink.ino.cpp
            ├── Blink.ino.cpp.d
            └── Blink.ino.cpp.o
../cache/
└── cores
    └── arduino_mbed_nano_nanorp2040connect_25dd5c58ea86ff4a3878a625724ee9ec
        └── core.a
.
├── Blink.ino
└── sketch.yaml

16 directories, 81 files

In the example above:

🐛 the sketch has been built in /tmp/arduino/sketches (even if we specified to use a build cache dir in ../cache)
✅ the platform core has been rebuilt (because we purged all the caches)
⚠️ the built core has been cached (copied) in the ../cache dir (not really a bug, but since we asked to use a specific build-path the global cache should be completely ignored)

~/Arduino/Blink$ rm -r /tmp/arduino
~/Arduino/Blink$ arduino-cli compile -b arduino:mbed_nano:nanorp2040connect \
                                     --build-cache-path ../cache \
                                     --build-path build
~/Arduino/Blink$ tree /tmp/arduino/ ../cache/ .
/tmp/arduino/  [error opening dir]
../cache/
└── cores
    └── arduino_mbed_nano_nanorp2040connect_25dd5c58ea86ff4a3878a625724ee9ec
        └── core.a
.
├── Blink.ino
├── build
│   ├── Blink.ino.bin
│   ├── Blink.ino.elf
│   ├── Blink.ino.hex
│   ├── Blink.ino.map
│   ├── Blink.ino.uf2
│   ├── build.options.json
│   ├── compile_commands.json
│   ├── core
│   │   ├── double_tap_usb_boot.cpp.d
│   │   ├── double_tap_usb_boot.cpp.o
│   │   ├── nina_pins.cpp.d
│   │   ├── nina_pins.cpp.o
│   │   ├── variant.cpp.d
│   │   └── variant.cpp.o
│   ├── includes.cache
│   ├── libraries
│   ├── libraries.cache
│   ├── linker_script.ld
│   └── sketch
│       ├── Blink.ino.cpp
│       ├── Blink.ino.cpp.d
│       └── Blink.ino.cpp.o
└── sketch.yaml

8 directories, 22 files

In the example above, after removing /tmp/arduino/, we made a new build by setting both the cache dir and the build dir:

🐛 the ../cache dir has been used to avoid rebuilding the core (since we asked to use a specific build-path the external cache should not be used)
✅ the sketch cache dir has been ignored and the--build-path to ~/Arduino/Blink/build has been used instead
🐛 since the cached core.a has been used the object files for the core are missing in the build-path

~/Arduino/Blink$ rm -r ../cache/
~/Arduino/Blink$ arduino-cli compile -b arduino:mbed_nano:nanorp2040connect \
                                     --build-cache-path ../cache \
                                     --build-path build
~/Arduino/Blink$ tree /tmp/arduino/ ../cache/ .
/tmp/arduino/  [error opening dir]
../cache/
└── cores
    └── arduino_mbed_nano_nanorp2040connect_25dd5c58ea86ff4a3878a625724ee9ec
        └── core.a
.
├── Blink.ino
├── build
│   ├── Blink.ino.bin
│   ├── Blink.ino.elf
│   ├── Blink.ino.hex
│   ├── Blink.ino.map
│   ├── Blink.ino.uf2
│   ├── build.options.json
│   ├── compile_commands.json
│   ├── core
│   │   ├── abi.cpp.d
│   │   ├── abi.cpp.o
│   │   ├── api
│   │   │   ├── CanMsg.cpp.d
│   │   │   ├── CanMsg.cpp.o
│   │   │   ├── CanMsgRingbuffer.cpp.d
│   │   │   ├── CanMsgRingbuffer.cpp.o
│   │   │   ├── Common.cpp.d
│   │   │   ├── Common.cpp.o
│   │   │   ├── IPAddress.cpp.d
│   │   │   ├── IPAddress.cpp.o
│   │   │   ├── PluggableUSB.cpp.d
│   │   │   ├── PluggableUSB.cpp.o
│   │   │   ├── Print.cpp.d
│   │   │   ├── Print.cpp.o
│   │   │   ├── Stream.cpp.d
│   │   │   ├── Stream.cpp.o
│   │   │   ├── String.cpp.d
│   │   │   └── String.cpp.o
│   │   ├── arm_hal_random.c.d
│   │   ├── arm_hal_random.c.o
│   │   ├── as_mbed_library
│   │   │   ├── variant.cpp.d
│   │   │   └── variant.cpp.o
│   │   ├── core.a
│   │   ├── double_tap_usb_boot.cpp.d
│   │   ├── double_tap_usb_boot.cpp.o
│   │   ├── Interrupts.cpp.d
│   │   ├── Interrupts.cpp.o
│   │   ├── itoa.c.d
│   │   ├── itoa.c.o
│   │   ├── main.cpp.d
│   │   ├── main.cpp.o
│   │   ├── mbed
│   │   │   └── platform
│   │   │       └── cxxsupport
│   │   │           ├── mstd_mutex.cpp.d
│   │   │           └── mstd_mutex.cpp.o
│   │   ├── nina_pins.cpp.d
│   │   ├── nina_pins.cpp.o
│   │   ├── pinToIndex.cpp.d
│   │   ├── pinToIndex.cpp.o
│   │   ├── random_seed.cpp.d
│   │   ├── random_seed.cpp.o
│   │   ├── Serial.cpp.d
│   │   ├── Serial.cpp.o
│   │   ├── timer.cpp.d
│   │   ├── timer.cpp.o
│   │   ├── Tone.cpp.d
│   │   ├── Tone.cpp.o
│   │   ├── USB
│   │   │   ├── PluggableUSBDevice.cpp.d
│   │   │   ├── PluggableUSBDevice.cpp.o
│   │   │   ├── USBCDC.cpp.d
│   │   │   ├── USBCDC.cpp.o
│   │   │   ├── USBSerial.cpp.d
│   │   │   └── USBSerial.cpp.o
│   │   ├── variant.cpp.d
│   │   ├── variant.cpp.o
│   │   ├── wiring_analog.cpp.d
│   │   ├── wiring_analog.cpp.o
│   │   ├── wiring.cpp.d
│   │   ├── wiring.cpp.o
│   │   ├── wiring_digital.cpp.d
│   │   ├── wiring_digital.cpp.o
│   │   ├── wiring_pulse.cpp.d
│   │   ├── wiring_pulse.cpp.o
│   │   ├── wiring_shift.cpp.d
│   │   ├── wiring_shift.cpp.o
│   │   ├── WMath.cpp.d
│   │   └── WMath.cpp.o
│   ├── includes.cache
│   ├── libraries
│   ├── libraries.cache
│   ├── linker_script.ld
│   └── sketch
│       ├── Blink.ino.cpp
│       ├── Blink.ino.cpp.d
│       └── Blink.ino.cpp.o
└── sketch.yaml

14 directories, 81 files

Finally, in the example above, after wiping the custom ../cache dir:

✅ the sketch has been fully rebuilt (so the object files related to the core are in the build path)
⚠️ the built core.a has been cached (copied) in the custom cache dir ../cache/cores (instead of ignoring the cache)

What is the new behavior?

The build_cache.path (and --build-cache-path) is now used to set the cache dir for both the cores and sketches.
Setting the --build-path will now disable all caches, this means that the platform core is rebuilt in the build path (and possibly reused for the next build if the --build-path remains the same).

Let's see the same examples as before:

~/Arduino/Blink$ rm -rf ../cache build/ /tmp/arduino/
~/Arduino/Blink$ arduino-cli compile -b arduino:mbed_nano:nanorp2040connect --build-cache-path ../cache
~/Arduino/Blink$ tree /tmp/arduino/ ../cache/ .
/tmp/arduino/  [error opening dir]
../cache/
├── cores
│   └── arduino_mbed_nano_nanorp2040connect_25dd5c58ea86ff4a3878a625724ee9ec
│       └── core.a
└── sketches
    └── 002050EAA7EFB9A4FC451CDFBC0FA2D3
        ├── Blink.ino.bin
        ├── Blink.ino.elf
        ├── Blink.ino.hex
        ├── Blink.ino.map
        ├── Blink.ino.uf2
        ├── build.options.json
        ├── compile_commands.json
        ├── core
        │   ├── abi.cpp.d
        │   ├── abi.cpp.o
        │   ├── api
        │   │   ├── CanMsg.cpp.d
        │   │   ├── CanMsg.cpp.o
        │   │   ├── CanMsgRingbuffer.cpp.d
        │   │   ├── CanMsgRingbuffer.cpp.o
        │   │   ├── Common.cpp.d
        │   │   ├── Common.cpp.o
        │   │   ├── IPAddress.cpp.d
        │   │   ├── IPAddress.cpp.o
        │   │   ├── PluggableUSB.cpp.d
        │   │   ├── PluggableUSB.cpp.o
        │   │   ├── Print.cpp.d
        │   │   ├── Print.cpp.o
        │   │   ├── Stream.cpp.d
        │   │   ├── Stream.cpp.o
        │   │   ├── String.cpp.d
        │   │   └── String.cpp.o
        │   ├── arm_hal_random.c.d
        │   ├── arm_hal_random.c.o
        │   ├── as_mbed_library
        │   │   ├── variant.cpp.d
        │   │   └── variant.cpp.o
        │   ├── core.a
        │   ├── double_tap_usb_boot.cpp.d
        │   ├── double_tap_usb_boot.cpp.o
        │   ├── Interrupts.cpp.d
        │   ├── Interrupts.cpp.o
        │   ├── itoa.c.d
        │   ├── itoa.c.o
        │   ├── main.cpp.d
        │   ├── main.cpp.o
        │   ├── mbed
        │   │   └── platform
        │   │       └── cxxsupport
        │   │           ├── mstd_mutex.cpp.d
        │   │           └── mstd_mutex.cpp.o
        │   ├── nina_pins.cpp.d
        │   ├── nina_pins.cpp.o
        │   ├── pinToIndex.cpp.d
        │   ├── pinToIndex.cpp.o
        │   ├── random_seed.cpp.d
        │   ├── random_seed.cpp.o
        │   ├── Serial.cpp.d
        │   ├── Serial.cpp.o
        │   ├── timer.cpp.d
        │   ├── timer.cpp.o
        │   ├── Tone.cpp.d
        │   ├── Tone.cpp.o
        │   ├── USB
        │   │   ├── PluggableUSBDevice.cpp.d
        │   │   ├── PluggableUSBDevice.cpp.o
        │   │   ├── USBCDC.cpp.d
        │   │   ├── USBCDC.cpp.o
        │   │   ├── USBSerial.cpp.d
        │   │   └── USBSerial.cpp.o
        │   ├── variant.cpp.d
        │   ├── variant.cpp.o
        │   ├── wiring_analog.cpp.d
        │   ├── wiring_analog.cpp.o
        │   ├── wiring.cpp.d
        │   ├── wiring.cpp.o
        │   ├── wiring_digital.cpp.d
        │   ├── wiring_digital.cpp.o
        │   ├── wiring_pulse.cpp.d
        │   ├── wiring_pulse.cpp.o
        │   ├── wiring_shift.cpp.d
        │   ├── wiring_shift.cpp.o
        │   ├── WMath.cpp.d
        │   └── WMath.cpp.o
        ├── includes.cache
        ├── libraries
        ├── libraries.cache
        ├── linker_script.ld
        └── sketch
            ├── Blink.ino.cpp
            ├── Blink.ino.cpp.d
            └── Blink.ino.cpp.o
.
├── Blink.ino
└── sketch.yaml

15 directories, 81 files

As we can see in this case:

✅ the custom build cache dir ../cache now contains both the cores and the sketches

~/Arduino/Blink$ rm -r ../cache/sketches/     # remove previously build sketch but keep cached cores
~/Arduino/Blink$ arduino-cli compile -b arduino:mbed_nano:nanorp2040connect \
                            --build-cache-path ../cache --build-path build
~/Arduino/Blink$ tree /tmp/arduino/ ../cache/ .
/tmp/arduino/  [error opening dir]
../cache/
└── cores
    └── arduino_mbed_nano_nanorp2040connect_25dd5c58ea86ff4a3878a625724ee9ec
        └── core.a
.
├── Blink.ino
├── build
│   ├── Blink.ino.bin
│   ├── Blink.ino.elf
│   ├── Blink.ino.hex
│   ├── Blink.ino.map
│   ├── Blink.ino.uf2
│   ├── build.options.json
│   ├── compile_commands.json
│   ├── core
│   │   ├── abi.cpp.d
│   │   ├── abi.cpp.o
│   │   ├── api
│   │   │   ├── CanMsg.cpp.d
│   │   │   ├── CanMsg.cpp.o
│   │   │   ├── CanMsgRingbuffer.cpp.d
│   │   │   ├── CanMsgRingbuffer.cpp.o
│   │   │   ├── Common.cpp.d
│   │   │   ├── Common.cpp.o
│   │   │   ├── IPAddress.cpp.d
│   │   │   ├── IPAddress.cpp.o
│   │   │   ├── PluggableUSB.cpp.d
│   │   │   ├── PluggableUSB.cpp.o
│   │   │   ├── Print.cpp.d
│   │   │   ├── Print.cpp.o
│   │   │   ├── Stream.cpp.d
│   │   │   ├── Stream.cpp.o
│   │   │   ├── String.cpp.d
│   │   │   └── String.cpp.o
│   │   ├── arm_hal_random.c.d
│   │   ├── arm_hal_random.c.o
│   │   ├── as_mbed_library
│   │   │   ├── variant.cpp.d
│   │   │   └── variant.cpp.o
│   │   ├── core.a
│   │   ├── double_tap_usb_boot.cpp.d
│   │   ├── double_tap_usb_boot.cpp.o
│   │   ├── Interrupts.cpp.d
│   │   ├── Interrupts.cpp.o
│   │   ├── itoa.c.d
│   │   ├── itoa.c.o
│   │   ├── main.cpp.d
│   │   ├── main.cpp.o
│   │   ├── mbed
│   │   │   └── platform
│   │   │       └── cxxsupport
│   │   │           ├── mstd_mutex.cpp.d
│   │   │           └── mstd_mutex.cpp.o
│   │   ├── nina_pins.cpp.d
│   │   ├── nina_pins.cpp.o
│   │   ├── pinToIndex.cpp.d
│   │   ├── pinToIndex.cpp.o
│   │   ├── random_seed.cpp.d
│   │   ├── random_seed.cpp.o
│   │   ├── Serial.cpp.d
│   │   ├── Serial.cpp.o
│   │   ├── timer.cpp.d
│   │   ├── timer.cpp.o
│   │   ├── Tone.cpp.d
│   │   ├── Tone.cpp.o
│   │   ├── USB
│   │   │   ├── PluggableUSBDevice.cpp.d
│   │   │   ├── PluggableUSBDevice.cpp.o
│   │   │   ├── USBCDC.cpp.d
│   │   │   ├── USBCDC.cpp.o
│   │   │   ├── USBSerial.cpp.d
│   │   │   └── USBSerial.cpp.o
│   │   ├── variant.cpp.d
│   │   ├── variant.cpp.o
│   │   ├── wiring_analog.cpp.d
│   │   ├── wiring_analog.cpp.o
│   │   ├── wiring.cpp.d
│   │   ├── wiring.cpp.o
│   │   ├── wiring_digital.cpp.d
│   │   ├── wiring_digital.cpp.o
│   │   ├── wiring_pulse.cpp.d
│   │   ├── wiring_pulse.cpp.o
│   │   ├── wiring_shift.cpp.d
│   │   ├── wiring_shift.cpp.o
│   │   ├── WMath.cpp.d
│   │   └── WMath.cpp.o
│   ├── includes.cache
│   ├── libraries
│   ├── libraries.cache
│   ├── linker_script.ld
│   └── sketch
│       ├── Blink.ino.cpp
│       ├── Blink.ino.cpp.d
│       └── Blink.ino.cpp.o
└── sketch.yaml

14 directories, 81 files

In this case:

✅ The sketch has been fully rebuilt, even if a cached core was present in ../cache

~/Arduino/Blink$ rm -rf ../cache/ /tmp/arduino build/
~/Arduino/Blink$ arduino-cli compile -b arduino:mbed_nano:nanorp2040connect --build-cache-path ../cache --build-path build
~/Arduino/Blink$ tree /tmp/arduino/ ../cache/ .
/tmp/arduino/  [error opening dir]
../cache/  [error opening dir]
.
├── Blink.ino
├── build
│   ├── Blink.ino.bin
│   ├── Blink.ino.elf
│   ├── Blink.ino.hex
│   ├── Blink.ino.map
│   ├── Blink.ino.uf2
│   ├── build.options.json
│   ├── compile_commands.json
│   ├── core
│   │   ├── abi.cpp.d
│   │   ├── abi.cpp.o
│   │   ├── api
│   │   │   ├── CanMsg.cpp.d
│   │   │   ├── CanMsg.cpp.o
│   │   │   ├── CanMsgRingbuffer.cpp.d
│   │   │   ├── CanMsgRingbuffer.cpp.o
│   │   │   ├── Common.cpp.d
│   │   │   ├── Common.cpp.o
│   │   │   ├── IPAddress.cpp.d
│   │   │   ├── IPAddress.cpp.o
│   │   │   ├── PluggableUSB.cpp.d
│   │   │   ├── PluggableUSB.cpp.o
│   │   │   ├── Print.cpp.d
│   │   │   ├── Print.cpp.o
│   │   │   ├── Stream.cpp.d
│   │   │   ├── Stream.cpp.o
│   │   │   ├── String.cpp.d
│   │   │   └── String.cpp.o
│   │   ├── arm_hal_random.c.d
│   │   ├── arm_hal_random.c.o
│   │   ├── as_mbed_library
│   │   │   ├── variant.cpp.d
│   │   │   └── variant.cpp.o
│   │   ├── core.a
│   │   ├── double_tap_usb_boot.cpp.d
│   │   ├── double_tap_usb_boot.cpp.o
│   │   ├── Interrupts.cpp.d
│   │   ├── Interrupts.cpp.o
│   │   ├── itoa.c.d
│   │   ├── itoa.c.o
│   │   ├── main.cpp.d
│   │   ├── main.cpp.o
│   │   ├── mbed
│   │   │   └── platform
│   │   │       └── cxxsupport
│   │   │           ├── mstd_mutex.cpp.d
│   │   │           └── mstd_mutex.cpp.o
│   │   ├── nina_pins.cpp.d
│   │   ├── nina_pins.cpp.o
│   │   ├── pinToIndex.cpp.d
│   │   ├── pinToIndex.cpp.o
│   │   ├── random_seed.cpp.d
│   │   ├── random_seed.cpp.o
│   │   ├── Serial.cpp.d
│   │   ├── Serial.cpp.o
│   │   ├── timer.cpp.d
│   │   ├── timer.cpp.o
│   │   ├── Tone.cpp.d
│   │   ├── Tone.cpp.o
│   │   ├── USB
│   │   │   ├── PluggableUSBDevice.cpp.d
│   │   │   ├── PluggableUSBDevice.cpp.o
│   │   │   ├── USBCDC.cpp.d
│   │   │   ├── USBCDC.cpp.o
│   │   │   ├── USBSerial.cpp.d
│   │   │   └── USBSerial.cpp.o
│   │   ├── variant.cpp.d
│   │   ├── variant.cpp.o
│   │   ├── wiring_analog.cpp.d
│   │   ├── wiring_analog.cpp.o
│   │   ├── wiring.cpp.d
│   │   ├── wiring.cpp.o
│   │   ├── wiring_digital.cpp.d
│   │   ├── wiring_digital.cpp.o
│   │   ├── wiring_pulse.cpp.d
│   │   ├── wiring_pulse.cpp.o
│   │   ├── wiring_shift.cpp.d
│   │   ├── wiring_shift.cpp.o
│   │   ├── WMath.cpp.d
│   │   └── WMath.cpp.o
│   ├── includes.cache
│   ├── libraries
│   ├── libraries.cache
│   ├── linker_script.ld
│   └── sketch
│       ├── Blink.ino.cpp
│       ├── Blink.ino.cpp.d
│       └── Blink.ino.cpp.o
└── sketch.yaml

11 directories, 80 files

This example shows that:
✅ the build cache is completely ignored if a user-defined build path is used, in fact, neither /tmp/arduino/ nor ../cache were created.

Does this PR introduce a breaking change, and is titled accordingly?

I'm unsure if this PR can be considered a breaking change, AFAICS in the worst-case scenario, this PR could cause a slower build time because the core may be rebuilt even if an already built one is in the build cache. On the other hand, the build now relies only on the build path provided by the user, which is a better outcome IMHO.
I may be missing some obscure use cases that this PR breaks, but strictly speaking, it won't break any public API.

Other information

/cc @egnor

cmaglie · 2024-07-30T14:12:08Z

Ok, besides some (no more valid) tests that are failing, one thing that I noticed is that now the commands upload and debug would not work anymore if a user-defined build cache is provided, because those commands lack the option --build-cache-path.
They will always try to find the compiled sketch in the default /tmp/arduino/... build cache, this must be fixed before making this PR ready to merge.

egnor · 2024-07-30T16:45:02Z

Thought exercise: Given this new behavior, would it make sense to eliminate/deprecate setting the build cache path entirely? (Do upload and debug take a --build-path option?)

cmaglie · 2024-07-30T22:31:11Z

(Do upload and debug take a --build-path option?)

Actually, they have it, it's not called --build-path but --input-dir.
upload --input-dir xxxx is meant to be used in conjunction with compile --output-dir xxxx, for example:

~/Arduino/Blink$ arduino-cli compile -b arduino:mbed_nano:nanorp2040connect --output-dir output
~/Arduino/Blink$ tree output/
output/
├── Blink.ino.bin
├── Blink.ino.elf
├── Blink.ino.hex
├── Blink.ino.map
└── Blink.ino.uf2

1 directory, 5 files
~/Arduino/Blink$ arduino-cli upload -b arduino:mbed_nano:nanorp2040connect --input-dir output
...perform the upload...

as you can see the output-dir is a subset of the build-path, it contains only the executable files without the intermediate build.
We may add upload/debug --build-path as an alias of upload/deb --input-dir, so we have 1:1 pairing between compile and upload/debug:

If `compile` with...	...then `upload` or `debug` with
`--build-path DIR`	`--build-path DIR`
`--output-dir DIR`	`--input-dir DIR`

Thought exercise: Given this new behavior, would it make sense to eliminate/deprecate setting the build cache path entirely?

I agree, but I'd rather deprecate only the command line flag --build-cache-path just leaving the setting in the configuration file.

codecov · 2024-08-09T16:42:06Z

Codecov Report

Attention: Patch coverage is 81.48148% with 15 lines in your changes missing coverage. Please review.

Project coverage is 67.68%. Comparing base (23d5036) to head (ec3a9cd).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
commands/service_compile.go	75.00%	5 Missing and 4 partials ⚠️
internal/cli/configuration/configuration.go	55.55%	3 Missing and 1 partial ⚠️
commands/service_debug.go	66.66%	1 Missing ⚠️
commands/service_upload_burnbootloader.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2673   +/-   ##
=======================================
  Coverage   67.67%   67.68%           
=======================================
  Files         234      234           
  Lines       22207    22226   +19     
=======================================
+ Hits        15028    15043   +15     
- Misses       5998     6001    +3     
- Partials     1181     1182    +1

Flag	Coverage Δ
unit	`67.68% <81.48%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

alessio-perugini

LGTM, I've also performed some tests to see if we have some edgy cases when using build-cache-extra-paths, but everything went fine. Plus we already added extensive integration tests in that area so I'm extra confident. 💪

The complete build will be performed in the specified build path. The build artifacts in the build path will be reused for the next build.

Previously it would reject only if ALL the arguments in the given set are used. Now it rejects if AT LEAST TWO arguments of the given set are used.

egnor · 2024-09-18T17:24:48Z

Yay!!!

cmaglie mentioned this pull request Jul 30, 2024

build_cache.path not honored for "sketches" #2668

Closed

3 tasks

cmaglie force-pushed the rework_build_paths branch 5 times, most recently from cf0b370 to b4a05c1 Compare August 9, 2024 15:01

cmaglie marked this pull request as ready for review August 9, 2024 16:41

cmaglie self-assigned this Aug 9, 2024

cmaglie requested review from MatteoPologruto, alessio-perugini and umbynos August 9, 2024 16:41

cmaglie added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Aug 9, 2024

cmaglie linked an issue Aug 9, 2024 that may be closed by this pull request

build_cache.path not honored for "sketches" #2668

Closed

3 tasks

alessio-perugini approved these changes Aug 12, 2024

View reviewed changes

cmaglie added 11 commits September 18, 2024 15:30

If a build path is specified ignore all build caches

291d960

The complete build will be performed in the specified build path. The build artifacts in the build path will be reused for the next build.

Fixed arguments.CheckFlagsConflicts helper

f6e77b6

Previously it would reject only if ALL the arguments in the given set are used. Now it rejects if AT LEAST TWO arguments of the given set are used.

Added --build-path as alias for --input-dir in upload and debug commands

13bca61

Created configuration defaults for build_cache.path setting

98f7608

The build_cache.path setting now affect also the sketches cache

044a2bd

Deprecated --build-cache-path option in compile

556e54c

Use default user's cache dir instead of tmp for build cache

d2ffeb0

Add notes in UPGRADING.md

661c2cc

Updated integration test

2a75aa1

Updated integration test

bc1521d

Updated integration test

d6fc941

cmaglie added 2 commits September 18, 2024 15:31

Updated integration test

3747166

Updated integration test

ec3a9cd

cmaglie force-pushed the rework_build_paths branch from 8e8d7ac to ec3a9cd Compare September 18, 2024 13:33

cmaglie merged commit 863c1ec into arduino:master Sep 18, 2024
104 checks passed

cmaglie deleted the rework_build_paths branch September 18, 2024 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refined the definition of `build_cache.path` and the `--build-path` behaviour #2673

Refined the definition of `build_cache.path` and the `--build-path` behaviour #2673

cmaglie commented Jul 25, 2024 •

edited

Loading

cmaglie commented Jul 30, 2024

egnor commented Jul 30, 2024

cmaglie commented Jul 30, 2024

codecov bot commented Aug 9, 2024 •

edited

Loading

alessio-perugini left a comment

egnor commented Sep 18, 2024

Refined the definition of build_cache.path and the --build-path behaviour #2673

Refined the definition of build_cache.path and the --build-path behaviour #2673

Conversation

cmaglie commented Jul 25, 2024 • edited Loading

Please check if the PR fulfills these requirements

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change, and is titled accordingly?

Other information

cmaglie commented Jul 30, 2024

egnor commented Jul 30, 2024

cmaglie commented Jul 30, 2024

codecov bot commented Aug 9, 2024 • edited Loading

Codecov Report

alessio-perugini left a comment

Choose a reason for hiding this comment

egnor commented Sep 18, 2024

Refined the definition of `build_cache.path` and the `--build-path` behaviour #2673

Refined the definition of `build_cache.path` and the `--build-path` behaviour #2673

cmaglie commented Jul 25, 2024 •

edited

Loading

codecov bot commented Aug 9, 2024 •

edited

Loading