8000 Add *.pc file for easier use in non-cmake environments. by hzeller · Pull Request #901 · elalish/manifold · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add *.pc file for easier use in non-cmake environments. #901

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

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

hzeller
Copy link
Contributor
@hzeller hzeller commented Aug 18, 2024

pkg-config is the common way how to find cflags and library flags for projects not using cmake.

Copy link
codecov bot commented Aug 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.26%. Comparing base (d437097) to head (0620985).
Report is 77 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #901      +/-   ##
==========================================
- Coverage   91.84%   87.26%   -4.58%     
==========================================
  Files          37       66      +29     
  Lines        4976     9442    +4466     
  Branches        0     1033    +1033     
==========================================
+ Hits         4570     8240    +3670     
- Misses        406     1202     +796     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hzeller hzeller force-pushed the feature-20240817-add-pkgconfig branch from 1ca1757 to 3c4a712 Compare August 18, 2024 00:38
@pca006132
Copy link
Collaborator

Thanks. Maybe we can add a simple test for *.pc as well? Similar to https://github.com/elalish/manifold/blob/master/test-cmake.sh.

@hzeller hzeller force-pushed the feature-20240817-add-pkgconfig branch from 3c4a712 to 7f77a78 Compare August 18, 2024 16:05
@hzeller
Copy link
Contributor Author
hzeller commented Aug 18, 2024

Done, added test-pkgconfig.sh.

While at it, I discovered an ambiguity that cmake allows, which is that cmake, allows to include manifold.h or manifold/manifold.h even though all include files are in $PREFIX/include/manifold/.

So only manifold/manifold.h is technically valid as include, but the sloppiness of allowing both paths should probably be fixed in the cmake installation. Modern libraries typically have a sub-directory in $PREFIX/include and should only be included as #include <$projectname/foo.h>.

pkg-config is the common way how to find cflags and library flags
for projects not using cmake.
@hzeller hzeller force-pushed the feature-20240817-add-pkgconfig branch from 7f77a78 to 0620985 Compare August 18, 2024 22:21
@pca006132
Copy link
Collaborator

Done, added test-pkgconfig.sh.

While at it, I discovered an ambiguity that cmake allows, which is that cmake, allows to include manifold.h or manifold/manifold.h even though all include files are in $PREFIX/include/manifold/.

So only manifold/manifold.h is technically valid as include, but the sloppiness of allowing both paths should probably be fixed in the cmake installation. Modern libraries typically have a sub-directory in $PREFIX/include and should only be included as #include <$projectname/foo.h>.

Yeah, we were thinking about fixing this in v3.0. We should also fix the include statements in other header files as well, but that may require changing the file structure in our repo, and I am not sure what is the best way of solving this.

Copy link
Collaborator
@pca006132 pca006132 left a comment

Choose a reason for hiding this comment

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

LGTM. Do you want to merge now or still have something else to add?

@hzeller
Copy link
Contributor Author
hzeller commented Aug 19, 2024

Nothing to add right now, ready to merge.

@pca006132 pca006132 merged commit 74e15b1 into elalish:master Aug 19, 2024
22 checks passed
@elalish elalish mentioned this pull request Nov 5, 2024
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.

4 participants
0