This platform uses a build hook to save a disassembler listing to the build folder when compiling sketches.
Due to the use of a redirection operator in the command, it was necessary to execute it via the Windows command interpreter by passing it as an argument to cmd /c.
Although paths with spaces can be used without difficulty in pattern generated commands executed directly by the Arduino framework, things are more complicated when it comes to executing commands indirectly via cmd /c.
Under certain conditions, the first and last quotation mark of the command passed via the argument to /c are stripped.
These conditions occur for the command generated via the platform's recipe.hooks.objcopy.postobjcopy.1.pattern.windows property when the both the path to avr-objdump and the temporary build path contain a space.
The removal of the essential quotes from the start and end of the command executed by cmd /c causes an error. for example, if the avr-objdump path was:
C:\Users\username with spaces\AppData\Local\\Arduino15\packages\arduino\tools\avr-gcc\7.3.0-atmel3.6.1-arduino7\bin\avr-objdump
The command will fail with the error:
'C:\Users\username' is not recognized as an internal or external command, operable program or batch file.
One of the required conditions for the unwanted quote stripping is that the first character of the argument to cmd /c is a double quote. So this unwanted behavior can be avoided by ensuring the first character is not a double quote. That is accomplished here by adding a dummy command (adds a blank line to the output, but the most suitable "no operation" candidate I found) that does not require quoting at the start of the argument.
Demo of bug
$ arduino-cli version
arduino-cli.exe Version: 0.27.1 Commit: a900cfb2 Date: 2022-09-18T15:57:51Z
$ export ARDUINO_DIRECTORIES_DATA="/tmp/arduino-cli-directories/has space/data" # Cause avr-objdump to be installed under a path with spaces
$ export ARDUINO_BOARD_MANAGER_ADDITIONAL_URLS="https://mcudude.github.io/MegaCore/package_MCUdude_MegaCore_index.json"
$ arduino-cli core update-index
Downloading index: package_index.tar.bz2 downloaded
Downloading index: package_MCUdude_MegaCore_index.json downloaded
$ arduino-cli core install MegaCore:avr
[...]
$ export ARDUINO_DIRECTORIES_USER="/tmp/arduino-cli-directories/user"
$ git clone --depth 1 https://github.com/MCUdude/MegaCore "${ARDUINO_DIRECTORIES_USER}/hardware/MegaCore-dev"
[...]
$ arduino-cli sketch new "/tmp/FooSketch"
Sketch created in: C:\Users\per\AppData\Local\Temp\FooSketch
$ arduino-cli compile --fqbn MegaCore-dev:avr:2560 --verbose --build-path "/tmp/arduino-cli-directories/has space/build" "/tmp/FooSketch"
[...]
cmd /C "C:\\Users\\per\\AppData\\Local\\Temp\\arduino-cli-directories\\has space\\data\\packages\\arduino\\tools\\avr-gcc\\7.3.0-atmel3.6.1-arduino7/bin/avr-objdump" --disassemble --source --line-numbers --demangle --section=.text "C:\\Users\\per\\AppData\\Local\\Temp\\arduino-cli-directories\\has space\\build/FooSketch.ino.elf" > "C:\\Users\\per\\AppData\\Local\\Temp\\arduino-cli-directories\\has space\\build/FooSketch.ino_atmega2560_16000000L.lst"
'C:\Users\per\AppData\Local\Temp\arduino-cli-directories\has' is not recognized as an internal or external command,
operable program or batch file.
[...]
(Arduino CLI used only because it makes it easy to customize the paths to contain spaces)
Additional information
Alternatives considered
It might seem that a less hacky approach would be to simply wrap the entire argument to cmd /c in an extra layer of sacrificial quotes, leaving the essential quotes intact in the command that is finally executed. While that does indeed work when running the command directly from the command line, it does not work when used in a pattern in the Arduino platform configuration framework. The problem is that the string generated from the platform pattern can not be executed as is. It must be split into arguments. The list of arguments space separated, with quoting where an argument contains a space.
These arguments may contain quote characters, which must be escaped. This escaping is where the simple approach of extra quotes becomes unusable. The os/exec Go package used to handle the escaping does that in a way that is compatible with directly executing a command, but is not compatible with the legacy escaping system used by cmd:
On Windows, processes receive the whole command line as a single string and do their own parsing. Command combines and quotes Args into a command line string with an algorithm compatible with applications using CommandLineToArgvW (which is the most common way). Notable exceptions are msiexec.exe and cmd.exe (and thus, all batch files), which have a different unquoting algorithm. In these or other similar cases, you can do the quoting yourself and provide the full command line in SysProcAttr.CmdLine, leaving Args empty.
Scope of bug
It seems this problem of the command having paths with spaces did not occur in Arduino IDE 1.x due to it using a Windows "8.3" format for the default build path, which does not contain spaces. Arduino CLI (and thus Arduino IDE 2.x) uses the full path format instead. However, the bug could also occur in Arduino IDE 1.x if the user sets a custom build.path with spaces.
recipe.hooks.savehex.presavehex.2.pattern.windows also uses cmd /C. However, this command is not subject to the quote stripping because it does not ever have a double quote as the first character of the argument.
This platform uses a build hook to save a disassembler listing to the build folder when compiling sketches.
Due to the use of a redirection operator in the command, it was necessary to execute it via the Windows command interpreter by passing it as an argument to
cmd /c
.Although paths with spaces can be used without difficulty in pattern generated commands executed directly by the Arduino framework, things are more complicated when it comes to executing commands indirectly via
cmd /c
.The
cmd /c
command has an odd handling of quotes:https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/cmd#remarks
Under certain conditions, the first and last quotation mark of the command passed via the argument to
/c
are stripped.These conditions occur for the command generated via the platform's
recipe.hooks.objcopy.postobjcopy.1.pattern.windows
property when the both the path toavr-objdump
and the temporary build path contain a space.The removal of the essential quotes from the start and end of the command executed by
cmd /c
causes an error. for example, if theavr-objdump
path was:The command will fail with the error:
One of the required conditions for the unwanted quote stripping is that the first character of the argument to
cmd /c
is a double quote. So this unwanted behavior can be avoided by ensuring the first character is not a double quote. That is accomplished here by adding a dummy command (adds a blank line to the output, but the most suitable "no operation" candidate I found) that does not require quoting at the start of the argument.Demo of bug
(Arduino CLI used only because it makes it easy to customize the paths to contain spaces)
Additional information
Alternatives considered
It might seem that a less hacky approach would be to simply wrap the entire argument to
cmd /c
in an extra layer of sacrificial quotes, leaving the essential quotes intact in the command that is finally executed. While that does indeed work when running the command directly from the command line, it does not work when used in a pattern in the Arduino platform configuration framework. The problem is that the string generated from the platform pattern can not be executed as is. It must be split into arguments. The list of arguments space separated, with quoting where an argument contains a space.These arguments may contain quote characters, which must be escaped. This escaping is where the simple approach of extra quotes becomes unusable. The
os/exec
Go package used to handle the escaping does that in a way that is compatible with directly executing a command, but is not compatible with the legacy escaping system used by cmd:https://pkg.go.dev/os/exec#Command
Scope of bug
It seems this problem of the command having paths with spaces did not occur in Arduino IDE 1.x due to it using a Windows "8.3" format for the default build path, which does not contain spaces. Arduino CLI (and thus Arduino IDE 2.x) uses the full path format instead. However, the bug could also occur in Arduino IDE 1.x if the user sets a custom
build.path
with spaces.recipe.hooks.savehex.presavehex.2.pattern.windows
also usescmd /C
. However, this command is not subject to the quote stripping because it does not ever have a double quote as the first character of the argument.Related
Originally reported by @cattledogGH
Previous discussion at https://github.com/MCUdude/MegaCoreX/pull/159