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

Automate release generation from tags and refactor CI #115

Merged
merged 21 commits into from
May 28, 2024

Conversation

hobu
Copy link
Contributor

@hobu hobu commented May 21, 2024

This PR does a bunch of stuff at once (sorry):

  • Remove Appveyor config
  • Remove Travis config
  • Add BUILD_SHARED_LIBS matrix for ON and OFF using similar config for macos-latest, windows-latest, and ubuntu-latest
  • Modernize CMake configuration to prevent flag/feature leaking
  • Adds CPack configuration so package_source target is available for dist generation
  • Generates release artifacts and attaches them to every build
  • Creates a release and attaches release artifacts for every tag of the OSGeo/libgeotiff repository
  • Attests the release artifacts if the github.repository_owner == OSGeo
  • Bumps the minimum CMake version to 3.13
  • Removes 16 year old dead makefile.mpw
  • Removes 6 year old dead makefile.vc
  • Updates FindGeoTIFF.cmake with GDAL's recent version
  • Updates FindPROJ.cmake with GDAL's recent version

@hobu
Copy link
Contributor Author

hobu commented May 22, 2024

Updated this builder to create the tarball using CMake/CPack and attach it as an artifact to the build

@hobu hobu changed the title [WIP] Moderinization attempt of CMake Automate release generation from tags and refactor CI May 23, 2024
@hobu
Copy link
Contributor Author

hobu commented May 23, 2024

@rouault can we kill the VCPKG CI builder? It's now busted and points at a bunch of old stuff. It either needs to be updated or dropped.

Additionally, we still need a CTest target for the tests on UNIX.

@rouault
Copy link
Member

rouault commented May 23, 2024

can we kill the VCPKG CI builder?

yes, it was just a means of testing on Windows, not an aim per se. Conda based builds will do just fine.

@rouault rouault mentioned this pull request May 24, 2024
@nono303
Copy link

nono303 commented May 25, 2024

Hi @hobu, Thx for this PR!
I’ve just tested CMake build on Windows MSVC and FIND_PACKAGE for PROJ & JPEG failed.
Fixed it changing some option and order; I’ll let you check that’s ok for your platform regression tests

