Always set the versioned runtime.tool property for all installed tools

[cmaglie]

For example if we have bossac 1.7.1 and 1.9.0 installed, this will result in the following properties available during upload:

{runtime.tools.bossac-1.7.0-arduino3.path} => /path/to/bossac/1.7.0
{runtime.tools.bossac-1.9.0-arduino3.path} => /path/to/bossac/1.9.0
{runtime.tools.bossac.path} => /path/to/bossac/1.9.0

some platforms fail to correctly specify the version of the tool in the package_index.json but they do the correct specification in the recipes. This patch allows to not fail in this latter case.

[matthijskooijman]

Sounds like a good change, since IIUC this might also help with cores installed into ~/Arduino/hardware/ that do not have a json file to specify their tool versions?

I do wonder: How does this interact with different vendors supplying a tool by the same name? AFAICS you can specify the tool vendor explicitly in the JSON, but how is this handled for GetAllInstalledToolsReleases()?

[cmaglie]

Sounds like a good change, since IIUC this might also help with cores installed into ~/Arduino/hardware/ that do not have a json file to specify their tool versions?

yes! that's another use case covered by this change

I do wonder: How does this interact with different vendors supplying a tool by the same name? AFAICS you can specify the tool vendor explicitly in the JSON, but how is this handled for GetAllInstalledToolsReleases()?

It's handled by FindToolsRequiredForBoard(board) in the loop that follows. The first loop with GetAllInstalledToolsReleases() is needed to create the full mapping withruntime.tools.TOOLNAME-VERSION.path.

[matthijskooijman]

It's handled by FindToolsRequiredForBoard(board) in the loop that follows. The first loop with GetAllInstalledToolsReleases() is needed to create the full mapping withruntime.tools.TOOLNAME-VERSION.path.

Yeah, but I mean what if say, there is an openocd version 1.0 tool by both the arduino and stm32 vendor? What would runtime.tools.openocd-1.0.path point to?

[cmaglie]

ah good point... as it is now the result is undefined. The specification says:

A {runtime.tools.TOOL_NAME.path} and {runtime.tools.TOOL_NAME-TOOL_VERSION.path} property is generated for the tools of Arduino AVR Boards and any other platform installed via Boards Manager. {runtime.tools.TOOL_NAME.path} points to the latest version of the tool available.

in theory, if you write the required tools in the platform_index, and you do things correctly you will never encounter the case above.

The problem may be present in the development phase where the package_index may still not be defined. Maybe adding a recipe like {runtime.tools.TOOL_PACKAGER.TOOL_NAME-TOOL_VERSION.path} may turn out useful? BTW this is going to break compatibility with previous release of the cli/IDE if used in a new platform release.

[matthijskooijman]

The problem may be present in the development phase where the package_index may still not be defined. Maybe adding a recipe like {runtime.tools.TOOL_PACKAGER.TOOL_NAME-TOOL_VERSION.path} may turn out useful? BTW this is going to break compatibility with previous release of the cli/IDE if used in a new platform release.

That sounds reasonable to me, yeah. AFAICS that allows fully and uniquely identifying a tool without ambiguity. Also, I wonder if this should be done instead of the versioned variables without the packager as added by the PR in its current state?

OTOH, I guess the ambiguity is already present in the current code, right? Am I understanding correctly that:

• Without this PR applied, runtime.tools.TOOL_NAME-VERSION.path is already available for the latest version of each tool?
• With this PR applied, runtime.tools.TOOL_NAME-VERSION.path is available for every version of each tool?

Because of the possible ambiguity, I think we should document that runtime.tools.TOOL_NAME-VERSION.path is deprecated (or at least not recommended) and platforms should prefer to use the (to-be-introduced) runtime.tools.TOOL_PACKAGER.TOOL_NAME-VERSION.path instead (unless they want to remain backward compatible for a while). If we do, I wonder if it makes sense to add more runtime.tools.TOOL_NAME-VERSION.path (like this PR does), or if we should skip that and just do that only for runtime.tools.TOOL_PACKAGER.TOOL_NAME-VERSION.path?

Also, I wonder about the runtime.tools.TOOL_NAME.path variable. Documentation says:

{runtime.tools.TOOL_NAME.path} points to the latest version of the tool available.

But is that really true? Doesn't this point to the version (and packager) of the tool as specified in the toolDependencies JSON, only falling back to the "latest version" for tools that are not specified as dependencies?

Related: Should there be also a runtime.tools.TOOL_PACKAGER.TOOL_NAME.path that points to the latest version of a given tool by a given packager, to remove the same ambiguity here?

[cmaglie]

Also, I wonder if this should be done instead of the versioned variables without the packager as added by the PR in its current state?

This PR allows solving, basically, 99.99% of the current user-facing issues and I would merge it anyway, even if it doesn't cover all the edge cases it helps to avoid the weird situation where you have a tool installed but it's not made available in the runtime variables.

{runtime.tools.TOOL_NAME.path} points to the latest version of the tool available.

But is that really true? Doesn't this point to the version (and packager) of the tool as specified in the toolDependencies JSON, only falling back to the "latest version" for tools that are not specified as dependencies?

yes the documentation is imprecise, we should fix it

[aentinger]

Tested - works 🚀

[matthijskooijman]

This PR allows solving, basically, 99.99% of the current user-facing issues and I would merge it anyway, even if it doesn't cover all the edge cases it helps to avoid the weird situation where you have a tool installed but it's not made available in the runtime variables.

Sounds fine to me.