Skip to content

Enable install find package #82

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 4 commits into from
Jun 12, 2025
Merged

Conversation

larryliu0820
Copy link
Contributor

@larryliu0820 larryliu0820 commented Jun 10, 2025

This pull request introduces several enhancements to the CMakeLists.txt file and adds a new configuration file for the tokenizers package. The changes focus on improving build system support, adding installation rules, and providing a package configuration file for easier integration with other projects.

Build system improvements:

  • CMakeLists.txt: Included CMakePackageConfigHelpers to enable the use of configure_package_config_file for generating CMake configuration files.
  • CMakeLists.txt: Updated target_include_directories to use $<BUILD_INTERFACE> for better handling of include directories during the build process.

Installation rules:

  • CMakeLists.txt: Added installation rules for the library, headers, and CMake configuration files, including tokenizers-targets.cmake and tokenizers-config.cmake. These rules ensure the library and its dependencies are correctly installed.

Package configuration:

  • cmake/tokenizers-config.cmake.in: Added a new configuration file for the tokenizers package, defining variables such as TOKENIZERS_FOUND, TOKENIZERS_INCLUDE_DIRS, and TOKENIZERS_LIBRARIES. It includes dependency checks and ensures the sentencepiece library is available.

For any CMake project we should be able to do the following:

# add tokenizers
ExternalProject_Add(
  tokenizers_external_project
  PREFIX ${CMAKE_CURRENT_BINARY_DIR}/../tokenizers
  SOURCE_DIR ${EXECUTORCH_ROOT}/extension/llm/tokenizers
  CMAKE_ARGS -DSUPPORT_REGEX_LOOKAHEAD=ON -DCMAKE_INSTALL_PREFIX=${CMAKE_CURRENT_BINARY_DIR}/../tokenizers
  INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/../tokenizers
)
add_dependencies(extension_llm_runner tokenizers_external_project)
find_package(tokenizers CONFIG HINTS ${CMAKE_INSTALL_PREFIX} ${CMAKE_CURRENT_BINARY_DIR}/../tokenizers)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jun 10, 2025
Copy link
Contributor

@jackzhxng jackzhxng left a comment

Choose a reason for hiding this comment

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

Excited to use ExternalProject_Add, I assume you will be refactoring ET to use this as a follow up?


install(
DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/third-party/json/single_include/
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this single include also be found in ${CMAKE_CURRENT_SOURCE_DIR}/include/? Then you can update the pattern to also accept *.hpp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not in ${CMAKE_CURRENT_SOURCE_DIR}/include/

Allowing parent repo to be able to cmake --target install and then
find_package(tokenizers CONFIG)

This comes in handy when we want to build tokenizers as an external
project or rely on FetchContent.
@larryliu0820 larryliu0820 force-pushed the enable_install_find_package branch from a2bac3b to 9aea7ff Compare June 12, 2025 21:11
@larryliu0820 larryliu0820 merged commit 3d67b29 into main Jun 12, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants