Skip to content

[UR][CMake]Set CMAKE_MSVC_RUNTIME_LIBRARY to fix UMF linking issues #17366

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

Jump to bottom
Merged
merged 1 commit into from
Mar 14, 2025
Merged

[UR][CMake]Set CMAKE_MSVC_RUNTIME_LIBRARY to fix UMF linking issues #17366

merged 1 commit into from
Mar 14, 2025

Conversation

jsji
Copy link
Contributor

@jsji jsji commented Mar 8, 2025

Caller might set CMAKE_MSVC_RUNTIME_LIBRARY to MultiThreaded to do
static linking of MSVC runtime. However, UMF is not yet ready for static
linking. Override the CMAKE_MSVC_RUNTIME_LIBRARY to fix lnking failures.

@jsji
[UR][CMake]Set CMAKE_MSVC_RUNTIME_LIBRARY to fix UMF linking issues
2a4cb20 
Caller might set CMAKE_MSVC_RUNTIME_LIBRARY to MultiThreaded to do
static linking of MSVC runtime. However, UMF is not yet ready for static
linking. Override the CMAKE_MSVC_RUNTIME_LIBRARY to fix lnking failures.
@jsji jsji requested a review from a team as a code owner March 8, 2025 03:37
@jsji jsji self-assigned this Mar 8, 2025
@jsji jsji temporarily deployed to WindowsCILock March 8, 2025 03:38 — with GitHub Actions Inactive
@jsji jsji temporarily deployed to WindowsCILock March 8, 2025 03:53 — with GitHub Actions Inactive
@jsji jsji temporarily deployed to WindowsCILock March 8, 2025 03:53 — with GitHub Actions Inactive
@kbenzie kbenzie requested a review from nrspruit March 11, 2025 13:54
nrspruit
nrspruit approved these changes Mar 11, 2025
View reviewed changes
Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

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

Looks good to me

@pbalcer , can you take a look to see if you see any issues with this fix?

@nrspruit nrspruit requested a review from pbalcer March 11, 2025 14:12
@pbalcer
Copy link
Contributor

pbalcer commented Mar 11, 2025

patch lgtm, but I'm curious what's the issue with UMF.

However, UMF is not yet ready for static linking.

@PatKamin #17347 this probably should be closed. If you have time, can you investigate how we can fix this properly in UMF?

@jsji jsji closed this Mar 11, 2025
@jsji jsji reopened this Mar 11, 2025
@jsji jsji temporarily deployed to WindowsCILock March 11, 2025 15:51 — with GitHub Actions Inactive
@jsji jsji temporarily deployed to WindowsCILock March 11, 2025 16:31 — with GitHub Actions Inactive
@jsji jsji temporarily deployed to WindowsCILock March 11, 2025 16:31 — with GitHub Actions Inactive
@PatKamin PatKamin mentioned this pull request Mar 11, 2025
Closed
@jsji
Copy link
Contributor Author

jsji commented Mar 14, 2025

@intel/llvm-gatekeepers This is ready for merge. Thanks.

@sarnex sarnex merged commit 1167ee6 into intel:sycl Mar 14, 2025
79 of 88 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nrspruit nrspruit nrspruit approved these changes

@pbalcer pbalcer pbalcer approved these changes

Assignees

@jsji jsji

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jsji @pbalcer @nrspruit @sarnex