Skip to content

Using MZ_COMPAT doesn't match old Minizip output #861

Open
@AndrewAtAvenza

Description

@AndrewAtAvenza

Platform: Windows (but compiling for Android with git-bash)
Compiler: clang++ w/Ninja via CMake
Versions: minizip-ng 4.0.9 and zlib-ng 2.2.4

I've been updating our build system to use zlib-ng & minizip-ng and while I started by trying to use the non-compatible versions, I quickly realized I was going to be stuck with the backwards compatible APIs for a while until some dependencies update. So I switched and started directing all my dependency builds to point to the -ng pair.

I've run into a couple of strange problems though.

  1. When using MZ_COMPAT, crypt.h isn't copied into the output folder from compat
  2. The find_package helpers don't have the correct minizip include path; they include minizip in the include path when they should be pointing at the folder below that (e.g. /install/location/include vs /install/location/include/minizip)

(1) I think this just requires

    list(APPEND MINIZIP_HDR
        compat/ioapi.h
        compat/unzip.h
        compat/zip.h)

to become

    list(APPEND MINIZIP_HDR
        compat/crypt.h
        compat/ioapi.h
        compat/unzip.h
        compat/zip.h)

I don't see a downside to this change, though it's possible there's some setting somewhere that would cause that header to included and I've somehow missed it.

(2) I checked the output of the original minizip and it's package helper has this block in it:

set_target_properties(MINIZIP::minizip PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
  ...
)

whereas the package helper from minizip-ng writes out this block:

set_target_properties(MINIZIP::minizip PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/minizip;${_IMPORT_PREFIX}/include/minizip"
  ...
)

I'm not sure why it repeats it twice, but if you change

    install(TARGETS ${MINIZIP_TARGET} ${MINIZIP_DEP}
            EXPORT ${MINIZIP_TARGET}
            INCLUDES DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/${MINIZIP_TARGET}"
            RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}"
            ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}"
            LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}")

to

    install(TARGETS ${MINIZIP_TARGET} ${MINIZIP_DEP}
            EXPORT ${MINIZIP_TARGET}
            INCLUDES DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
            RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}"
            ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}"
            LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}")

you get this improved output:

set_target_properties(MINIZIP::minizip PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/minizip;${_IMPORT_PREFIX}/include"
  ...
)

Still not sure where the first one is coming from, but this would seem to cover both cases if they were important I think.

I'm going to make these changes on my private copy of the repo but I think these changes are worth considering, unless of course I'm doing something wrong. I don't think I am though, my build is pretty generic (variables point to local builds of dependencies):

   -DMZ_COMPAT=ON
   -DMZ_FETCH_LIBS=OFF
   -DMZ_OPENSSL=ON
   -DOPENSSL_ROOT_DIR=$openssl_root_dir
   -DOPENSSL_INCLUDE_DIR=$openssl_root_dir/include
   -DOPENSSL_CRYPTO_LIBRARY=$openssl_root_dir/lib/libcrypto.a
   -DOPENSSL_SSL_LIBRARY=$openssl_root_dir/lib/libssl.a
   -DCMAKE_FIND_ROOT_PATH=$zlib_root_dir

If you want a PR I can whip one up pretty easily, these are small changes but I thought I'd check first I wasn't missing something or that there was some ramification of these that I haven't noticed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    build systemBuild system and script changescompilationIssues related to compiling source code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions