Hooks are recognized even when pattern property name uses string key in place of numeric index

[ArminJo]

Describe the problem

Here https://arduino.github.io/arduino-cli/0.34/platform-specification/#pre-and-post-build-hooks-since-arduino-ide-165
the hook recipe.hooks.core.prebuild.set_core_build_flag is not documented.
But it is used By ESP32 platform.txt line 195 ff.

# Set -DARDUINO_CORE_BUILD only on core file compilation
file_opts.path={build.path}/file_opts
recipe.hooks.prebuild.set_core_build_flag.pattern=bash -c ": > {file_opts.path}"
recipe.hooks.core.prebuild.set_core_build_flag.pattern=bash -c "echo -DARDUINO_CORE_BUILD > {file_opts.path}"
recipe.hooks.core.postbuild.set_core_build_flag.pattern=bash -c ": > {file_opts.path}"

recipe.hooks.prebuild.set_core_build_flag.pattern.windows=cmd /c type nul > "{file_opts.path}"
recipe.hooks.core.prebuild.set_core_build_flag.pattern.windows=cmd /c echo "-DARDUINO_CORE_BUILD" > "{file_opts.path}"
recipe.hooks.core.postbuild.set_core_build_flag.pattern.windows=cmd /c type nul > "{file_opts.path}"

see also Sloeber/arduino-eclipse-plugin#1582

To reproduce

Compile any sketch with any ESP32 board:

Creating esp32 image...
Merged 1 ELF section
Successfully created esp32 image.
cmd /c if exist "C:\\Users\\...\\Temp\\.arduinoIDE-unsaved2023915-8796-1rs03z4.hnwd\\SimpleReceiver\\build_opt.h" COPY /y "C:\\Users\\...\\Temp\\.arduinoIDE-unsaved2023915-8796-1rs03z4.hnwd\\SimpleReceiver\\build_opt.h" "C:\\Users\\...\\Temp\\arduino\\sketches\\10BDBB355C916CDD0AC5166109C1A196\\build_opt.h"
cmd /c if not exist "C:\\Users\\...\\Temp\\arduino\\sketches\\10BDBB355C916CDD0AC5166109C1A196\\build_opt.h" type nul > "C:\\Users\\...\\Temp\\arduino\\sketches\\10BDBB355C916CDD0AC5166109C1A196\\build_opt.h"
cmd /c type nul > "C:\\Users\\Armin\\AppData\\Local\\Temp\\arduino\\sketches\\10BDBB355C916CDD0AC5166109C1A196/file_opts"

see last line, where the recipe is executed.

Expected behavior

Please document it, or disable it if it is not officially supported.
Thanks

Arduino CLI version

2.2.0

Operating system

Windows

Operating system version

10

Additional context

Please document it, or disable it, if it is not officially supported.
Thanks

Issue checklist

• I searched for previous reports in the issue tracker
• I verified the problem still occurs when using the nightly build
• My report contains all necessary details

[per1234]

Hi @ArminJo. Thanks for your report. The specification is as intended. The ESP32 platform developers are unnecessarily abusing an incidental behavior where strings happen to be supported in addition to integers as the index in the build hook pattern properties:

arduino-cli/arduino/builder/builder.go

Line 401 in ef72bde

if err := b.RunRecipe("recipe.hooks.core.prebuild", ".pattern", false); err != nil {

arduino-cli/arduino/builder/recipe.go

Line 34 in ef72bde

recipes := findRecipes(buildProperties, prefix, suffix)

arduino-cli/arduino/builder/recipe.go

Line 64 in ef72bde

if strings.HasPrefix(key, patternPrefix) && strings.HasSuffix(key, patternSuffix) && buildProperties.Get(key) != "" {

So this should be considered a bug in the ESP32 platform and we will not provide explicit support for this format of build hook pattern properties.

I think your proposal that the Arduino CLI codebase be changed to drop support for this non-compliant usage is reasonable since otherwise we end up risking being forced to officially support this jankiness after it spreads to many other platforms due to cursory copy/pasting by platform authors who don't make the effort to understand what they are copying.

The github.com/arduino/go-properties-orderedmap module used to parse platform.txt provides specific handling of the list format used by the hook pattern properties:

https://pkg.go.dev/github.com/arduino/go-properties-orderedmap#Map.ExtractSubIndexSets

Numeric subindex cannot be mixed with non-numeric, in that case only the numeric sub index sets will be returned.

[ArminJo]

Thank you very much for this analysis! 🥇
It seems that the ESP 3.x version does not use this recipe any more 😀 .