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

Is libyaml required as a submodule? #56

Open
tocic opened this issue Nov 16, 2024 · 4 comments
Open

Is libyaml required as a submodule? #56

tocic opened this issue Nov 16, 2024 · 4 comments
Labels
packaging Supporting new platforms, packaging, installation.

Comments

@tocic
Copy link

tocic commented Nov 16, 2024

I see there's a submodule for libyaml. But when I uninstall its global package and expect the submodule to be used, I actually get:

/tmp/makepkg/doxide/src/doxide-0.7.0/src/doxide.hpp:19:10: fatal error: yaml.h: No such file or directory
   19 | #include <yaml.h>

Should the submodule be removed (so that git submodule update --recursive wouldn't download an extra dep) or this config changed to reference contrib/libyaml?

@lawmurray lawmurray added the packaging Supporting new platforms, packaging, installation. label Nov 16, 2024
@lawmurray
Copy link
Owner

Thanks @tocic, is this causing a mess on the AUR package for you? Appreciate you maintaining that and would like to make it as easy as possible for you!

On Linux and Mac it's not necessary to have libyaml as a submodule, nor CLI11 for that matter, as these are widely available in Linux distributions and Homebrew on Mac and it is better to be using those versions as dependencies in my opinion. The issue is Windows, specifically users building from source on Windows, where it is convenient to have those dependencies to hand, otherwise they need to be built manually.

Suggestions (from anyone) would be welcome for tidying this up. Something like vcpkg on Windows could be useful, or perhaps CMake has some magic, or perhaps we can just assume that Windows users are installing from Chocolatey or the installer.exe and not from source, and bake the libyaml and CLI11 dependencies into the packaging pipeline instead.

@tocic
Copy link
Author

tocic commented Nov 17, 2024

Maybe something like find_package -> if (WIN32 AND NOT YAML_CPP_LIBRARIES) -> FetchContent/CPMAddPackage/submodule in CMake would help. Alternatively, everyone may use contrib/libyaml directly without an external dependency. In Arch specifically there is a package for every submodule except tree-sitter-cuda (but I can package it separately), so it would be possible to get rid of submodules entirely if CMakeLists.txt supported it (check for an existing package and only if not present use a submodule).

But the problem is not critical, it only causes one useless git clone in case you don't want to manually specify all the required submodules.

@lawmurray
Copy link
Owner

I like that approach @tocic, as in using find_package but falling back to the submodules if necessary. That would address the two priorities of using distribution-provided packages where available on Linux, but making source-based installation easier on Windows. It would even allow Windows users to use Conan or vcpkg or the like to manage dependencies (I am not so familiar with these, but I believe they work with find_package).

Any chance you have some bandwidth to put together a pull request with necessary updates to CMakeLists.txt?

@lawmurray
Copy link
Owner

#64 adds some logic to check whether the submodule is required. Will keep this issue open until I've run some more checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packaging Supporting new platforms, packaging, installation.
Projects
None yet
Development

No branches or pull requests

2 participants