Skip to content
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

szip: set cpp_info.name for cmake_find_package[_multi] #1088

Merged
merged 1 commit into from
Mar 30, 2020
Merged

szip: set cpp_info.name for cmake_find_package[_multi] #1088

merged 1 commit into from
Mar 30, 2020

Conversation

Morwenn
Copy link
Contributor

@Morwenn Morwenn commented Mar 10, 2020

find_package(SZIP) is the usual CMake way to locate szip, which seems required to implement #1072

Specify library name and version: szip/2.1.1

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

`find_package(SZIP)` is the usual CMake way to locate szip, which seems required to implement #1072
@conan-center-bot
Copy link
Collaborator

Some configurations of 'szip/2.1.1' failed in build 1 (837a43bcfa1d662ed6a561d2c9883237d96f63e9):

@Morwenn
Copy link
Contributor Author

Morwenn commented Mar 10, 2020

Timeout issue when downloading the sources, I'll try again later.

@uilianries uilianries closed this Mar 10, 2020
@uilianries uilianries reopened this Mar 10, 2020
@conan-center-bot
Copy link
Collaborator

All green in build 2 (837a43bcfa1d662ed6a561d2c9883237d96f63e9)! 😊

@uilianries
Copy link
Member

The szip project has szip-config.cmake, but on the other hand, the target there in the file is upper case. It sounds be a problem for conan-io/conan#5090

@Morwenn
Copy link
Contributor Author

Morwenn commented Mar 10, 2020

Oh wow, I didn't even think it was possible to have different names for the file and the find_package name, how come every project I could find searches it with SZIP if the file name is lowercase (including HDF5, which is developed by the same group)? If there is a difference, I'd say that we care more about find_package that about the actual file name since we would then replace it with the SZIPConfig.cmake or FindSZIP.cmake generated by the Conan generators (which would also produce an uppercase target).

@Morwenn
Copy link
Contributor Author

Morwenn commented Mar 14, 2020

@SpaceIm what's your take on this one? I know that you worked around it in the HDF recipes, but what do you generally think about it from a cmake_find_package consumer POV?

@danimtb danimtb requested review from SSE4 and uilianries March 26, 2020 17:35
@danimtb
Copy link
Member

danimtb commented Mar 26, 2020

WDYT about this @SpaceIm? Will this be something useful for the hdf5 recipe?

@SpaceIm
Copy link
Contributor

SpaceIm commented Mar 26, 2020

Here is how hdf4 or hdf5 find SZIP:

  if (NOT SZIP_USE_EXTERNAL)
    find_package (SZIP NAMES ${SZIP_PACKAGE_NAME}${HDF_PACKAGE_EXT} COMPONENTS static shared)
    if (NOT SZIP_FOUND)
      find_package (SZIP) # Legacy find
      if (SZIP_FOUND)
        set (LINK_COMP_LIBS ${LINK_COMP_LIBS} ${SZIP_LIBRARIES})
        set (LINK_COMP_SHARED_LIBS ${LINK_COMP_SHARED_LIBS} ${SZIP_LIBRARIES})
      endif ()
    endif ()
  endif ()
  if (SZIP_FOUND)
    set (H5_HAVE_FILTER_SZIP 1)
    set (H5_HAVE_SZLIB_H 1)
    set (H5_HAVE_LIBSZ 1)
    set (SZIP_INCLUDE_DIR_GEN ${SZIP_INCLUDE_DIR})
    set (SZIP_INCLUDE_DIRS ${SZIP_INCLUDE_DIRS} ${SZIP_INCLUDE_DIR})
  else ()
    if (HDF5_ALLOW_EXTERNAL_SUPPORT MATCHES "GIT" OR HDF5_ALLOW_EXTERNAL_SUPPORT MATCHES "TGZ")
      EXTERNAL_SZIP_LIBRARY (${HDF5_ALLOW_EXTERNAL_SUPPORT} ${HDF5_ENABLE_SZIP_ENCODING})
      set (H5_HAVE_FILTER_SZIP 1)
      set (H5_HAVE_SZLIB_H 1)
      set (H5_HAVE_LIBSZ 1)
      message (STATUS "Filter SZIP is built")
    else ()
      message (FATAL_ERROR "SZIP is Required for SZIP support in HDF5")
    endif ()
  endif ()
  if (BUILD_SHARED_LIBS)
    set (LINK_COMP_SHARED_LIBS ${LINK_COMP_SHARED_LIBS} ${SZIP_SHARED_LIBRARY})
  endif ()
  set (LINK_COMP_LIBS ${LINK_COMP_LIBS} ${SZIP_STATIC_LIBRARY})
  INCLUDE_DIRECTORIES (${SZIP_INCLUDE_DIRS})

Currently, I replace this with set (LINK_COMP_LIBS ${LINK_COMP_LIBS} "CONAN_PKG::${CONAN_SZIP_LIBNAME}"), and I inject CONAN_SZIP_LIBNAME by either libaec or szip. So yes I guess this PR would be useful, but I would have to manage libaec anyway (I don't know if libaec exports a target, and if it's SZIP).

@danimtb danimtb merged commit da7bbb4 into conan-io:master Mar 30, 2020
@Morwenn Morwenn deleted the patch-2 branch March 30, 2020 08:02
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.

6 participants