Skip to content

Conversation

@lgisler
Copy link
Contributor

@lgisler lgisler commented Feb 10, 2021

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
Copy link
Member

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
Copy link
Contributor Author

lgisler commented Feb 10, 2021

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.

@lgisler lgisler force-pushed the linking-issue-between-system-zlib_h-and-custom-zlib_h branch from cea41f4 to 34f5f81 Compare February 11, 2021 10:24
@TheAssassin
Copy link
Member

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
Copy link
Contributor Author

lgisler commented Feb 12, 2021

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
Copy link
Member

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

@TheAssassin TheAssassin merged commit 486fc9a into AppImageCommunity:master Feb 12, 2021
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