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

CMake: remove unnecessary WITH_TIFF and IF() #122

Open
nono303 opened this issue Jun 14, 2024 · 2 comments
Open

CMake: remove unnecessary WITH_TIFF and IF() #122

nono303 opened this issue Jun 14, 2024 · 2 comments

Comments

@nono303
Copy link

nono303 commented Jun 14, 2024

Hi @rouault
Thx for merging #120!
Bumping on #115 (comment) - Here is the CMakeList.txt patch corresponding to my previous comment based on e00dcd6

  • TIFF & PROJ : REQUIRED
    • So WITH_TIFF option doesn’t make sense, in my point of view
  • Zlib & JPEG also REQUIRED if WITH_x option is set
  • As find_package(x REQUIRED) failed if not FOUND, if(x_FOUND) is not necessary

Just tell me if this seems coherent doesn't make sense to U.

diff --git a/libgeotiff/CMakeLists.txt b/libgeotiff/CMakeLists.txt
index 9d4404a..5a0e7b6 100644
--- a/libgeotiff/CMakeLists.txt
+++ b/libgeotiff/CMakeLists.txt
@@ -100,16 +100,7 @@ if (MSVC)
 endif (MSVC)
 
 ###############################################################################
-# Search for dependencies
-
-
-# TIFF support - required, default=ON
-option(WITH_TIFF "Choose if TIFF support should be built" ON)
-
-FIND_PACKAGE(PROJ NO_MODULE QUIET)
-if (NOT PROJ_FOUND)
-  FIND_PACKAGE(PROJ REQUIRED)
-endif ()
+# Optionnal supports
 
 # Zlib support - optional, default=OFF
 option(WITH_ZLIB "Choose if zlib support should be built" OFF)
@@ -135,7 +126,6 @@ INCLUDE_DIRECTORIES(BEFORE ${CMAKE_CURRENT_BINARY_DIR})
 
 MESSAGE(STATUS "Generating geo_config.h header - done")
 
-
 ###############################################################################
 # Installation settings
 
@@ -220,55 +210,44 @@ set_property(TARGET ${GEOTIFF_LIBRARY_TARGET} PROPERTY C_STANDARD 99)
 set_property(TARGET ${GEOTIFF_LIBRARY_TARGET} PROPERTY POSITION_INDEPENDENT_CODE ON)
 set_property(TARGET ${GEOTIFF_LIBRARY_TARGET} PROPERTY OUTPUT_NAME ${GEOTIFF_LIB_NAME})
 
-IF(WITH_JPEG)
-    FIND_PACKAGE(JPEG NO_MODULE QUIET)
-    if (NOT JPEG_FOUND)
-      FIND_PACKAGE(JPEG)
-    endif ()
-
-    IF(JPEG_FOUND)
-        SET(HAVE_JPEG 1)
-        TARGET_INCLUDE_DIRECTORIES(${GEOTIFF_LIBRARY_TARGET} PRIVATE ${JPEG_INCLUDE_DIR})
-        target_compile_definitions(${GEOTIFF_LIBRARY_TARGET} PRIVATE -DHAVE_JPEG=${HAVE_JPEG})
-    ENDIF()
-ENDIF()
+# mandatory
 
+FIND_PACKAGE(PROJ REQUIRED)
 
-IF(WITH_TIFF)
-    FIND_PACKAGE(TIFF NO_MODULE QUIET)
-    if (NOT TIFF_FOUND)
-      FIND_PACKAGE(TIFF REQUIRED)
-    endif ()
+FIND_PACKAGE(TIFF REQUIRED)
+# Confirm required API is available
+INCLUDE(CheckFunctionExists)
+SET(CMAKE_REQUIRED_LIBRARIES ${TIFF_LIBRARIES})
 
-    IF(TIFF_FOUND)
-        # Confirm required API is available
-        INCLUDE(CheckFunctionExists)
-        SET(CMAKE_REQUIRED_LIBRARIES ${TIFF_LIBRARIES})
+CHECK_FUNCTION_EXISTS(TIFFOpen HAVE_TIFFOPEN)
+IF(NOT HAVE_TIFFOPEN)
+    SET(TIFF_FOUND) # ReSET to NOT found for TIFF library
+    MESSAGE(FATAL_ERROR "Failed to link with libtiff - TIFFOpen function not found")
+ENDIF()
 
