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

Make init-compiler.sh fall back to and work with generic compiler #14756

Merged

Conversation

omajid
Copy link
Member

@omajid omajid commented May 8, 2024

If a version-specific compiler (eg, clang-17) is passed to the script, it works as before. And like before, if a generic compiler (eg clang) is passed to the script, it prefers to find and use the few specific/known/tested versions of compilers (eg, clang means use clang-18 if possible). However, unlike before, if none of those specific known versions are found, it correctly falls back to the generic compiler binary (eg clang) in $PATH.

The script was actually supposed to do all this already, but it seems to have been broken when warning messages were added. These warning messages were parsed by msbuild. Msbuild would see the warning messages and fail the build.

With a generic compiler, this script now also invokes it via $compiler -dumpversion (which works on both clang and gcc) to find the version for various (existing) version checks. The compiler was already being invoked to check if -fuse-ld=lld is supported.

Fixes: #14632

@omajid
Copy link
Member Author

omajid commented May 8, 2024

CI run to show that this works on macos as well: dotnet/runtime#100828

@omajid
Copy link
Member Author

omajid commented May 8, 2024

I have made one major change from the previous version of this PR (#14696).

I did some testing on macOS, and it looks like only the version-less clang, clang++ binaries are available. I have dropped the fail-on-Darwin check for that.

@omajid
Copy link
Member Author

omajid commented May 9, 2024

cc @akoeplinger @am11 @janvorli @tmds

@jakubch1 any chance you can test to confirm this doesn't break anything for you?

Copy link
Member

@am11 am11 left a comment

Choose a reason for hiding this comment

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

Few nits and shellcheck SC2235 fix, otherwise looks good. Thanks!

eng/common/native/init-compiler.sh Outdated Show resolved Hide resolved
eng/common/native/init-compiler.sh Outdated Show resolved Hide resolved
eng/common/native/init-compiler.sh Outdated Show resolved Hide resolved
eng/common/native/init-compiler.sh Outdated Show resolved Hide resolved
eng/common/native/init-compiler.sh Outdated Show resolved Hide resolved
eng/common/native/init-compiler.sh Outdated Show resolved Hide resolved
omajid and others added 3 commits May 21, 2024 06:42
If a version-specific compiler (eg, clang-17) is passed to the script,
it works as before. And like before, if a generic compiler (eg clang) is
passed to the script, it prefers to find and use the few
specific/known/tested versions of compilers (eg, clang means use
clang-18 if possible). However, unlike before, if none of those specific
known versions are found, it correctly falls back to the generic
compiler binary (eg clang) in $PATH.

The script was actually supposed to do all this already, but it seems to
have been broken when warning messages were added. These warning
messages were parsed by msbuild. Msbuild would see the warning messages
and fail the build.

With a generic compiler, this script now also invokes it via $compiler
-dumpversion (which works on both clang and gcc) to find the version for
various (existing) version checks. The compiler was already being
invoked to check if -fuse-ld=lld is supported.

Fixes: dotnet#14632
We just need the actual path to commands, not any error/warning
messages.
@omajid omajid force-pushed the remove-hardcoded-compiler-list-init-compiler-2 branch from 2fdd8b2 to f3eaf07 Compare May 21, 2024 10:42
@omajid
Copy link
Member Author

omajid commented May 21, 2024

I don't have merge powers, can someone please merge this for me?

@janvorli janvorli merged commit e1ff12e into dotnet:main May 21, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow building with newer clang versions without changing init-compilers.sh.
4 participants