We've already added ZLIB_LIBRARIES to LIBGIT2_LIBS so don't also add the z library#5080
Conversation
…add the `z` library (libgit2/libgit2#5079).
pks-t
left a comment
There was a problem hiding this comment.
Thanks for your PR! We definitely do this wrong right now, but removing this whole block altogether is wrong, too.
| LIST(APPEND LIBGIT2_LIBS ${ZLIB_LIBRARIES}) | ||
| IF(APPLE OR CMAKE_SYSTEM_NAME MATCHES "FreeBSD") | ||
| LIST(APPEND LIBGIT2_LIBS "z") | ||
| LIST(APPEND LIBGIT2_PC_LIBS "-lz") |
There was a problem hiding this comment.
I agree that the first line is definitely bogus, we don't want to append twice to LIBGIT2_LIBS. But appending to LIBGIT2_PC_LIBS is in fact required. Doing so will not cause us to link twice, but the PC_LIBS variable will cause us to append this lib to the generated pkgconfig file.
There was a problem hiding this comment.
Agree -- updates committed.
| LIST(APPEND LIBGIT2_PC_LIBS "-lz") | ||
| ELSE() | ||
| LIST(APPEND LIBGIT2_PC_REQUIRES "zlib") | ||
| ENDIF() |
There was a problem hiding this comment.
Sorry to be a nag, but there's some leading spaces now in front of the tabs. If you convert that to all-tabs then we're ready to go!
There was a problem hiding this comment.
Tabs versus spaces can be a pain! Hopefully all is now good.
pks-t
left a comment
There was a problem hiding this comment.
Certainly looks good to me now! Thanks a lot for fixing this.
This resolves issue #5079.