Skip to content

Add prefer_clang_cl_over_msvc#1516

Merged
NobodyXu merged 5 commits intorust-lang:mainfrom
dennisameling:add-clang-cl-preference-tests
Aug 23, 2025
Merged

Add prefer_clang_cl_over_msvc#1516
NobodyXu merged 5 commits intorust-lang:mainfrom
dennisameling:add-clang-cl-preference-tests

Conversation

@dennisameling
Copy link
Copy Markdown
Contributor

@dennisameling dennisameling commented Aug 10, 2025

Closes #882

This could probably replace #1367. Now that #1506 has been merged, windows_registry_find_tool can find clang-cl.exe correctly.

This PR also adds a fix for something I missed in find_tools as part of #1506: clang_cl should be set to true when using that toolchain.

Lastly, this PR adds some tests to ensure that the logic that @briansmith described here works as expected.

dennisameling
dennisameling commented Aug 10, 2025
View reviewed changes
Comment thread src/windows/find_tools.rs Outdated
Comment thread tests/support/mod.rs
Comment thread tests/test.rs Outdated
@dennisameling
Copy link
Copy Markdown
Contributor Author

dennisameling commented Aug 10, 2025

The failing tests in CI seem to be caused by ca81dcc, which has the same failing tests on the main branch.

@NobodyXu
Copy link
Copy Markdown
Contributor

NobodyXu commented Aug 11, 2025

#1426

Confirmed that nightly has removed env in favor of abi, so it's not caused by this PR or that one

@NobodyXu
Copy link
Copy Markdown
Contributor

NobodyXu commented Aug 11, 2025

Fixed in #1517, just rebasing would fix it

@dennisameling dennisameling force-pushed the add-clang-cl-preference-tests branch from 064f3f0 to 27efba8 Compare August 11, 2025 12:59
NobodyXu
NobodyXu requested changes Aug 11, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thanks!

I think you need to modify line 177 to add clang-cl to the array?

NobodyXu
NobodyXu requested changes Aug 11, 2025
View reviewed changes
Comment thread tests/test.rs Outdated
Comment thread tests/test.rs
@NobodyXu NobodyXu mentioned this pull request Aug 11, 2025
Closed
@dennisameling dennisameling force-pushed the add-clang-cl-preference-tests branch from 27efba8 to 44975d7 Compare August 17, 2025 15:21
@dennisameling
Copy link
Copy Markdown
Contributor Author

dennisameling commented Aug 17, 2025 •
edited
Loading

Looks like some (seemingly unrelated) tests are now failing with:

 thread 'apple_sdkroot_wrong' panicked at tests/support/mod.rs:143:13:
didn't find "/Applications/Xcode_15.4.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS17.5.sdk" in ["-O2", "-fPIC", "--target=arm64-apple-ios", "-miphoneos-version-min=17.5", "-isysroot", "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk", "-Wall", "-Wextra", "-o", "/Users/runner/work/cc-rs/cc-rs/target/aarch64-apple-darwin/debug/cc-shim-teste6yNGK/db3b6bfb95261072-foo.o", "-c", "foo.c"]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

... which is interesting because the documentation for macos-14-arm64 shows that the iOS 17.5 SDK should be installed. Any thoughts on this?

@NobodyXu
Copy link
Copy Markdown
Contributor

NobodyXu commented Aug 18, 2025 •
edited
Loading

It seems that the path to the sdj has changed.

We expect to find /Applications/Xcode_15.4.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS17.5.sdk, but found:

/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk

Maybe we are using the wrong sdk?

NobodyXu
NobodyXu requested changes Aug 18, 2025
View reviewed changes
Comment thread Cargo.toml Outdated
Comment thread src/windows/find_tools.rs
@dennisameling dennisameling force-pushed the add-clang-cl-preference-tests branch from 44975d7 to 5a2790e Compare August 23, 2025 09:33
dennisameling
dennisameling commented Aug 23, 2025
View reviewed changes
Comment thread src/lib.rs
NobodyXu
NobodyXu approved these changes Aug 23, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thank you!

@NobodyXu NobodyXu merged commit d264a7c into rust-lang:main Aug 23, 2025
77 checks passed
@dennisameling dennisameling mentioned this pull request Mar 24, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NobodyXu NobodyXu NobodyXu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add "prefer to use clang instead of MSVC" API that can automatically find Microsoft-provided clang

2 participants

@dennisameling @NobodyXu