Skip to content

Allow ML.NET native binaries to work on Windows machines that don't have the VC runtime installed. #1828

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

Merged
merged 1 commit into from
Dec 5, 2018

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Dec 5, 2018

This allows ML.NET to run on Windows Nano containers.

I also ported 2 Unix compile options we are using in core-setup and corefx that were missed when originally creating the ML.NET native build infrastructure.

Fix #1823

…ave the VC runtime installed.

This allows ML.NET to run on Windows Nano containers.

I also ported 2 Unix compile options we are using in core-setup and corefx that were missed when originally creating the ML.NET native build infrastructure.

Fix dotnet#1823
@@ -62,6 +65,8 @@ if(WIN32)
list(APPEND RESOURCES ${CMAKE_SOURCE_DIR}/../../Tools/NativeVersion.rc)
else()
add_compile_options(-Wno-unused-local-typedef)
add_compile_options(-fPIC)
Copy link
Member Author

Choose a reason for hiding this comment

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

I originally missed these setting because I copied the "common" CMake file from dotnet/core-setup. However, /MT for Windows and these two settings on Unix aren't in the common CMake file in that repo, but are spread to the individual CMake files for each library:

https://github.com/dotnet/core-setup/blob/8fe49b4211abaa649784966a4f6b55a38a237d9e/src/corehost/cli/hostpolicy/CMakeLists.txt#L7-L14
https://github.com/dotnet/core-setup/blob/8fe49b4211abaa649784966a4f6b55a38a237d9e/src/corehost/cli/fxr/CMakeLists.txt#L7-L14

Which is why they were overlooked originally.

Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka left a comment

Choose a reason for hiding this comment

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

:shipit:

@eerhardt eerhardt merged commit 4c047bf into dotnet:master Dec 5, 2018
@shauheen shauheen added the Build Build related issue label Dec 5, 2018
@eerhardt eerhardt deleted the FixNano branch December 6, 2018 00:22
@spsinghats
Copy link

I tried with version 0.10 it still does not find the CPUMathNative,

@spsinghats
Copy link

add_library(CpuMathNative SHARED ${SOURCES} ${RESOURCES})

Shouldn't this be STATIC instead of SHARED ?

@eerhardt
Copy link
Member Author

@spsinghats - can you open a new issue with your repro steps? it still does not find the CPUMathNative isn't enough information to help you here.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Build Build related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants