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

Use CMake's find_package to include libdeflate #1588

Closed
bebuch opened this issue Nov 2, 2023 · 7 comments · Fixed by #1613
Closed

Use CMake's find_package to include libdeflate #1588

bebuch opened this issue Nov 2, 2023 · 7 comments · Fixed by #1613

Comments

@bebuch
Copy link

bebuch commented Nov 2, 2023

I'm working on Windows. PkgConfig is not available. I installed imath and libdeflate from the same sources as FetchContent in cmake/OpenEXRSetup.cmake. Imath works without problems, but libdeflate is not recognized although modern CMake config files are installed.

C:/.../usr
│
├───bin
│       deflate.dll
│       deflated.dll
│       deflaterd.dll
│       Imath-3_1.dll
│       Imath-3_1d.dll
│       Imath-3_1rd.dll
│
├───include
│   ├───deflate
│   │       libdeflate.h
│   │
│   └───imath
│       └───Imath
│               half.h
│               halfFunction.h
│               halfLimits.h
│               ImathBox.h
│               ImathBoxAlgo.h
│               ImathColor.h
│               ImathColorAlgo.h
│               ImathConfig.h
│               ImathEuler.h
│               ImathExport.h
│               ImathForward.h
│               ImathFrame.h
│               ImathFrustum.h
│               ImathFrustumTest.h
│               ImathFun.h
│               ImathGL.h
│               ImathGLU.h
│               ImathInt64.h
│               ImathInterval.h
│               ImathLine.h
│               ImathLineAlgo.h
│               ImathMath.h
│               ImathMatrix.h
│               ImathMatrixAlgo.h
│               ImathNamespace.h
│               ImathPlane.h
│               ImathPlatform.h
│               ImathQuat.h
│               ImathRandom.h
│               ImathRoots.h
│               ImathShear.h
│               ImathSphere.h
│               ImathTypeTraits.h
│               ImathVec.h
│               ImathVecAlgo.h
│
└───lib
    │   deflate.lib
    │   deflated.lib
    │   deflaterd.lib
    │   Imath-3_1.lib
    │   Imath-3_1d.lib
    │   Imath-3_1rd.lib
    │
    └───cmake
        ├───Imath
        │       ImathConfig.cmake
        │       ImathConfigVersion.cmake
        │       ImathTargets-debug.cmake
        │       ImathTargets-release.cmake
        │       ImathTargets-relwithdebinfo.cmake
        │       ImathTargets.cmake
        │
        └───libdeflate
                libdeflate-config-version.cmake
                libdeflate-config.cmake
                libdeflate-targets-debug.cmake
                libdeflate-targets-release.cmake
                libdeflate-targets-relwithdebinfo.cmake
                libdeflate-targets.cmake

The PkgConfig approach doesn't work for me because there is no PkgConfig available in my environment. The FetchConfig approach doesn't work for me because there is no Git available.

Therefore, I use the manual deployment of the library. This does not require 3rdparty tools. Unfortunately, this option is currently not implemented. In my opinion this should work out of the box. It would also make the benefits of modern CMake targets available for libdeflate with FetchConfig.

@kmilos
Copy link

kmilos commented Nov 2, 2023

I'm working on Windows. PkgConfig is not available.

Just as a side note - it is in theory via vcpkg (or chocolatey, or MSYS2).

But yeah, agree there is no reason not to try the direct CMake approach for newer libdeflate versions that supply the config files.

@bebuch
Copy link
Author

bebuch commented Nov 2, 2023

I'm in a Docker container with minimal dependencies. ;-)

@bebuch
Copy link
Author

bebuch commented Nov 6, 2023

I have made a patch to 3.1.4 that work for me to link to the shared version of libdeflate:

