sdcv update broke mac CI

it looks like the patch is not applying correctly?
caused by #1269

patching file src/sdcv.cpp
Hunk #1 succeeded at 62 with fuzz 2 (offset -4 lines).
missing header for unified diff at line 18 of patch
can't find file to patch at input line 18
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|                    //"./locale"//< for testing
--------------------------
File to patch: 
Skip this patch? [y] 
Skipping patch.
3 out of 3 hunks ignored
make[3]: *** [sdcv-download-prefix/src/sdcv-download-stamp/sdcv-download-patch] Error 1
make[2]: *** [CMakeFiles/sdcv-download.dir/all] Error 2
make[1]: *** [all] Error 2
CMake Error at /Users/runner/work/koreader-base/koreader-base/thirdparty/cmake_modules/DownloadProject.cmake:179 (message):
  Build step for sdcv failed: 2
Call Stack (most recent call first):
  CMakeLists.txt:77 (download_project)

It also fails locally with a incremental build, but works on a clean checkout.

[1/7] Performing update step for 'sdcv-download'
FAILED: sdcv-download-prefix/src/sdcv-download-stamp/sdcv-download-update 
cd /koreader/base/thirdparty/sdcv/build/arm-kindlepw2-linux-gnueabi/sdcv-src && /usr/bin/cmake -P /koreader/base/thirdparty/sdcv/build/arm-kindlepw2-linux-gnueabi/sdcv-download/sdcv-download-prefix/tmp/sdcv-download-gitupdate.cmake
fatal: bad object 4ae420734990ab9f5ccc038262368256b9323f4a
From https://github.com/Dushistov/sdcv
   0c1b821..6c3b136  gh-pages   -> origin/gh-pages
   958ec35..4ae4207  master     -> origin/master
 * [new tag]         v0.5.3     -> v0.5.3
error: Your local changes to the following files would be overwritten by checkout:
	src/sdcv.cpp
Please commit your changes or stash them before you switch branches.
Aborting
CMake Error at /koreader/base/thirdparty/sdcv/build/arm-kindlepw2-linux-gnueabi/sdcv-download/sdcv-download-prefix/tmp/sdcv-download-gitupdate.cmake:201 (message):
  Failed to checkout tag: '4ae420734990ab9f5ccc038262368256b9323f4a'


ninja: build stopped: subcommand failed.
CMake Error at /koreader/base/thirdparty/cmake_modules/DownloadProject.cmake:179 (message):
  Build step for sdcv failed: 1
Call Stack (most recent call first):
  CMakeLists.txt:77 (download_project)

PRs welcome. From CMake 3.11 it should switch to FetchContent.

I'm not sure what you're saying about the Mac build? It only caches .ccache.

koreader-base/.github/workflows/build.yml

Lines 22 to 32 in 7c1556e

    
                 - name: Cache for ccache 
        
                   uses: actions/cache@v1 
        
                   env: 
        
                     cache-name: cache-ccache 
        
                   with: 
        
                     path: ~/.ccache # ccache cache files are stored in `~/.ccache` on Linux/macOS 
        
                     key: ${{ runner.os }}-build-${{ env.cache-name }} 
        
                     restore-keys: | 
        
                       ${{ runner.os }}-build-${{ env.cache-name }}- 
        
                       ${{ runner.os }}-build- 
        
                       ${{ runner.os }}-

I had thought that the 2 failure might be related, but the github mac CI has failed since that commit with the message above.

I don't know Cmake at all, but it looks like the other libraries are using ExternalProject_Add instead of download_project, I will try updating it later and see if it helps.

I will try updating it later and see if it helps.

Please don't; it's much more convenient this way. ;-)

I will try updating it later and see if it helps.

Please don't; it's much more convenient this way. ;-)

What's the difference?

The problem lies in the download command part but I will not fix it until we can move on to FetchProject.

What's the difference?

It integrates it more CMake-y, as opposed to running a sub-CMake all over again.

@yparitcher Concretely, it allows us to do this:

