Change defaults and improve CMake export #26
Conversation
CMakeLists.txt
Outdated
| @@ -56,7 +56,7 @@ find_package(PCRE2 REQUIRED) | |||
| if(USE_BUILTIN_SENTENCEPIECE) | |||
| list(APPEND SLIMT_PUBLIC_LIBS SentencePiece::SentencePiece) | |||
| else(USE_BUILTIN_SENTENCEPIECE) | |||
| list(APPEND SLIMT_PRIVATE_LIBS SentencePiece::SentencePiece) | |||
| list(APPEND SLIMT_PUBLIC_LIBS SentencePiece::SentencePiece) | |||
There was a problem hiding this comment.
As in if and else we have the same code for SentencePiece::SentencePiece
we can change code as
list(APPEND SLIMT_PUBLIC_LIBS SentencePiece::SentencePiece)
if (NOT USE_BUILTIN_SENTENCEPIECE)
list(APPEND SLIMT_PRIVATE_LIBS SentencePiece::Protobuf)
endif()
| @@ -116,7 +115,7 @@ if(EXPORT_CMAKE_FILE) | |||
|
|
|||
| # This will double install, but is-ok. | |||
| install( | |||
| TARGETS slimt-shared slimt-static | |||
| TARGETS slimt-shared slimt-static sentencepiece | |||
There was a problem hiding this comment.
Is it work if we don't sentencepiece ? (if we have it install in system)
There was a problem hiding this comment.
This entire block will only work if it is system sentencepiece (i.e installed via libsentencepiece-dev in debian based machines, sentencepiece-git maybe from ArchLinux AUR). For distributions where this is not available, the recommendation is to independently (of slimt) install sentencepiece from sources. If it's sentencepiece from sources supplied here via submodule (USE_BUILTIN_SENTENCEPIECE=ON), the code will fail at configure step.
sentencepiece only provides the pkg-config based sentencepiece.pc.in. This is used in our builds to induct the
sentencpiece target (a.k.a Sentencepiece::Sentencepiece). This happens here in source.
What we are doing here is merely exposing this target as slimt::sentencepiece, as a dependency for slimt::slimt-shared. i.e, it refers to the already installed sentencepiece from elsewhere. This target for usage cmake's packaging is not provided by the original source (only pkg-config), and since we are providing it as slimt::sentencepiece it should not be an issue.
You can look at the source of the generated ${CMAKE_INSTALL_PREFIX}/lib/cmake/slimt/slimtTargets.cmake to see how this is laid out in a real installation. On my installation, it looks like this.
I'm no expert, just thinking out loud here. I'm experimenting with CMakeLists.txt and figuring out where these things go.
jerinphilip
force-pushed
the
kde-drive
branch
12 times, most recently
from
c9997da to
8db14eb
Compare
Fix broken CI due to actions-runner. Manually install llvm-17 and make it preferred. The upgrade improves static analysis, leading to the following fixes, uniformities: - misc-include-cleaner: Add missing headers, remove unused headers - Use `\n` instead of `std::endl` - Move includes to included impl files - Fixes to HTML.cc - Add explicit `default: break` for switches - Fix use-after-free
jerinphilip
force-pushed
the
kde-drive
branch
from
8db14eb to
a4512de
Compare
jerinphilip
force-pushed
the
kde-drive
branch
from
e62e754 to
e82a441
Compare
jerinphilip
changed the title
No description provided.