Skip to content
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.

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

Pass native build args from top-level scripts #2071

Merged
merged 7 commits into from
Jan 30, 2020
Merged

Pass native build args from top-level scripts #2071

merged 7 commits into from
Jan 30, 2020

Conversation

am11
Copy link
Member

@am11 am11 commented Jan 23, 2020

Add ability to pass --cross, --clangx.y, --gccx.y and --cmakeargs to native scripts via top-level build.sh. e.g.

./build.sh --subsetCategory coreclr --clang8 --cross --cmakeargs -DFeatureX=1 --cmakeargs Y='testing'
./build.sh --subsetCategory installer --gcc5 --cross --cmakeargs -DFeatureX=1 --cmakeargs Y='testing'
./build.sh --subsetCategory libraries --clang --cross --cmakeargs -DFeatureX=1 --cmakeargs Y='testing'

Also address CR feedback from PRs #1753 and #1602 (in separate commits).

@jkotas jkotas requested a review from ViktorHofer January 23, 2020 13:55
@am11 am11 marked this pull request as ready for review January 23, 2020 14:23
<_CoreClrBuildArg Condition="'$(ContinuousIntegrationBuild)' == 'true'" Include="-ci" />
<_CoreClrBuildArg Condition="'$(CrossBuild)' == 'true'" Include="-cross" />
Copy link
Member Author

@am11 am11 Jan 23, 2020

Choose a reason for hiding this comment

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

[IMO]
Now that all native build scripts share common arguments via:

usage()
{
echo "Usage: $0 <options>"
echo ""
echo "Common Options:"
echo ""
echo "BuildArch can be: -arm, -armel, -arm64, -armel, x64, x86, -wasm"
echo "BuildType can be: -debug, -checked, -release"
echo "-bindir: output directory (defaults to $__ProjectRoot/artifacts)"
echo "-ci: indicates if this is a CI build."
echo "-clang: optional argument to build using clang in PATH (default)."
echo "-clangx.y: optional argument to build using clang version x.y."
echo "-cmakeargs: user-settable additional arguments passed to CMake."
echo "-configureonly: do not perform any builds; just configure the build."
echo "-cross: optional argument to signify cross compilation,"
echo " will use ROOTFS_DIR environment variable if set."
echo "-gcc: optional argument to build using gcc in PATH."
echo "-gccx.y: optional argument to build using gcc version x.y."
echo "-msbuildonunsupportedplatform: build managed binaries even if distro is not officially supported."
echo "-ninja: target ninja instead of GNU make"
echo "-numproc: set the number of build processes."
echo "-portablebuild: pass -portablebuild=false to force a non-portable build."
echo "-skipconfigure: skip build configuration."
echo "-skipgenerateversion: disable version generation even if MSBuild is supported."
echo "-stripsymbols: skip native image generation."
echo "-verbose: optional argument to enable verbose build output."
echo ""
echo "Additional Options:"
echo ""
for i in "${!usage_list[@]}"; do
echo "${usage_list[${i}]}"
done
echo ""
exit 1
}
we can extract these argument construction lines from coreclr/coreclr.proj, installer/corehost/build.proj and libraries/Native/build-native.proj into a common .props file, to avoid three different approaches of doing exactly the same thing (coreclr: ItemGroup, installer: property concatenation and libraries: separate properties followed by concatenation).

Copy link
Member

Choose a reason for hiding this comment

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

@am11
Copy link
Member Author

am11 commented Jan 29, 2020

@janvorli, could you please give it a pass?

eng/build.sh Outdated Show resolved Hide resolved
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@janvorli
Copy link
Member

@am11 thank you for spending time to figure out a cleaner solution to the problem of passing the argument with spaces.

@janvorli janvorli merged commit 643e9cf into dotnet:master Jan 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants