Silent failures with invalid arch
PazerOP opened this issue · 3 comments
See #26 as well.
If you accidentally provide an invalid arch (like Win32 instead of x86), this action will report success, but will not properly configure the PATH.
For example:
Run ilammy/msvc-dev-cmd@v1.5.0
with:
arch: Win32
Found with vswhere: C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvarsall.bat
Configured Developer Command Prompt
Running CMake after this then tries to use GCC.
Hm... Indeed.
I assumed that vcvarsall.bat likes to pretend that it's successful while not doing anything if the architecture parameter is something that it does not understand. However, it does report the issue. It's this action which does not see the failure and assumes it was successful.
Though, it does seem that vcvarsall.bat does not set the exit code correctly. It just prints
[ERROR:vcvarsall.bat] Invalid argument found : Win32
[ERROR:vcvarsall.bat] Error in script usage. The correct usage is:
Syntax:
vcvarsall.bat [arch] [platform_type] [winsdk_version] [-vcvars_ver=vc_version] [-vcvars_spectre_libs=spectre_mode]
where :
[arch]: x86 | amd64 | x86_amd64 | x86_arm | x86_arm64 | amd64_x86 | amd64_arm | amd64_arm64
[platform_type]: {empty} | store | uwp
[winsdk_version] : full Windows 10 SDK number (e.g. 10.0.10240.0) or "8.1" to use the Windows 8.1 SDK.
[vc_version] : {none} for latest installed VC++ compiler toolset |
"14.0" for VC++ 2015 Compiler Toolset |
"14.xx" for the latest 14.xx.yyyyy toolset installed (e.g. "14.11") |
"14.xx.yyyyy" for a specific full version number (e.g. "14.11.25503")
[spectre_mode] : {none} for libraries without spectre mitigations |
"spectre" for libraries with spectre mitigations
The store parameter sets environment variables to support Universal Windows Platform application,development and is an alias for 'uwp'.
For example:
vcvarsall.bat x86_amd64
vcvarsall.bat x86_amd64 10.0.10240.0
vcvarsall.bat x86_arm uwp 10.0.10240.0
vcvarsall.bat x86_arm onecore 10.0.10240.0 -vcvars_ver=14.0
vcvarsall.bat x64 8.1
vcvarsall.bat x64 store 8.1
Please make sure either Visual Studio or C++ Build SKU is installed.
Then seems to exit successfully, so we extract the (unchanged) environment, and assume it's okay.
I guess I can make this action validate the arch parameter against a list of allowed architectures and fail if it's something weird. (Now I wonder if other parameters should be validated too...)
However, the better way will be to detect that vcvarsall.bat has failed on invalid input.
I've just published v1.6.0 (aliased as v1) which should react properly to incorrect parameter values and fail this action instead of allowing the build to continue.
Note that arch: Win32 is now not an error as well.