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

Do not expose mustache (and other depedency/tools) in public headers. #564

Closed
mgautierfr opened this issue Jun 24, 2021 · 2 comments · Fixed by #574
Closed

Do not expose mustache (and other depedency/tools) in public headers. #564

mgautierfr opened this issue Jun 24, 2021 · 2 comments · Fixed by #574
Assignees
Labels
Milestone

Comments

@mgautierfr
Copy link
Member

The public header otherTools.h include mustache.h (https://github.com/kiwix/libkiwix/blob/master/include/tools/otherTools.h#L27).
By doing thing, we defacto make mustache a direct dependency of projects using libkiwix.
If somehow otherTools.h is include by a project foo. The mustache header must be present even if foo never use mustache.
This is what happen here https://github.com/kiwix/kiwix-desktop/runs/2904840393?check_suite_focus=true (PR kiwix/kiwix-desktop#656) where the compilation of kiwix-desktop fails as mustache.hpp is not present.

We must not include header of our private dependencies in public our headers.

More than just cleaning the mustache inclusion (and other tools, we also include zim.h and we should not), we should probably carrefully select what helper function are part of our API and move all other tools private.

@legoktm
Copy link
Member

legoktm commented Jun 28, 2021

More than just cleaning the mustache inclusion (and other tools, we also include zim.h and we should not),

In the Debian package we currently depend on:

 libzim-dev (>= 6.0.0),
 libicu-dev,
 libpugixml-dev,
 libcurl4-gnutls-dev,
 libmicrohttpd-dev

since those are exposed as part of the public API. These are also listed in kiwix.pc.

@mgautierfr
Copy link
Member Author

libzim will become a dev dependency.
But for other projects, we should remove them from the public api. (At least not include the headers.)

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

Successfully merging a pull request may close this issue.

2 participants