Skip to content

Prevent build scripts from letting random values through #35642

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 13, 2020
Merged

Prevent build scripts from letting random values through #35642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a343302
Arch, OS, and Config no longer let random values through when buildin…
ivdiazsa Apr 24, 2020
50d2169
Fixed the bash build script! Also, casing restrictions are gone!
ivdiazsa Apr 29, 2020
6e0564e
Added safeguards for bogus values on -rc and -lc on the sh build script.
ivdiazsa Apr 29, 2020
4fd492f
Merge
ivdiazsa May 6, 2020
35f16a3
Git is stubborn
ivdiazsa May 6, 2020
e3a8339
Fixed the merge conflict on build.sh
ivdiazsa May 6, 2020
0ffb1d8
Used awk instead of declare for increased compatibility on build.sh
ivdiazsa May 6, 2020
f694241
Restored execute permissions to build.sh
ivdiazsa May 7, 2020
ee291ef
Merge branch 'master' into build_gateways
ivdiazsa May 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions eng/build.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
Param(
[switch][Alias('h')]$help,
[switch][Alias('t')]$test,
[string[]][Alias('c')]$configuration = @("Debug"),
[ValidateSet("Debug","Release","Checked")][string[]][Alias('c')]$configuration = @("Debug"),
[string][Alias('f')]$framework,
[string]$vs,
[string]$os,
[ValidateSet("Windows_NT","Unix")][string]$os,
[switch]$allconfigurations,
[switch]$coverage,
[string]$testscope,
[switch]$testnobuild,
[string[]][Alias('a')]$arch = @([System.Runtime.InteropServices.RuntimeInformation]::ProcessArchitecture.ToString().ToLowerInvariant()),
[ValidateSet("x86","x64","arm","arm64")][string[]][Alias('a')]$arch = @([System.Runtime.InteropServices.RuntimeInformation]::ProcessArchitecture.ToString().ToLowerInvariant()),
[Parameter(Position=0)][string][Alias('s')]$subset,
[ValidateSet("Debug","Release","Checked")][string][Alias('rc')]$runtimeConfiguration,
[ValidateSet("Debug","Release")][string][Alias('lc')]$librariesConfiguration,
Expand Down
96 changes: 89 additions & 7 deletions eng/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ while [[ $# > 0 ]]; do
usage
exit 0
;;

-subset|-s)
if [ -z ${2+x} ]; then
arguments="$arguments /p:Subset=help"
Expand All @@ -116,23 +117,46 @@ while [[ $# > 0 ]]; do
shift 2
fi
;;

-arch)
if [ -z ${2+x} ]; then
echo "No architecture supplied. See help (--help) for supported architectures." 1>&2
exit 1
fi
arch=$2
passedArch="$(echo "$2" | awk '{print tolower($0)}')"
case "$passedArch" in
x64|x86|arm|armel|arm64|wasm)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional that armel is missing in build.ps1 above?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is, as there is no armel flavor of Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I followed the help command when adding these validations. Thanks for the clarification Jan!

arch=$passedArch
;;
*)
echo "Unsupported target architecture '$2'."
echo "The allowed values are x86, x64, arm, armel, arm64, and wasm."
exit 1
;;
esac
shift 2
;;

-configuration|-c)
if [ -z ${2+x} ]; then
if [ -z ${2+x} ]; then
echo "No configuration supplied. See help (--help) for supported configurations." 1>&2
exit 1
fi
val="$(tr '[:lower:]' '[:upper:]' <<< ${2:0:1})${2:1}"
passedConfig="$(echo "$2" | awk '{print tolower($0)}')"
case "$passedConfig" in
debug|release|checked)
val="$(tr '[:lower:]' '[:upper:]' <<< ${passedConfig:0:1})${passedConfig:1}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry, I've misread the tr command.

;;
*)
echo "Unsupported target configuration '$2'."
echo "The allowed values are Debug, Release, and Checked."
exit 1
;;
esac
arguments="$arguments -configuration $val"
shift 2
;;

