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.

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

support vc runtime static linkage #472

Merged
merged 5 commits into from
Jun 20, 2023
Merged

support vc runtime static linkage #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d5b3dc
support vc runtime static linkage
wenbingl Jun 12, 2023
83a457e
typo
wenbingl Jun 12, 2023
0e3f4b8
add unit test
wenbingl Jun 12, 2023
b977512
Update ci.yml
wenbingl Jun 13, 2023
5f5143c
Merge branch 'main' into vcstat
wenbingl Jun 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .pipelines/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,17 @@ jobs:
ctest -C RelWithDebInfo --output-on-failure
displayName: Run C++ native tests

- job: WindowsStaticVC
pool:
vmImage: 'windows-latest'

steps:
- script: |
call .\build.bat -DOCOS_ENABLE_CTEST=ON -DCMAKE_MSVC_RUNTIME_LIBRARY="MultiThreaded$<$<CONFIG:Debug>:Debug>"
Copy link
Member Author

Choose a reason for hiding this comment

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

the argument string seems not to be easily used, any better idea?

Copy link

Choose a reason for hiding this comment

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

one could use a cmake option ... ;D

Copy link
Member Author

Choose a reason for hiding this comment

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

that could be a solution, but I noticed what protobuf did the same thing actually brought us some trouble to add some extra code to switch the flag back and forth, especially when ort-extensions was used as cmake sub-directory for another project.

DCMAKE_MSVC_RUNTIME_LIBRARY is documented feature from cmake and it's kind of a standard that we should follow.

Copy link
Contributor

Choose a reason for hiding this comment

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

we could perhaps add a magic string to replace "MultiThreaded$<$CONFIG:Debug:Debug>" so it is simple for the user when passing the parameter for DCMAKE_MSVC_RUNTIME_LIBRARY to build.bat - and then we can parse it in the YAML file.

cd out/Windows
ctest -C RelWithDebInfo --output-on-failure
displayName: build and test ort-extensions with VC static runtime.

################
# Windows Python
################
Expand Down
6 changes: 5 additions & 1 deletion cmake/externals/sentencepieceproject.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ if(NOT _ONNXRUNTIME_EMBEDDED)
set(protobuf_BUILD_PROTOC_BINARIES OFF CACHE BOOL "")
endif()
set(protobuf_BUILD_SHARED_LIBS OFF CACHE BOOL "")
set(protobuf_MSVC_STATIC_RUNTIME OFF CACHE BOOL "")
if("${CMAKE_MSVC_RUNTIME_LIBRARY}" STREQUAL "" OR "${CMAKE_MSVC_RUNTIME_LIBRARY}" MATCHES "DLL$")
set(protobuf_MSVC_STATIC_RUNTIME OFF CACHE BOOL "")
else()
set(protobuf_MSVC_STATIC_RUNTIME ON CACHE BOOL "")
endif()
set(protobuf_DISABLE_RTTI ON CACHE BOOL "Disable RTTI")
FetchContent_GetProperties(protobuf)
if(NOT protobuf_POPULATED)
Expand Down
6 changes: 5 additions & 1 deletion docs/development.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,12 @@ Use `./tools/ios/build_xcframework.py` to build an iOS xcframework package.
## Web-Assembly
ONNXRuntime-Extensions will be built as a static library and linked with ONNXRuntime due to the lack of a good dynamic linking mechanism in WASM. Here are two additional arguments [–-use_extensions and --extensions_overridden_path](https://github.com/microsoft/onnxruntime/blob/860ba8820b72d13a61f0d08b915cd433b738ffdc/tools/ci_build/build.py#L416) on building onnxruntime to include ONNXRuntime-Extensions footprint in the ONNXRuntime package.

## The C++ share library
## The C++ shared library
for any other cases, please run `build.bat` or `bash ./build.sh` to build the library. By default, the DLL or the library will be generated in the directory `out/<OS>/<FLAVOR>`. There is a unit test to help verify the build.


**VC Runtime static linkage**
If you want to build the binary with VC Runtime static linkage, please add a parameter _-DCMAKE_MSVC_RUNTIME_LIBRARY="MultiThreaded$<$<CONFIG:Debug>:Debug>"_ on running build.bat

## Copyright guidance
check this link https://docs.opensource.microsoft.com/releasing/general-guidance/copyright-headers/ for source file copyright header.