-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Extract detecting CC/CXX into helper script so we can use it from mono.proj #31854
Conversation
compier -> compiler
Sorry about that typo, I happened to stumble upon that condition today on different platform and fixed in #31865 😄 (will revert from there). |
@akoeplinger This works. It fixes the issue I hit nicely. I'm going to close my PR as this supersedes it. |
</PropertyGroup> | ||
|
||
<Message Text="Configuring Mono with '$(_MonoConfigureParams)'" Importance="High" /> | ||
<MakeDir Directories="$(MonoObjDir)" /> | ||
<Exec Command="NOCONFIGURE=1 $(MonoProjectRoot)autogen.sh" /> | ||
<Exec Command="$(MonoProjectRoot)configure $(_MonoConfigureParams)" WorkingDirectory="$(MonoObjDir)" /> | ||
<Exec Command="bash -c 'source $(RepositoryEngineeringDir)native/init-compiler.sh $(Platform) clang && $(MonoProjectRoot)configure $(_MonoConfigureParams)'" WorkingDirectory="$(MonoObjDir)" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akoeplinger, would it be possible to let eng/build.sh
determine which compiler to use? i.e. the hardcoded clang
to be replaced by $(Compiler)
defined here:
Lines 275 to 292 in 2ef008e
-clang*) | |
arguments="$arguments /p:Compiler=$opt" | |
shift 1 | |
;; | |
-cmakeargs) | |
if [ -z ${2+x} ]; then | |
echo "No cmake args supplied." 1>&2 | |
exit 1 | |
fi | |
cmakeargs="${cmakeargs} ${opt} $2" | |
shift 2 | |
;; | |
-gcc*) | |
arguments="$arguments /p:Compiler=$opt" | |
shift 1 | |
;; |
Doing that will help systems which require gcc build (
runtime/build.sh -gcc ...
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@am11 sure, that would be great!
Also fixed a small typo in the detection script, see first commit.
Closes #31771