-        CHECK_FUNCTION_EXISTS(TIFFOpen HAVE_TIFFOPEN)
-        IF(NOT HAVE_TIFFOPEN)
-            SET(TIFF_FOUND) # ReSET to NOT found for TIFF library
-            MESSAGE(FATAL_ERROR "Failed to link with libtiff - TIFFOpen function not found")
-        ENDIF()
+CHECK_FUNCTION_EXISTS(TIFFMergeFieldInfo HAVE_TIFFMERGEFIELDINFO)
+IF(NOT HAVE_TIFFMERGEFIELDINFO)
+    SET(TIFF_FOUND) # ReSET to NOT found for TIFF library
+    MESSAGE(FATAL_ERROR "Failed to link with libtiff - TIFFMergeFieldInfo function not found. libtiff 3.6.0 Beta or later required. Please upgrade or use an older version of libgeotiff")
+ENDIF()
 
-        CHECK_FUNCTION_EXISTS(TIFFMergeFieldInfo HAVE_TIFFMERGEFIELDINFO)
-        IF(NOT HAVE_TIFFMERGEFIELDINFO)
-            SET(TIFF_FOUND) # ReSET to NOT found for TIFF library
-            MESSAGE(FATAL_ERROR "Failed to link with libtiff - TIFFMergeFieldInfo function not found. libtiff 3.6.0 Beta or later required. Please upgrade or use an older version of libgeotiff")
-        ENDIF()
+TARGET_INCLUDE_DIRECTORIES(${GEOTIFF_LIBRARY_TARGET} PRIVATE ${TIFF_INCLUDE_DIR})
+TARGET_COMPILE_DEFINITIONS(${GEOTIFF_LIBRARY_TARGET} PRIVATE -DHAVE_TIFF=1)
 
-        TARGET_INCLUDE_DIRECTORIES(${GEOTIFF_LIBRARY_TARGET} PRIVATE ${TIFF_INCLUDE_DIR})
-        TARGET_COMPILE_DEFINITIONS(${GEOTIFF_LIBRARY_TARGET} PRIVATE -DHAVE_TIFF=1)
-    ENDIF(TIFF_FOUND)
-ENDIF(WITH_TIFF)
+# optionnal
+
+IF(WITH_JPEG)
+    FIND_PACKAGE(JPEG REQUIRED)
+    SET(HAVE_JPEG 1)
+    TARGET_INCLUDE_DIRECTORIES(${GEOTIFF_LIBRARY_TARGET} PRIVATE ${JPEG_INCLUDE_DIR})
+    target_compile_definitions(${GEOTIFF_LIBRARY_TARGET} PRIVATE -DHAVE_JPEG=${HAVE_JPEG})
+ENDIF()
 
 IF(WITH_ZLIB)
-    FIND_PACKAGE(ZLIB REQUIRED )
-    IF(ZLIB_FOUND)
-        SET(HAVE_ZIP 1)
-        TARGET_INCLUDE_DIRECTORIES(${GEOTIFF_LIBRARY_TARGET} PRIVATE ${ZLIB_INCLUDE_DIR})
-        TARGET_COMPILE_DEFINITIONS(${GEOTIFF_LIBRARY_TARGET} PRIVATE -DHAVE_ZIP=${HAVE_ZIP})
-    ENDIF()
+    FIND_PACKAGE(ZLIB REQUIRED)
+    SET(HAVE_ZIP 1)
+    TARGET_INCLUDE_DIRECTORIES(${GEOTIFF_LIBRARY_TARGET} PRIVATE ${ZLIB_INCLUDE_DIR})
+    TARGET_COMPILE_DEFINITIONS(${GEOTIFF_LIBRARY_TARGET} PRIVATE -DHAVE_ZIP=${HAVE_ZIP})
 ENDIF()
 
 TARGET_INCLUDE_DIRECTORIES(${GEOTIFF_LIBRARY_TARGET} PUBLIC
@@ -404,8 +383,9 @@ Summary of build options:
    Build man pages:          ${BUILD_MAN}
    Build doc files:          ${BUILD_DOC}
    Build GeoTIFF utilities:  ${WITH_UTILITIES}
-   Build TIFF support:       ${WITH_TIFF}
-   Build zlib support:       ${WITH_ZLIB}
-   Build JPEG support:       ${WITH_JPEG}
+   PROJ version:             ${PROJ_VERSION_STRING}
+   TIFF version:	     ${TIFF_VERSION_STRING}
+   Build zlib support:       ${WITH_ZLIB} ${ZLIB_VERSION}
+   Build JPEG support:       ${WITH_JPEG} ${JPEG_VERSION}
    Build TOWGS84 support:    ${WITH_TOWGS84}
 ################################")
@rouault
Copy link
Member

rouault commented Jun 14, 2024

  • Zlib & JPEG also REQUIRED if WITH_x option is set

Actually, I don't see any explicit use of them in the sources! They could probably be just removed

Could you submit your changes through pull requests?

@nono303
Copy link
Author

nono303 commented Jun 14, 2024

#123

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

No branches or pull requests

2 participants