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

Add version flag #64

Merged
merged 2 commits into from
Dec 3, 2024
Merged

Add version flag #64

merged 2 commits into from
Dec 3, 2024

Conversation

jkhoogland
Copy link

Adds a --version,-v flag to show the version of application.

  • version is pulled the cmake variablePROJECT_VERSION set in CMakeLists.txt.

Fixes cmake build on macos; libyaml was not properly included.

@lawmurray
Copy link
Owner

Thanks @jkhoogland, will review.

Linking #56 as related on the libyaml issue. The idea there is to use system packages where available (e.g. libyaml might be installed from Homebrew on macos) and only use the submodules in contrib if they are not available.

@lawmurray
Copy link
Owner

Thanks again for this @jkhoogland . The --version feature works for me, so that's great. The libyaml changes are not working for me on Ubuntu 24.04, however. It's not finding the system-installed libyaml (from the libyaml-dev package) and deferring to the submodule.

If you're in a position to test that change some more on Linux, then I'd appreciate that. If not, can we just remove the libyaml change from this pull request to handle it separately, and I can merge the --version changes?

@jkhoogland
Copy link
Author

From what I can tell both on macos and linux (arch) the standard package for libyaml does not contain any cmake config files for libyaml. So find_package will error out.

I add a switch to only do this on macos if that makes the PR acceptable?

On archlinux:

pacman -Ql libyaml

gives

libyaml /usr/
libyaml /usr/include/
libyaml /usr/include/yaml.h
libyaml /usr/lib/
libyaml /usr/lib/libyaml-0.so.2
libyaml /usr/lib/libyaml-0.so.2.0.9
libyaml /usr/lib/libyaml.so
libyaml /usr/lib/pkgconfig/
libyaml /usr/lib/pkgconfig/yaml-0.1.pc
libyaml /usr/share/
libyaml /usr/share/licenses/
libyaml /usr/share/licenses/libyaml/
libyaml /usr/share/licenses/libyaml/LICENSE

@jkhoogland
Copy link
Author

I made the build of contrib/libyaml optional.

Use -DBUILD_YAML=ON to add the local build of libyaml.

@lawmurray lawmurray merged commit cca378b into lawmurray:main Dec 3, 2024
3 checks passed
@lawmurray
Copy link
Owner

Looks good @jkhoogland, that setup for libyaml seems sensible to me. Thanks for the work on this one and for contributing to Doxide!

Closes #61.

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

Successfully merging this pull request may close these issues.

2 participants