Skip to content

add pkgconfig feature - make static builds possible #2

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dragonnn
Copy link

@dragonnn dragonnn commented Mar 4, 2022

Hi!
Thanks for this nice crate, does exactly what I need in my project where I need to deal with bad/broken xmls from a legacy software.
I often use the musl target in my projects so the binary can be easy moved across different linux OS-es so I add in this PR a option to build tidy into tidy-sys static. To keep the old option I add a feature pkg-config with enables linking against the system library. Took some inspiration from the zstd crate. If you wish to keep the old default I can do that, just adding pkg-config to default features.
BTW did you consider publishing it on crates.io? I know this a "pet" project but I saw far worse pet projects on crates.io :D, in my use case this was a god bless :).

@terminalstatic
Copy link
Owner

Hi, thanks for your feedback and I especially enjoyed the last paragraph, I pretty much agree to that in any language/lib manager, esp. when it comes to ffi :). As for my motivation for not pushing it to to crates.io I was reluctant because I didn‘t want to contribute to „wrapper packages cluttering.“ If I‘d have done a „native“ lib I would probably have pushed it to the registry.
Another reason is that I actually rarely write in rust (fun language, really like it but too many loc for my taste and too little benefits to compensate for this to be my „go to“ language (pun) ;)) and therefore don‘t really follow the languages progression regularly.

And as it‘s pretty straight forward to invoke github code in a rust project I felt that anybody who needs it can easily make use of it.

Btw, as you can tell from the example my use case was exactly the same as yours :).

Last not least, thanks for the pull request, didn‘t check it yet but as I said at the beginning liked your comment and wanted to post a quick reply :).

@terminalstatic
Copy link
Owner

terminalstatic commented Mar 6, 2022

Tbc, as I just notice quite painfully myself, it's also rather tricky and quite an effort to reliably support cross platform compatibility as it currently doesn't build on mac at all (a general challenge with ffi). What I also didn't mention is to keep track of changes in the underlying library, e.g. api/filename changes.

@terminalstatic
Copy link
Owner

terminalstatic commented Mar 6, 2022

Unfortunately can't merge this as it is, without changes (removing pkg-config from features) this won't build (now on ubuntu). After successful musl build binary throws segfault. Also there's a quite some preliminary stuff to do which would need documentation (cloning tidy-html5 for example, would be probably even better to do this programmatically). I'd probably also have to look a little bit into musl to understand what's going on. Default target then stops working aswell, complains about missing static library, didn't look into this further.

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