search

AppImageCommunity / zsync2

Rewrite of https://github.com/AppImage/zsync-curl, using modern C++, providing both a library and standalone tools.
Other
132 stars 25 forks source link

Make linking to custom zlib private in libzsync #54

Closed lgisler closed 3 years ago

lgisler commented 3 years ago

I encountered compile errors in zsync2/src/zsclient.cpp due to lib/zlib/zlib.h being included I'm assuming incorrectly since the building instructions suggest installing zlib1g-dev and zlib/zlib.h is missing the gzwrite function (see zlib/zlib.h:980).

TheAssassin commented 3 years ago

Looks good to me. But we'll have to see if it breaks AppImageUpdate.

I suppose librcksum is also a candidate for private linking?

lgisler commented 3 years ago

The rcksum.h is included in src/zsmake.cpp:12 I was unable to find an apt package that provides rcksum.h so perhaps it is best to leave it public.

TheAssassin commented 3 years ago

I was unable to find an apt package that provides rcksum.h so perhaps it is best to leave it public.

It's included in a source file, but not elsewhere, so marking it PRIVATE will cause CMake to still add the -I to the compilation of the source unit, but not expose it to other targets which target_link_library(<target> ... zsync2). Therefore it shouldn't hurt.

lgisler commented 3 years ago

Making the linking of librcksum PRIVATE required adding librcksum to ZSYNC_DEPS explicitly. I have attached a git diff of the change. If this is preferred I can amend the commit in this PR.

private_librcksum.diff.txt

TheAssassin commented 3 years ago

No, it's fine as-is. Thanks!