Also, related to #117 here are some improvements that could be integrated:

  • WITH_* passed as OPTION (seems to be better than SET to be shown on cmake -LH)
  • Added BUILD_MAN & BUILD_DOC (useless in my "Windows" case)
  • Added LIB_SUFFIX to remove the _i suffix for lib (doesn't seem to be standard anymore, at least for WIndows)
  • INSTALL pdb file if exist
  • Display Summary of build options

cmake %CMAKE_OPTS% -G %CMAKE_TGT_NINJA% ^
-DCMAKE_INSTALL_PREFIX=%PATH_INSTALL% ^
-DCMAKE_BUILD_TYPE=%CMAKE_BUILD_TYPE% ^
-DBUILD_SHARED_LIBS=ON ^
-DPROJ_LIBRARY=%PATH_INSTALL:\=/%/_proj/lib/proj.lib ^
-DPROJ_INCLUDE_DIR=%PATH_INSTALL:\=/%/_proj/include ^
-DJPEG_LIBRARY_RELEASE=%PATH_INSTALL:\=/%/lib/jpeg.lib ^
-DJPEG_DIR=%PATH_INSTALL:\=/% ^
-DWITH_ZLIB=ON ^
-DWITH_UTILITIES=OFF ^
-DWITH_TIFF=ON ^
-DWITH_JPEG=ON ^
-DWITH_TOWGS84=ON ^
-DBUILD_DOC=OFF ^
-DBUILD_MAN=OFF ^
-DLIB_SUFFIX= ^
%PATH_SRC%\%1\%1

libgeotiff_vs17-x64_build.log


diff --git "a/libgeotiff/CMakeLists.txt" "b/libgeotiff/CMakeLists.txt"
index 44c6b23..811505b 100644
--- "a/libgeotiff/CMakeLists.txt"
+++ "b/libgeotiff/CMakeLists.txt"
@@ -46,6 +46,8 @@ SET(CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake ${CMAKE_MODULE_PATH})
 # General build settings
 
 option(BUILD_SHARED_LIBS "Set ON to build shared library" ON)
+option(BUILD_MAN "Set ON to build man pages" ON)
+option(BUILD_DOC "Set ON to build doc files" ON)
 
 IF(NOT CMAKE_BUILD_TYPE)
     SET(CMAKE_BUILD_TYPE Debug CACHE STRING
@@ -82,26 +84,31 @@ CHECK_INCLUDE_FILES(strings.h GEOTIFF_HAVE_STRINGS_H)
 ###############################################################################
 # User-defined build settings
 
-SET(WITH_UTILITIES TRUE CACHE BOOL "Choose if GeoTIFF utilities should be built")
+option(WITH_UTILITIES "Choose if GeoTIFF utilities should be built" ON)
+
+SET(LIB_SUFFIX _i CACHE STRING "Static library suffix")
 
 ###############################################################################
 # Search for dependencies
 
 
 # TIFF support - required, default=ON
-SET(WITH_TIFF TRUE CACHE BOOL "Choose if TIFF support should be built")
+option(WITH_TIFF "Choose if TIFF support should be built" ON)
 
-FIND_PACKAGE(PROJ NO_MODULE REQUIRED)
+FIND_PACKAGE(PROJ NO_MODULE QUIET)
+if (NOT PROJ_FOUND)
+  FIND_PACKAGE(PROJ REQUIRED)
+endif ()
 
 # Zlib support - optional, default=OFF
-SET(WITH_ZLIB FALSE CACHE BOOL "Choose if zlib support should be built")
+option(WITH_ZLIB "Choose if zlib support should be built" OFF)
 
 # JPEG support - optional, default=OFF
-SET(WITH_JPEG FALSE CACHE BOOL "Choose if JPEG support should be built")
+option(WITH_JPEG  "Choose if JPEG support should be built" OFF)
 
 
 # Turn off TOWGS84 support
-SET(WITH_TOWGS84 TRUE CACHE BOOL "Build with TOWGS84 support")
+option(WITH_TOWGS84 "Build with TOWGS84 support" ON)
 IF (NOT WITH_TOWGS84)
    SET(GEO_NORMALIZE_DISABLE_TOWGS84 1)
 endif()
@@ -154,13 +161,16 @@ SET(GEOTIFF_MAN_PAGES
 #    ${PROJECT_BINARY_DIR}/geotiff_version.h
 
 # Install doc files
-INSTALL(FILES
+if(BUILD_DOC)
+  INSTALL(FILES
     AUTHORS ChangeLog COPYING LICENSE README README_BIN README.WIN
     DESTINATION ${CMAKE_INSTALL_DOCDIR})
+endif ()
 
 # Install man pages
-INSTALL(FILES ${GEOTIFF_MAN_PAGES} DESTINATION ${CMAKE_INSTALL_MANDIR}/man1)
-
+if(BUILD_MAN)
+  INSTALL(FILES ${GEOTIFF_MAN_PAGES} DESTINATION ${CMAKE_INSTALL_MANDIR}/man1)
+endif ()
 
 # Install header files for development distribution
 INSTALL(FILES ${GEOTIFF_LIB_HEADERS} DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
@@ -202,9 +212,9 @@ set_property(TARGET ${GEOTIFF_LIBRARY_TARGET} PROPERTY POSITION_INDEPENDENT_CODE
 set_property(TARGET ${GEOTIFF_LIBRARY_TARGET} PROPERTY OUTPUT_NAME ${GEOTIFF_LIB_NAME})
 
 IF(WITH_JPEG)
-    FIND_PACKAGE(JPEG NO_MODULE QUIET CONFIG)
+    FIND_PACKAGE(JPEG NO_MODULE QUIET)
     if (NOT JPEG_FOUND)
-      FIND_PACKAGE(JPEG CONFIG)
+      FIND_PACKAGE(JPEG)
     endif ()
 
     IF(JPEG_FOUND)
@@ -257,7 +267,7 @@ TARGET_INCLUDE_DIRECTORIES(${GEOTIFF_LIBRARY_TARGET} PUBLIC
                             $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/libxtiff>
                             $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>)
 IF(WIN32 AND MSVC)
-    SET_TARGET_PROPERTIES(${GEOTIFF_LIBRARY_TARGET} PROPERTIES IMPORT_SUFFIX "_i.lib")
+    SET_TARGET_PROPERTIES(${GEOTIFF_LIBRARY_TARGET} PROPERTIES IMPORT_SUFFIX "${LIB_SUFFIX}.lib")
 ENDIF(WIN32 AND MSVC)
 
 # Unix, linux:
@@ -332,10 +342,11 @@ target_include_directories( ${GEOTIFF_LIBRARY_TARGET} PUBLIC
 
 INSTALL( TARGETS ${GEOTIFF_LIBRARY_TARGET}
      EXPORT depends
-     RUNTIME DESTINATION ${CMAKE_INSTALLL_BINDIR}
+     RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
      LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
      PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
      ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} )
+INSTALL(FILES $<TARGET_PDB_FILE:${GEOTIFF_LIBRARY_TARGET}> DESTINATION ${CMAKE_INSTALL_BINDIR} OPTIONAL)
 
 # Install libgeotiff.pc
 set(prefix "${CMAKE_INSTALL_PREFIX}")
@@ -371,3 +382,16 @@ IF(WITH_UTILITIES)
 ENDIF()
 
 ADD_SUBDIRECTORY(cmake)
+
+message(STATUS "
+################################
+Summary of build options:
+   Build shared library:     ${BUILD_SHARED_LIBS}
+   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}
+   Build TOWGS84 support:    ${WITH_TOWGS84}
+################################")

++NoNo

@rouault
Copy link
Member

rouault commented May 28, 2024

@hobu Any comment regarding above @nono303 's remark ? I know that with GDAL I had some issues with find_package(PROJ) with module vs config, depending on the PROJ version. In https://github.com/OSGeo/gdal/blob/be425562003bf26bd9927a99076edc807f189ee5/cmake/helpers/CheckDependentLibraries.cmake#L381I've some fairly complicated logic to make that configurable
I'm also wondering how to deal with the commit history. Perhaps I should just stash everything as it might not be easy to create a clean commit history

@hobu
Copy link
Contributor Author

hobu commented May 28, 2024

@nono303 I have applied your patch with some small modifications

  • guarded the PDB install with MSVC
  • renamed LIB_SUFFIX to INTERFACE_LIB_PREFIX and guarded it by MSVC. Changing behavior of this would be disruptive to a lot of people's build systems.

@hobu
Copy link
Contributor Author

hobu commented May 28, 2024

I'm also wondering how to deal with the commit history. Perhaps I should just stash everything as it might not be easy to create a clean commit history

Squash it all down as you see fit. Sorry for such a big ball of stuff.

@rouault rouault merged commit ff1f6d8 into OSGeo:master May 28, 2024
7 of 8 checks passed
@rouault rouault added this to the 1.7.4 milestone May 28, 2024
@rouault
Copy link
Member

rouault commented May 28, 2024

Squashed and merged. Thanks @hobu (and @nono303) for this update!

@nono303
Copy link

nono303 commented May 28, 2024

Many Thx @hobu! U applied it in a better way ;)
Pulled 9af3298 ant tested it with success!

...just to be sure about FIND_PACKAGE(PROJ NO_MODULE QUIET) l.100

  • PROJ NO_MODULE QUIET no message output on what had been checked & found (personally I don't like that)
  • PROJ NO_MODULE output
-- Found SQLite3: C:/sdk/release/vs17_x64-avx2/include (found version "3.46.0")
-- Found TIFF: C:/sdk/release/vs17_x64-avx2/lib/cmake/tiff (found version "4.6.0")
-- Found TIFF: C:/sdk/release/vs17_x64-avx2/lib/cmake/CURL/CURLConfig.cmake (found version "8.8.0-DEV")
  • PROJ MODULE or PROJ (with or without REQUIRED)
-- Found PROJ: C:/sdk/release/vs17_x64-avx2/_proj/lib/proj.lib (found version "9.4.0")

Is this what you are looking for : SQLite3 TIFF TIFF?
In this case FIND_PACKAGE(PROJ REQUIRED) l.102 will not check the same think than FIND_PACKAGE(PROJ NO_MODULE QUIET) l.100

@hobu
Copy link
Contributor Author

hobu commented May 28, 2024

PROJ and TIFF are hard requirements of libgeotiff. We should update the find_package calls to reflect this (it wasn't always true).

@nono303
Copy link

nono303 commented May 28, 2024

All clear now !

  • 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

patch.txt
result.txt

nono303 pushed a commit to nono303/win-build-scripts that referenced this pull request May 28, 2024
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.

3 participants