koreader-base/thirdparty/sdcv/CMakeLists.txt

Lines 87 to 88 in 783c000

    
           add_subdirectory("${CMAKE_BINARY_DIR}/sdcv-src" 
        
                            "${CMAKE_BINARY_DIR}/sdcv-build")

It shaves off a number of seconds in CI needlessly repeating all of the compiler tests, but more important it allows us to mostly work with CMake rather than against it:

koreader-base/thirdparty/sdcv/CMakeLists.txt

Lines 24 to 65 in 783c000

    
           # because cmake needs all kinds of annoying special cmake variables 
        
           # Compiler and linker flags 
        
           if(DEFINED ENV{DARWIN}) 
        
               set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -framework CoreFoundation -framework Security") 
        
           endif() 
        
           if(DEFINED ENV{ANDROID}) 
        
               set(CMAKE_SYSTEM_NAME Android) 
        
               set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libgcc -static") 
        
           endif() 
        
           # took me an eternity to find $<SEMICOLON> 
        
           # important docs here https://cmake.org/cmake/help/v2.8.11/cmake.html#command:add_custom_command 
        
           set(GLIB2_INCLUDE_DIRS "${GLIB_DIR}/include/glib-2.0") 
        
           set(GLIB2_INCLUDE_DIRS "${GLIB2_INCLUDE_DIRS}$<SEMICOLON>${GLIB_DIR}/lib/glib-2.0/include$<SEMICOLON>${GETTEXT_DIR}/include") 
        
           set(GLIB2_LIBRARIES "${GLIB}") 
        
           # `CMAKE_PREFIX_PATH`, `CMAKE_INCLUDE_PATH` and `CMAKE_LIBRARY_PATH` are key 
        
           # It's very hard to tell CMake to use specific directories, but it's easy to 
        
           # tell it to search in specific directories. 
        
           set(CMAKE_PREFIX_PATH "${GLIB2_INCLUDE_DIRS}$<SEMICOLON>${ZLIB_DIR}$<SEMICOLON>${LIBICONV_DIR}$<SEMICOLON>${GETTEXT_DIR}") 
        
           # For some reason this doesn't actually work and CMake keeps finding mostly .so 
        
           # Which is funny, because the .a is in the *same* directory. Just saying. 
        
           # Instead we add semi-hardcoded references to the right libraries in GLIB2_LIBRARIES 
        
           if(DEFINED ENV{ANDROID} OR DEFINED ENV{DARWIN}) 
        
               # glib2 also needs to link with libiconv and gettext 
        
               # this is a fairly clean hack 
        
               # CMAKE_CXX_FLAGS with -I and -L doesn't seem to have much of an effect 
        
               set(GLIB2_LIBRARIES "${GLIB2_LIBRARIES}$<SEMICOLON>${LIBICONV_DIR}/lib/libiconv.a$<SEMICOLON>${GETTEXT_DIR}/lib/libintl.a") 
        
           endif() 
        
           set(ZLIB_INCLUDE_DIR "${ZLIB_DIR}/include") 
        
           set(ZLIB_LIBRARIES "${ZLIB}") 
        
           # I just want to be able to -I and -L and have things work. CMake, CMake... 
        
           set(ZLIB_LIBRARY_RELEASE "${ZLIB}") 
        
           ### Disable the silly build tree RPATH 
        
           set(CMAKE_SKIP_BUILD_RPATH True) 
        
           set(ENABLE_NLS False CACHE BOOL "") 
        
           set(WITH_READLINE False CACHE BOOL "")

Compare that to working against it:

koreader-base/thirdparty/libjpeg-turbo/CMakeLists.txt

