Prevent build scripts from letting random values through #35642
Conversation
…g the repo. Also, removed the casing restrictions on the bash version.
|
Tagging subscribers to this area: @ViktorHofer |
| passedConfig="$(echo "$2" | awk '{print tolower($0)}')" | ||
| case "$passedConfig" in | ||
| debug|release|checked) | ||
| val="$(tr '[:lower:]' '[:upper:]' <<< ${passedConfig:0:1})${passedConfig:1}" |
There was a problem hiding this comment.
A nit - you can avoid this double transformation (once using awk and then using tr) by just keeping the original command and then passing the val to case and checking for Debug, Release and Checked.
There was a problem hiding this comment.
Do you mean by converting all to lowercase and then the first one to uppercase in the same command? I ask this because the tr command in val changes the first letter to uppercase, but leaves the rest untouched. That's why I'm using awk beforehand to convert everything to lowercase beforehand, in order to allow input like "RELEASE" to be valid. I'm removing the casing restrictions on the Linux scripts, aside from fixing the problems.
There was a problem hiding this comment.
I am sorry, I've misread the tr command.
| arch=$2 | ||
| passedArch="$(echo "$2" | awk '{print tolower($0)}')" | ||
| case "$passedArch" in | ||
| x64|x86|arm|armel|arm64|wasm) |
There was a problem hiding this comment.
Is it intentional that armel is missing in build.ps1 above?
There was a problem hiding this comment.
I think it is, as there is no armel flavor of Windows.
There was a problem hiding this comment.
Yes, I followed the help command when adding these validations. Thanks for the clarification Jan!
ghost
locked as resolved and limited conversation to collaborators
Fixes #35222 and a few more things.
The runtime repo's build scripts,
build.cmdandbuild.sh, were allowing random values through certain flags, such as--os,--arch, and--configuration. This caused certain tasks prior to building like restoring projects to trigger, giving the user a false sense of security that it would work. However, later on, the build would obviously fail. The error messages shown would be entirely unrelated and therefore the user couldn't know what went wrong. This was a common scenario due to typos.With this PR, invalid values to these flags are immediately called out to the user, alongside a suggestion on which arguments are valid for the given flag. This in turn prevents loss of time waiting for the eventual build failure.
As a follow up to improve the experience with these scripts, the letter casing restrictions in the Linux script (
build.sh) have been removed. Users can now type--os LINUXor--os linuxfor example, and it will work. The casing stuff is now managed internally. Also,--runtimeConfigurationand--librariesConfigurationwere fixed as well.