jasper-software / xv

XV Software
Other
26 stars 9 forks source link

JP2K support not found correctly in cmake build #2

Closed sthen closed 2 years ago

sthen commented 2 years ago

The cmake build fails for me. While the Jasper library is found, cmake's FindJasper module doesn't set the target, so "TARGET Jasper::Jasper" fails:

-- Found Jasper: /usr/local/lib/libjasper.so.5.0 (found version "3.0.2") 
CMake Warning at CMakeLists.txt:136 (message):
  Disabling JPEG-2000 Support.

-- Found ZLIB: /usr/lib/libz.so.6.0 (found version "1.2.11") 
-- Found PNG: /usr/local/lib/libpng.so.18.0 (found version "1.6.37") 
-- Found TIFF: /usr/local/lib/libtiff.so.41.0 (found version "4.3.0")  
JP2K: OFF
JPEG: ON
TIFF: ON
PNG: ON
-- Configuring done

I'm not super-familiar with cmake but in the grand tradition of OS porters of copying/pasting/tweaking from other files until it builds, I came up with this by stealing bits of the FindJPEG module. Can't say if it's "correct" but it seems to work for me.

Index: CMakeLists.txt
--- CMakeLists.txt.orig
+++ CMakeLists.txt
@@ -132,6 +132,17 @@ if(NOT MATH_LIBRARY)
 endif()

 find_package(Jasper)
+add_library(Jasper::Jasper UNKNOWN IMPORTED)
+if(JASPER_INCLUDE_DIR)
+   set_target_properties(Jasper::Jasper PROPERTIES
+       INTERFACE_INCLUDE_DIRECTORIES "${JPEG_INCLUDE_DIRS}")
+endif()
+if(EXISTS "${JASPER_LIBRARY}")
+   set_target_properties(Jasper::Jasper PROPERTIES
+       IMPORTED_LINK_INTERFACE_LANGUAGES "C"
+       IMPORTED_LOCATION "${JASPER_LIBRARY}")
+endif()
+
 if(XV_ENABLE_JP2K AND NOT TARGET Jasper::Jasper)
    message(WARNING "Disabling JPEG-2000 Support.")
    set(XV_ENABLE_JP2K OFF)
sthen commented 2 years ago

btw this is with cmake 3.20.3 on OpenBSD

mdadams commented 2 years ago

The problem (I think) is that the Jasper::Jasper imported target is not provided by pre-3.22 versions of CMake. This is my fault, however, not yours, as the CMakeLists.txt file for XV claims to only need CMake 3.12 (which is technically incorrect). Since many people are using older versions of CMake, I tried adding a workaround (based on your solution) that should define the Jasper::Jasper target when it doesn't get created in older CMake versions. Can you let me know if it works? If it doesn't perhaps you can make a pull request that tweaks it so it does? I cannot actually test my fix easily as I have new versions of CMake on all of my systems. My changes are in commit d84be152ca495f65837758b0fd06118d84646db5.

sthen commented 2 years ago

It does indeed work, thank you for the quick fix Michael.

mdadams commented 2 years ago

Thanks for confirming that it works.