Lines 18 to 44 in 783c000

    
           # CMake hell. 
        
           # We expect lib later on in Makefile.third, even on multilib systems... 
        
           list(APPEND CMAKE_ARGS "-DCMAKE_INSTALL_PREFIX=${BINARY_DIR}") 
        
           list(APPEND CMAKE_ARGS "-DCMAKE_INSTALL_LIBDIR=lib") 
        
           list(APPEND CMAKE_ARGS "-DCMAKE_INSTALL_INCLUDEDIR=include") 
        
           list(APPEND CMAKE_ARGS "-DCMAKE_SKIP_BUILD_RPATH:BOOL=True") 
        
           # c.f., https://android.googlesource.com/platform/ndk/+/master/build/cmake/android.toolchain.cmake 
        
           # and   https://github.com/taka-no-me/android-cmake 
        
           # In the meantime, I'll be sitting in the corner, crying hysterically. 
        
           if(DEFINED ENV{ANDROID}) 
        
           	list(APPEND CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux") 
        
           	# Magical value that inhibits all of CMake's own NDK handling code. (Or shit goes boom.) 
        
           	list(APPEND CMAKE_ARGS "-DCMAKE_SYSTEM_VERSION=1") 
        
           endif() 
        
           # And, finally, the actual libjpeg-turbo build options 
        
           list(APPEND CMAKE_ARGS "-DENABLE_STATIC=OFF") 
        
           list(APPEND CMAKE_ARGS "-DENABLE_SHARED=ON") 
        
           list(APPEND CMAKE_ARGS "-DWITH_JAVA=OFF") 
        
           list(APPEND CMAKE_ARGS "-DWITH_JPEG8=ON") 
        
           # make sure we disable ASM if we don't support SIMD 
        
           if(NOT DEFINED ENV{WANT_SIMD}) 
        
               list(APPEND CMAKE_ARGS "-DREQUIRE_SIMD=OFF") 
        
               list(APPEND CMAKE_ARGS "-DWITH_SIMD=OFF") 
        
           endif()

Although as you can see in the comments I'm not sure CMake ever works with you much, but that aside.

CMake 3.2+ or so is called "modern" CMake, which is allegedly good. I'd say the current versions (3.10+ or so) are a lot better, possibly even reasonably good. I don't think it makes too much of a difference compared to the 3.5 we're still supporting atm, but 3.11 or 3.13 would definitely simplify some things.

I'm not entirely sure what to do with the CMake version. I could easily put 3.16 in the CI just as we all use locally, but then we wouldn't be sure it'd work out of the box on older operating systems. Perhaps I could get away with combining Ubuntu 18.04 (CMake 3.10 by default) with the Ubuntu 20.04 CMake version (3.16) as a reasonable compromise.

tl;dr I don't want to turn off potential new contributors by having to install software not easily found in the repos.

I have 3.19 here on arch linux 😄

Pardon, I meant to write 3.16+. Actually I use the default 3.13 in Debian Buster on my desktop, which is a pretty good release. Basically all the good stuff we want from 3.11 with fewer bugs. It's a pity that Ubuntu 18.04 comes with 3.10 by default.

	- name: Cache for ccache
	uses: actions/cache@v1
	env:
	cache-name: cache-ccache
	with:
	path: ~/.ccache # ccache cache files are stored in `~/.ccache` on Linux/macOS
	key: ${{ runner.os }}-build-${{ env.cache-name }}
	restore-keys: \|
	${{ runner.os }}-build-${{ env.cache-name }}-
	${{ runner.os }}-build-
	${{ runner.os }}-

	add_subdirectory("${CMAKE_BINARY_DIR}/sdcv-src"
	"${CMAKE_BINARY_DIR}/sdcv-build")

	# because cmake needs all kinds of annoying special cmake variables
	# Compiler and linker flags
	if(DEFINED ENV{DARWIN})
	set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -framework CoreFoundation -framework Security")
	endif()

	if(DEFINED ENV{ANDROID})
	set(CMAKE_SYSTEM_NAME Android)
	set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libgcc -static")
	endif()

	# took me an eternity to find $<SEMICOLON>
	# important docs here https://cmake.org/cmake/help/v2.8.11/cmake.html#command:add_custom_command
	set(GLIB2_INCLUDE_DIRS "${GLIB_DIR}/include/glib-2.0")
	set(GLIB2_INCLUDE_DIRS "${GLIB2_INCLUDE_DIRS}$<SEMICOLON>${GLIB_DIR}/lib/glib-2.0/include$<SEMICOLON>${GETTEXT_DIR}/include")
	set(GLIB2_LIBRARIES "${GLIB}")

	# `CMAKE_PREFIX_PATH`, `CMAKE_INCLUDE_PATH` and `CMAKE_LIBRARY_PATH` are key
	# It's very hard to tell CMake to use specific directories, but it's easy to
	# tell it to search in specific directories.
	set(CMAKE_PREFIX_PATH "${GLIB2_INCLUDE_DIRS}$<SEMICOLON>${ZLIB_DIR}$<SEMICOLON>${LIBICONV_DIR}$<SEMICOLON>${GETTEXT_DIR}")

	# For some reason this doesn't actually work and CMake keeps finding mostly .so
	# Which is funny, because the .a is in the same directory. Just saying.
	# Instead we add semi-hardcoded references to the right libraries in GLIB2_LIBRARIES
	if(DEFINED ENV{ANDROID} OR DEFINED ENV{DARWIN})
	# glib2 also needs to link with libiconv and gettext
	# this is a fairly clean hack
	# CMAKE_CXX_FLAGS with -I and -L doesn't seem to have much of an effect
	set(GLIB2_LIBRARIES "${GLIB2_LIBRARIES}$<SEMICOLON>${LIBICONV_DIR}/lib/libiconv.a$<SEMICOLON>${GETTEXT_DIR}/lib/libintl.a")
	endif()

	set(ZLIB_INCLUDE_DIR "${ZLIB_DIR}/include")
	set(ZLIB_LIBRARIES "${ZLIB}")
	# I just want to be able to -I and -L and have things work. CMake, CMake...
	set(ZLIB_LIBRARY_RELEASE "${ZLIB}")

	### Disable the silly build tree RPATH
	set(CMAKE_SKIP_BUILD_RPATH True)

	set(ENABLE_NLS False CACHE BOOL "")
	set(WITH_READLINE False CACHE BOOL "")

	# CMake hell.
	# We expect lib later on in Makefile.third, even on multilib systems...
	list(APPEND CMAKE_ARGS "-DCMAKE_INSTALL_PREFIX=${BINARY_DIR}")
	list(APPEND CMAKE_ARGS "-DCMAKE_INSTALL_LIBDIR=lib")
	list(APPEND CMAKE_ARGS "-DCMAKE_INSTALL_INCLUDEDIR=include")
	list(APPEND CMAKE_ARGS "-DCMAKE_SKIP_BUILD_RPATH:BOOL=True")

	# c.f., https://android.googlesource.com/platform/ndk/+/master/build/cmake/android.toolchain.cmake
	# and https://github.com/taka-no-me/android-cmake
	# In the meantime, I'll be sitting in the corner, crying hysterically.
	if(DEFINED ENV{ANDROID})
	list(APPEND CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux")
	# Magical value that inhibits all of CMake's own NDK handling code. (Or shit goes boom.)
	list(APPEND CMAKE_ARGS "-DCMAKE_SYSTEM_VERSION=1")
	endif()

	# And, finally, the actual libjpeg-turbo build options
	list(APPEND CMAKE_ARGS "-DENABLE_STATIC=OFF")
	list(APPEND CMAKE_ARGS "-DENABLE_SHARED=ON")
	list(APPEND CMAKE_ARGS "-DWITH_JAVA=OFF")
	list(APPEND CMAKE_ARGS "-DWITH_JPEG8=ON")

	# make sure we disable ASM if we don't support SIMD
	if(NOT DEFINED ENV{WANT_SIMD})
	list(APPEND CMAKE_ARGS "-DREQUIRE_SIMD=OFF")
	list(APPEND CMAKE_ARGS "-DWITH_SIMD=OFF")
	endif()