diff --git a/openexr_original/cmake/OpenEXRSetup.cmake b/openexr/cmake/OpenEXRSetup.cmake
index 425992d..fa80886 100644
--- a/openexr_original/cmake/OpenEXRSetup.cmake
+++ b/openexr/cmake/OpenEXRSetup.cmake
@@ -160,71 +160,79 @@ set(OPENEXR_DEFLATE_TAG "v1.18" CACHE STRING "Tag to use for libdeflate source r
 if(NOT OPENEXR_FORCE_INTERNAL_DEFLATE)
   #TODO: ^^ Release should not clone from main, this is a place holder
   set(CMAKE_IGNORE_PATH "${CMAKE_CURRENT_BINARY_DIR}/_deps/deflate-src/config;${CMAKE_CURRENT_BINARY_DIR}/_deps/deflate-build/config")
-  include(FindPkgConfig)
-  pkg_check_modules(deflate IMPORTED_TARGET GLOBAL libdeflate)
-  set(CMAKE_IGNORE_PATH)
-  if (deflate_FOUND)
-    message(STATUS "Using libdeflate from ${deflate_LINK_LIBRARIES}")
+  find_package(libdeflate CONFIG)
+  if(libdeflate_FOUND)
+    message(STATUS "Using libdeflate from ${libdeflate_DIR}")
+  else()
+    include(FindPkgConfig)
+    pkg_check_modules(deflate IMPORTED_TARGET GLOBAL libdeflate)
+    set(CMAKE_IGNORE_PATH)
+    if (deflate_FOUND)
+      message(STATUS "Using libdeflate from ${deflate_LINK_LIBRARIES}")
+    endif()
   endif()
 endif()
 
-if(NOT TARGET PkgConfig::deflate AND NOT deflate_FOUND)
-  if(OPENEXR_FORCE_INTERNAL_DEFLATE)
-    message(STATUS "libdeflate forced internal, installing from ${OPENEXR_DEFLATE_REPO} (${OPENEXR_DEFLATE_TAG})")
-  else()
-    message(STATUS "libdeflate was not found, installing from ${OPENEXR_DEFLATE_REPO} (${OPENEXR_DEFLATE_TAG})")
-  endif()
-  include(FetchContent)
-  FetchContent_Declare(Deflate
-    GIT_REPOSITORY "${OPENEXR_DEFLATE_REPO}"
-    GIT_TAG "${OPENEXR_DEFLATE_TAG}"
-    GIT_SHALLOW ON
-    )
+if(libdeflate_FOUND)
+  set(EXR_DEFLATE_LIB libdeflate::libdeflate_shared)
+else()
+  if(NOT TARGET PkgConfig::deflate AND NOT deflate_FOUND)
+    if(OPENEXR_FORCE_INTERNAL_DEFLATE)
+      message(STATUS "libdeflate forced internal, installing from ${OPENEXR_DEFLATE_REPO} (${OPENEXR_DEFLATE_TAG})")
+    else()
+      message(STATUS "libdeflate was not found, installing from ${OPENEXR_DEFLATE_REPO} (${OPENEXR_DEFLATE_TAG})")
+    endif()
+    include(FetchContent)
+    FetchContent_Declare(Deflate
+      GIT_REPOSITORY "${OPENEXR_DEFLATE_REPO}"
+      GIT_TAG "${OPENEXR_DEFLATE_TAG}"
+      GIT_SHALLOW ON
+      )
 
-  FetchContent_GetProperties(Deflate)
-  if(NOT Deflate_POPULATED)
-    FetchContent_Populate(Deflate)
-  endif()
+    FetchContent_GetProperties(Deflate)
+    if(NOT Deflate_POPULATED)
+      FetchContent_Populate(Deflate)
+    endif()
 
-  # Rather than actually compile something, just embed the sources
-  # into exrcore. This could in theory cause issues when compiling as
-  # a static library into another application which also uses
-  # libdeflate but we switch the export symbol to hidden which should
-  # hide the symbols when linking...
-  set(EXR_DEFLATE_SOURCES
-    lib/arm/cpu_features.c
-    lib/x86/cpu_features.c
-    lib/utils.c
-    lib/deflate_compress.c
-    lib/deflate_decompress.c
-    lib/adler32.c
-    lib/zlib_compress.c
-    lib/zlib_decompress.c)
-  # don't need these
-  # lib/crc32.c
-  # lib/gzip_compress.c
-  # lib/gzip_decompress.c
-  file(READ ${deflate_SOURCE_DIR}/lib/lib_common.h DEFLATE_HIDE)
-  string(REPLACE "visibility(\"default\")" "visibility(\"hidden\")" DEFLATE_HIDE "${DEFLATE_HIDE}")
-  string(REPLACE "__declspec(dllexport)" "/**/" DEFLATE_HIDE "${DEFLATE_HIDE}")
-  file(WRITE ${deflate_SOURCE_DIR}/lib/lib_common.h "${DEFLATE_HIDE}")
+    # Rather than actually compile something, just embed the sources
+    # into exrcore. This could in theory cause issues when compiling as
+    # a static library into another application which also uses
+    # libdeflate but we switch the export symbol to hidden which should
+    # hide the symbols when linking...
+    set(EXR_DEFLATE_SOURCES
+      lib/arm/cpu_features.c
+      lib/x86/cpu_features.c
+      lib/utils.c
+      lib/deflate_compress.c
+      lib/deflate_decompress.c
+      lib/adler32.c
+      lib/zlib_compress.c
+      lib/zlib_decompress.c)
+    # don't need these
+    # lib/crc32.c
+    # lib/gzip_compress.c
+    # lib/gzip_decompress.c
+    file(READ ${deflate_SOURCE_DIR}/lib/lib_common.h DEFLATE_HIDE)
+    string(REPLACE "visibility(\"default\")" "visibility(\"hidden\")" DEFLATE_HIDE "${DEFLATE_HIDE}")
+    string(REPLACE "__declspec(dllexport)" "/**/" DEFLATE_HIDE "${DEFLATE_HIDE}")
+    file(WRITE ${deflate_SOURCE_DIR}/lib/lib_common.h "${DEFLATE_HIDE}")
   
-  # cmake makes fetch content name lowercase for the properties (to deflate)
-  list(TRANSFORM EXR_DEFLATE_SOURCES PREPEND ${deflate_SOURCE_DIR}/)
-  set(EXR_DEFLATE_INCLUDE_DIR ${deflate_SOURCE_DIR})
-  set(EXR_DEFLATE_LIB)
-else()
-  set(EXR_DEFLATE_INCLUDE_DIR)
-  set(EXR_DEFLATE_LIB ${deflate_LIBRARIES})
-  # set EXR_DEFATE_LDFLAGS for OpenEXR.pc.in for static build
-  if (BUILD_SHARED_LIBS)
-    set(EXR_DEFLATE_LDFLAGS "")
+    # cmake makes fetch content name lowercase for the properties (to deflate)
+    list(TRANSFORM EXR_DEFLATE_SOURCES PREPEND ${deflate_SOURCE_DIR}/)
+    set(EXR_DEFLATE_INCLUDE_DIR ${deflate_SOURCE_DIR})
+    set(EXR_DEFLATE_LIB)
   else()
-    set(EXR_DEFLATE_LDFLAGS "-l${deflate_LIBRARIES}")
+    set(EXR_DEFLATE_INCLUDE_DIR)
+    set(EXR_DEFLATE_LIB ${deflate_LIBRARIES})
+    # set EXR_DEFATE_LDFLAGS for OpenEXR.pc.in for static build
+    if (BUILD_SHARED_LIBS)
+      set(EXR_DEFLATE_LDFLAGS "")
+    else()
+      set(EXR_DEFLATE_LDFLAGS "-l${deflate_LIBRARIES}")
+    endif()
+    set(EXR_DEFLATE_SOURCES)
   endif()
-  set(EXR_DEFLATE_SOURCES)
 endif()
-
 #######################################
 # Find or install Imath
 #######################################

If you ignore the indent changes its just:

diff --git a/openexr_original/cmake/OpenEXRSetup.cmake b/openexr/cmake/OpenEXRSetup.cmake
index 425992d..eaac6fa 100644
--- a/openexr_original/cmake/OpenEXRSetup.cmake
+++ b/openexr/cmake/OpenEXRSetup.cmake
@@ -160,14 +160,22 @@ set(OPENEXR_DEFLATE_TAG "v1.18" CACHE STRING "Tag to use for libdeflate source r
 if(NOT OPENEXR_FORCE_INTERNAL_DEFLATE)
   #TODO: ^^ Release should not clone from main, this is a place holder
   set(CMAKE_IGNORE_PATH "${CMAKE_CURRENT_BINARY_DIR}/_deps/deflate-src/config;${CMAKE_CURRENT_BINARY_DIR}/_deps/deflate-build/config")
-  include(FindPkgConfig)
-  pkg_check_modules(deflate IMPORTED_TARGET GLOBAL libdeflate)
-  set(CMAKE_IGNORE_PATH)
-  if (deflate_FOUND)
-    message(STATUS "Using libdeflate from ${deflate_LINK_LIBRARIES}")
+  find_package(libdeflate CONFIG)
+  if(libdeflate_FOUND)
+    message(STATUS "Using libdeflate from ${libdeflate_DIR}")
+  else()
+    include(FindPkgConfig)
+    pkg_check_modules(deflate IMPORTED_TARGET GLOBAL libdeflate)
+    set(CMAKE_IGNORE_PATH)
+    if (deflate_FOUND)
+      message(STATUS "Using libdeflate from ${deflate_LINK_LIBRARIES}")
+    endif()
   endif()
 endif()
 
+if(libdeflate_FOUND)
+  set(EXR_DEFLATE_LIB libdeflate::libdeflate_shared)
+else()
 if(NOT TARGET PkgConfig::deflate AND NOT deflate_FOUND)
   if(OPENEXR_FORCE_INTERNAL_DEFLATE)
     message(STATUS "libdeflate forced internal, installing from ${OPENEXR_DEFLATE_REPO} (${OPENEXR_DEFLATE_TAG})")
@@ -224,7 +232,7 @@ else()
   endif()
   set(EXR_DEFLATE_SOURCES)
 endif()
-
+endif()
 #######################################
 # Find or install Imath
 #######################################

@brechtvl
Copy link
Contributor

@bebuch are you planning to submit this as a pull request? We need the same thing for Blender. I can make a pull request too if that helps.

The one further change we need is to fall back to the static library if there is no shared one.

if(libdeflate::libdeflate_shared)
  set(EXR_DEFLATE_LIB libdeflate::libdeflate_shared)
else()
  set(EXR_DEFLATE_LIB libdeflate::libdeflate_static)
endif()

@bebuch
Copy link
Author

bebuch commented Nov 13, 2023

@brechtvl The patch is not a general solution, therefore I didn't make a PR. Please feel free to use it as part of your PR. ;-)

@cary-ilm
Copy link
Member

Following up on this, @brechtvl, will you submit a PR?

brechtvl added a commit to brechtvl/openexr that referenced this issue Jan 10, 2024
When not using the internal deflate, the only option was pkgconfig which
is not well supported on Windows. This adds support for using the
libdeflate cmake config files when available.

It also turns the internal libdeflate into a target for consistency.

Resolves AcademySoftwareFoundation#1588
brechtvl added a commit to brechtvl/openexr that referenced this issue Jan 10, 2024
When not using the internal deflate, the only option was pkgconfig which
is not well supported on Windows. This adds support for using the
libdeflate cmake config files when available.

This change also affects the generated OpenEXR cmake config files. For
a static OpenEXR library, INTERFACE_LINK_LIBRARIES now appropriately
includes libdeflate along with the existing dependencies like Imath.

It also turns the internal libdeflate into a target for consistency.

Resolves AcademySoftwareFoundation#1588
brechtvl added a commit to brechtvl/openexr that referenced this issue Jan 10, 2024
When not using the internal deflate, the only option was pkgconfig which
is not well supported on Windows. This adds support for using the
libdeflate cmake config files when available.

This change also affects the generated OpenEXR cmake config files. For
a static OpenEXR library, INTERFACE_LINK_LIBRARIES now appropriately
includes libdeflate along with the existing dependencies like Imath.

It also turns the internal libdeflate into a target for consistency.

Resolves AcademySoftwareFoundation#1588

Signed-off-by: Brecht Van Lommel <[email protected]>
@brechtvl
Copy link
Contributor

I submitted one now.

cary-ilm pushed a commit that referenced this issue Jan 22, 2024
* Support cmake config and shared library for libdeflate

When not using the internal deflate, the only option was pkgconfig which
is not well supported on Windows. This adds support for using the
libdeflate cmake config files when available.

This change also affects the generated OpenEXR cmake config files. For
a static OpenEXR library, INTERFACE_LINK_LIBRARIES now appropriately
includes libdeflate along with the existing dependencies like Imath.

It also turns the internal libdeflate into a target for consistency.

Resolves #1588

Signed-off-by: Brecht Van Lommel <[email protected]>

* Back out of adding a target for internal libdeflate

Signed-off-by: Brecht Van Lommel <[email protected]>

* Use Requires.private for generated pkgconfig file

Signed-off-by: Brecht Van Lommel <[email protected]>

---------

Signed-off-by: Brecht Van Lommel <[email protected]>
cary-ilm pushed a commit to cary-ilm/openexr that referenced this issue Feb 13, 2024
* Support cmake config and shared library for libdeflate

When not using the internal deflate, the only option was pkgconfig which
is not well supported on Windows. This adds support for using the
libdeflate cmake config files when available.

This change also affects the generated OpenEXR cmake config files. For
a static OpenEXR library, INTERFACE_LINK_LIBRARIES now appropriately
includes libdeflate along with the existing dependencies like Imath.

It also turns the internal libdeflate into a target for consistency.

Resolves AcademySoftwareFoundation#1588

Signed-off-by: Brecht Van Lommel <[email protected]>

* Back out of adding a target for internal libdeflate

Signed-off-by: Brecht Van Lommel <[email protected]>

* Use Requires.private for generated pkgconfig file

Signed-off-by: Brecht Van Lommel <[email protected]>

---------

Signed-off-by: Brecht Van Lommel <[email protected]>
cary-ilm pushed a commit that referenced this issue Feb 16, 2024
* Support cmake config and shared library for libdeflate

When not using the internal deflate, the only option was pkgconfig which
is not well supported on Windows. This adds support for using the
libdeflate cmake config files when available.

This change also affects the generated OpenEXR cmake config files. For
a static OpenEXR library, INTERFACE_LINK_LIBRARIES now appropriately
includes libdeflate along with the existing dependencies like Imath.

It also turns the internal libdeflate into a target for consistency.

Resolves #1588

Signed-off-by: Brecht Van Lommel <[email protected]>

* Back out of adding a target for internal libdeflate

Signed-off-by: Brecht Van Lommel <[email protected]>

* Use Requires.private for generated pkgconfig file

Signed-off-by: Brecht Van Lommel <[email protected]>

---------

Signed-off-by: Brecht Van Lommel <[email protected]>
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 a pull request may close this issue.

4 participants