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.

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

Using systemwide duktape. (and other systemwide deps) #870

Open
shamefulCake1 opened this issue Feb 26, 2025 · 3 comments
Open

Using systemwide duktape. (and other systemwide deps) #870

shamefulCake1 opened this issue Feb 26, 2025 · 3 comments

Comments

@shamefulCake1
Copy link

Dear mrdocs developers,

duktape can be installed systemwide as a shared library, and when it is, bundling an unnecessary copy is superflous.

Please, consider adding support for a systemwide duktape, using, for example

find_package(PkgConfig REQUIRED)
pkg_check_modules(duktape REQUIRED IMPORTED_TARGET duktape)
target_link_libraries(my_executable PRIVATE PkgConfig::duktape)
@alandefreitas
Copy link
Collaborator

duktape is not bundled:

mrdocs/CMakeLists.txt

Lines 126 to 131 in da3305b

find_package(Duktape CONFIG)
if (NOT DUKTAPE_FOUND)
# Duktape doesn't nativelly support CMake.
# Some config script patches use the capitalized version.
find_package(duktape REQUIRED CONFIG)
endif()

It uses find_package for duktape.

What mrdocs bundles is CMake scripts to install and find duktape. Internally, these scripts do basically what you suggest:

pkg_check_modules(PC_DUK QUIET duktape libduktape)

@shamefulCake1
Copy link
Author

shamefulCake1 commented Feb 27, 2025

But that's an "in" file, not a proper duktapeConfig.cmake, so it is not seen by the top-level CMakeLists.txt, so find_package(Duktape CONFIG) fails not because duktape is not installed, but because there is no corresponding duktapeConfig.cmake

@alandefreitas
Copy link
Collaborator

But that's an "in" file

Yes. I agree. I'm just pointing out there's no superfluous copy of duktape.

The reason for the "in" file is a different issue. When you install duktape with vcpkg, you use find_package, so we want to support this use case.

I don't know if the logic you provided would already cover this use case or duktape installed directly with the cmake file provided. My guess is it would fail at find_package(PkgConfig REQUIRED), which is not required in other use cases and rarely available on Windows.

So that PkgConfig logic would need to come after the find_package logic. In addition to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants