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 generation of registry file for pkg-config #151

Merged
merged 1 commit into from
Sep 29, 2024

Conversation

rherilier
Copy link
Contributor

No description provided.

@uilianries
Copy link
Member

@rherilier Thank you for your contribution. Could you please elaborate your need of this PR or point an associated issue? Do you really consume .pc files? What have you doing so far without them?

@rherilier
Copy link
Contributor Author

using pkg-config in Makefile or when communicating a one-line compilation commands for test purpose helps early detection of installed dependency. Using CMake or meson (which can use CMake's registry) for such cases is either irrelevant or not possible.

I admit this MR is not essential for my part.

@uilianries
Copy link
Member

@rherilier Thank you for your answer. I'll take a look during the weekend to add a new validation using CMake + pkg-config, so we will be good to go.

@rherilier rherilier force-pushed the add-registry-file-for-pkg-config branch from 6d6d95f to 769d57e Compare September 23, 2024 14:53
@rherilier
Copy link
Contributor Author

I simply fixed some typo.

@rherilier rherilier force-pushed the add-registry-file-for-pkg-config branch from 769d57e to 063327e Compare September 23, 2024 15:00
@rherilier
Copy link
Contributor Author

sorry... I did a stupid fix

@rherilier rherilier force-pushed the add-registry-file-for-pkg-config branch from 063327e to 5ce428b Compare September 23, 2024 15:09
@rherilier
Copy link
Contributor Author

sorry for the garbage... the installation directory is pkgconfig but the program is pkg-config...

@rherilier rherilier force-pushed the add-registry-file-for-pkg-config branch from 5ce428b to 651570a Compare September 24, 2024 17:28
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested locally. Thank you for your PR!

$ cmake -S . -B build -DCMAKE_INSTALL_PREFIX=/tmp/install -DTAOCPP_JSON_BUILD_TESTS=OFF -DTAOCPP_JSON_BUILD_EXAMPLES=OFF -DTAOCPP_JSON_BUILD_PERFORMANCE=OFF
$ cmake --build build --target install
-- Install configuration: ""
-- Installing: /tmp/install/share/pegtl/cmake/pegtl-config.cmake
-- Installing: /tmp/install/share/pegtl/cmake/pegtl-config-version.cmake
-- Up-to-date: /tmp/install/include
-- Up-to-date: /tmp/install/include/tao
...
 Installing: /tmp/install/share/taocpp-json/cmake/taocpp-json-config.cmake
-- Installing: /tmp/install/share/taocpp-json/cmake/taocpp-json-config-version.cmake
-- Installing: /tmp/install/share/pkgconfig/taocpp-json.pc
-- Installing: /tmp/install/share/taocpp-json/cmake/taocpp-json-targets.cmake
...

$ PKG_CONFIG_PATH=/tmp/install/share/pkgconfig pkg-config taocpp-json --cflags 
-I/tmp/install/include

@uilianries uilianries self-assigned this Sep 28, 2024
@rherilier
Copy link
Contributor Author

If I read well, the workflows for android run on Ubuntu 22.04 which includes CMake 3.22 and HOMEPAGE_URL exists since 3.12...

@uilianries
Copy link
Member

@rherilier No, it's running 3.10:

https://github.com/taocpp/json/actions/runs/11018882661/job/30798933170?pr=151#step:4:12

CMake Error: Could not find cmake module file: /home/developer/build/CMakeFiles/3.10.2/CMakeHOMEPAGE_URLCompiler.cmake

I would ask you reducing your PR to only install the .pc file and its configuration. Nothing more.

The CI looks broken, so I don't want to mix your PR or even block it due that error, so let's be minimal here. Later, I'll revisit to fix the CI and re-introduce that CMake feature.

Thank you.

@rherilier
Copy link
Contributor Author

@rherilier Thank you for the explanation.

I'll fix it.

@rherilier rherilier force-pushed the add-registry-file-for-pkg-config branch from 651570a to 54d3441 Compare September 29, 2024 07:40
@uilianries uilianries merged commit fe1a60b into taocpp:main Sep 29, 2024
51 of 60 checks passed
@uilianries
Copy link
Member

@rherilier Merged. Thank you again!

@rherilier
Copy link
Contributor Author

@uilianries you're welcome ;)

@rherilier rherilier deleted the add-registry-file-for-pkg-config branch September 29, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants