Skip to content

Update CMakeLists.txt #2

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

Merged
merged 2 commits into from
May 22, 2023
Merged

Update CMakeLists.txt #2

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b0fa8c
Update CMakeLists.txt
songkq May 22, 2023
64aefe3
Update CMakeLists.txt
songkq May 22, 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
7 changes: 5 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ set(TOKENIZERS_CPP_CARGO_TARGET "")

# extra link libraries
set(TOKENIZERS_CPP_LINK_LIBS "")
set(TOKENIZERS_C_LINK_LIBS "")
set(CARGO_EXTRA_ENVS "")
message(STATUS "system-name" ${CMAKE_SYSTEM_NAME})

if (CMAKE_SYSTEM_NAME STREQUAL "Emscripten")
if (CMAKE_SYSTEM_NAME STREQUAL "Linux")
list(APPEND TOKENIZERS_C_LINK_LIBS ${CMAKE_DL_LIBS})
elseif (CMAKE_SYSTEM_NAME STREQUAL "Emscripten")
set(TOKENIZERS_CPP_CARGO_TARGET wasm32-unknown-emscripten)
elseif (CMAKE_SYSTEM_NAME STREQUAL "iOS")
set(TOKENIZERS_CPP_CARGO_TARGET aarch64-apple-ios)
Expand Down Expand Up @@ -107,7 +110,7 @@ endif()
add_subdirectory(sentencepiece sentencepiece EXCLUDE_FROM_ALL)

add_library(tokenizers_c INTERFACE ${TOKENIZERS_RUST_LIB})
target_link_libraries(tokenizers_c INTERFACE ${TOKENIZERS_RUST_LIB})
target_link_libraries(tokenizers_c INTERFACE ${TOKENIZERS_RUST_LIB} ${TOKENIZERS_C_LINK_LIBS})
Copy link
Contributor

@tqchen tqchen May 22, 2023

Choose a reason for hiding this comment

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

actually we can rename TOKENIZERS_CPP_LINK_LIBS to TOKENIZERS_C_LINK_LIBS as they are all intended to fix the rust lib deps

Copy link
Contributor Author

@songkq songkq May 22, 2023

Choose a reason for hiding this comment

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

Yeah, TOKENIZERS_CPP_LINK_LIBS can be replaced with TOKENIZERS_C_LINK_LIBS in this case. I think the TOKENIZERS_CPP_LINK_LIBS maybe also kept empty for extension of some other potential cpp extra libraries.

Copy link
Contributor

@tqchen tqchen May 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

What I meant is that we should move some of the TOKENIZERS_CPP_LINK_LIBS deps to TOKENIZERS_C_LINK_LIBS as they are related to C lib really

https://github.com/mlc-ai/tokenizers-cpp/blob/main/CMakeLists.txt#L20

and here
https://github.com/mlc-ai/tokenizers-cpp/blob/main/CMakeLists.txt#L37


add_library(tokenizers_cpp STATIC $<TARGET_OBJECTS:tokenizer_cpp_objs>)
target_link_libraries(tokenizers_cpp PRIVATE tokenizers_c sentencepiece-static ${TOKENIZERS_CPP_LINK_LIBS})
Expand Down