Skip to content

Fixes in the CMake install setup for EXPORT #103

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

swolchok
Copy link

@swolchok swolchok commented Jul 16, 2025

These changes seem necessary to support pytorch/executorch#8954 .

  • lib64 is not consistently used; usually it's lib, and CMAKE_INSTALL_LIBDIR is the right way to get the name
  • the link options for regex_lookahead are already exported; don't need to do them again in our manual config.cmake
  • had to unbreak the macros we use to set the link options so that they work when exported, though.

@swolchok swolchok requested a review from larryliu0820 July 16, 2025 20:44
@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 16, 2025
@swolchok
Copy link
Author

swolchok commented Jul 16, 2025

CMake Error at CMakeLists.txt:45 (target_link_libraries):
Error evaluating generator expression:

$<TARGET_FILE:tokenizers::regex_lookahead>

No target "tokenizers::regex_lookahead"

Hmm. The problem is that after install, the target is namespaced, but before install, it's not. I'll have to look into what we're supposed to do

@swolchok swolchok marked this pull request as draft July 16, 2025 21:05
- lib64 is not consistently used; usually it's lib, and CMAKE_INSTALL_LIBDIR is the right way to get the name
- the link options for regex_lookahead are already exported; don't need to do them again in our manual config.cmake
- had to unbreak the macros we use to set the link options so that they work when exported, though
@swolchok
Copy link
Author

added namespace-prefixed aliases for the tokenizers targets for internal use

@swolchok swolchok marked this pull request as ready for review July 16, 2025 21:47
@swolchok
Copy link
Author

not sure if the existing test failure is a real problem. looks like labs doesn't have HUD? https://github.com/pytorch-labs/tokenizers/actions/runs/16330717524/job/46132342175?pr=103#step:15:1315

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.

1 participant