-framework|-f)
if [ -z ${2+x} ]; then
echo "No framework supplied. See help (--help) for supported frameworks." 1>&2
Expand All @@ -142,19 +166,47 @@ while [[ $# > 0 ]]; do
arguments="$arguments /p:BuildTargetFramework=$val"
shift 2
;;

-os)
if [ -z ${2+x} ]; then
echo "No target operating system supplied. See help (--help) for supported target operating systems." 1>&2
exit 1
fi
os=$2
arguments="$arguments /p:TargetOS=$2"
passedOS="$(echo "$2" | awk '{print tolower($0)}')"
case "$passedOS" in
windows_nt)
os="Windows_NT" ;;
linux)
os="Linux" ;;
freebsd)
os="FreeBSD" ;;
osx)
os="OSX" ;;
tvos)
os="tvOS" ;;
ios)
os="iOS" ;;
android)
os="Android" ;;
browser)
os="Browser" ;;
sunos)
os="SunOS" ;;
*)
echo "Unsupported target OS '$2'."
echo "The allowed values are Windows_NT, Linux, FreeBSD, OSX, tvOS, iOS, Android, Browser, and SunOS."
exit 1
;;
esac
arguments="$arguments /p:TargetOS=$os"
shift 2
;;

-allconfigurations)
arguments="$arguments /p:BuildAllConfigurations=true"
shift 1
;;

-testscope)
if [ -z ${2+x} ]; then
echo "No test scope supplied. See help (--help) for supported test scope values." 1>&2
Expand All @@ -163,40 +215,68 @@ while [[ $# > 0 ]]; do
arguments="$arguments /p:TestScope=$2"
shift 2
;;

-testnobuild)
arguments="$arguments /p:TestNoBuild=true"
shift 1
;;

-coverage)
arguments="$arguments /p:Coverage=true"
shift 1
;;

-runtimeconfiguration|-rc)
if [ -z ${2+x} ]; then
echo "No runtime configuration supplied. See help (--help) for supported runtime configurations." 1>&2
exit 1
fi
val="$(tr '[:lower:]' '[:upper:]' <<< ${2:0:1})${2:1}"
passedRuntimeConf="$(echo "$2" | awk '{print tolower($0)}')"
case "$passedRuntimeConf" in
debug|release|checked)
val="$(tr '[:lower:]' '[:upper:]' <<< ${passedRuntimeConf:0:1})${passedRuntimeConf:1}"
;;
*)
echo "Unsupported runtime configuration '$2'."
echo "The allowed values are Debug, Release, and Checked."
exit 1
;;
esac
arguments="$arguments /p:RuntimeConfiguration=$val"
shift 2
;;

-librariesconfiguration|-lc)
if [ -z ${2+x} ]; then
echo "No libraries configuration supplied. See help (--help) for supported libraries configurations." 1>&2
exit 1
fi
arguments="$arguments /p:LibrariesConfiguration=$2"
passedLibConf="$(echo "$2" | awk '{print tolower($0)}')"
case "$passedLibConf" in
debug|release)
val="$(tr '[:lower:]' '[:upper:]' <<< ${passedLibConf:0:1})${passedLibConf:1}"
;;
*)
echo "Unsupported libraries configuration '$2'."
echo "The allowed values are Debug and Release."
exit 1
;;
esac
arguments="$arguments /p:LibrariesConfiguration=$val"
shift 2
;;

-cross)
crossBuild=1
arguments="$arguments /p:CrossBuild=True"
shift 1
;;

-clang*)
arguments="$arguments /p:Compiler=$opt"
shift 1
;;

-cmakeargs)
if [ -z ${2+x} ]; then
echo "No cmake args supplied." 1>&2
Expand All @@ -205,10 +285,12 @@ while [[ $# > 0 ]]; do
cmakeargs="${cmakeargs} ${opt} $2"
shift 2
;;

-gcc*)
arguments="$arguments /p:Compiler=$opt"
shift 1
;;

*)
extraargs="$extraargs $1"
shift 1
Expand Down