Is CMAKE >= 3.13 really necessary? Refuses to build on Ubuntu 18.04 about timg

[hzeller]

Passing on to @coldtobi who added the CMake files. He certainly would know best.

@MestreLion if you edit the requirement to be 3.10, does it work ?

[MestreLion]

@MestreLion if you edit the requirement to be 3.10, does it work ?

Tried that already, and no, it doesn't. I was able to track down to the add_executable() function, which in CMake 3.11 the sources argument changed from mandatory to optional - as long as you specify them later in an add_target(), which timg does in the next line. So 3.10 complains about the missing argument.

But that was as far as I went, given my complete ignorance on CMake. A fix might be as easy as simply copy-and-paste the argument from add_target() to add_executable() but they might have a different syntax or extra requirements or a smarter/saner way to do so without duplicating the entire list (which seems quite awkward and non-DRY), so I just left it alone for more knowledgeable folks to handle.

[hzeller]

If someone knowledgeable in CMake and best practices there, I am happy to accept pull requests that make it compatible with older installations.

[coldtobi]

@MestreLion Can you please add the complete error you are getting with the minimum version reduced to 3.10?
(As Debian Buster does have CMake 3.13 already and Debian Stretch had 3.7 ... so it is not easy to reproduce for me.)

[coldtobi]

(Coffee starts working… Managed to compile timg with CMake 3.7)

@MestreLion yes, with newer CMake one can forward-declare targets; so moving one source file to the add_executable() will make this error go away.

However, you will run into more issues:
target_compile_features(timg PRIVATE cxx_std_14)
needs a newer CMake as well; However, if you have a recent gcc (>6.3, IIRC), C++14 is already the default, so dropping that line should work there. (I wouldn't drop the line generally, as cxx_std_14 is supposed to be compiler agnostic…)

Finally, install(TARGETS timg), will also need additional parameters. If you are only for a local build and do not want to install it, you can remove that line or specify the missing parameter (CMake will tell you. I didnt record the exact message.) This is likely quite easy to fix with include(GNUInstallDirs) and using the CMAKE_INSTALL_BIN variable, but I need to investigate, before I should say so…)

I hope with that info you can at least locally…
Alternatives:

• try building timg with the old Makefile (I've provided CMake to be able to package timg for Debian**, so to have less pain there, especially if it comes to porting and maintainability.) But the old Makefile might just work well for your usecase…
• recompile CMake yourself (I did at work a backport of CMake 3.18 for Ubuntu 18; however, it might need updated dependencies, IIRC libarchive was among them.)
• consider upgrading your installation to the current LTS supported by Canonical. While 18.04. is still supported by Canonical*, its starts showing its age. It might be enough to get an container with a newer Ubuntu and just use it for compiling.

* possibly you should ask them to provide a updated CMake... After all, its still supported… just kidding, they wont do it, its not how they define "supported".

** will happening soon(tm)

[MestreLion]

@coldtobi : not sure if this will be very helpful, but here it goes:

03:38:17 rodrigo@desktop ~/install/timg/build main * $ cmake ../ -DWITH_VIDEO_DECODING=Off -DWITH_VIDEO_DEVICE=Off -DWITH_OPENSLIDE_SUPPORT=Off
-- The CXX compiler identification is GNU 7.5.0
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.29.1") 
-- Found Git: /usr/bin/git (found version "2.17.1") 
-- Checking for module 'GraphicsMagick++'
--   Found GraphicsMagick++, version 1.3.28
-- Checking for module 'libturbojpeg'
--   Found libturbojpeg, version 1.5.2
-- Checking for module 'zlib'
--   Found zlib, version 1.2.11
-- Checking for module 'libexif'
--   Found libexif, version 0.6.21
-- Checking for module 'libavutil'
--   Found libavutil, version 55.78.100
-- Checking for module 'libswscale'
--   Found libswscale, version 4.8.100
-- Looking for C++ include pthread.h
-- Looking for C++ include pthread.h - found
-- Looking for pthread_create
-- Looking for pthread_create - not found
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE  
CMake Error at src/CMakeLists.txt:3 (add_executable):
  add_executable called with incorrect number of arguments


CMake Error at src/CMakeLists.txt:4 (target_sources):
  Cannot specify sources for target "timg" which is not built by this
  project.


CMake Error at src/CMakeLists.txt:25 (target_link_libraries):
  Cannot specify link libraries for target "timg" which is not built by this
  project.


-- Configuring incomplete, errors occurred!
See also "/home/rodrigo/install/timg/build/CMakeFiles/CMakeOutput.log".
See also "/home/rodrigo/install/timg/build/CMakeFiles/CMakeError.log".

The culprit, as I mentioned earlier, is the add_executable called with incorrect number of arguments, as its API changed in 3.11, making the sources argument optional. They are declared in target_sources() (which I erroneously called add_target() before).

Maybe the solution is as simple as moving the sources declaration from target_sources() to add_executable(), so instead of:

add_executable(timg)
target_sources(timg PRIVATE
  timg.cc

  buffered-write-sequencer.h buffered-write-sequencer.cc
  display-options.h
  framebuffer.h     framebuffer.cc
  image-display.h   image-display.cc
  image-source.h    image-source.cc
  iterm2-canvas.h   iterm2-canvas.cc
  jpeg-source.h     jpeg-source.cc
  kitty-canvas.h    kitty-canvas.cc
  renderer.h        renderer.cc
  terminal-canvas.h terminal-canvas.cc
  termutils.h       termutils.cc
  thread-pool.h
  timg-base64.h
  timg-png.h        timg-png.cc
  timg-time.h
  unicode-block-canvas.h unicode-block-canvas.cc
)

We would have this:

add_executable(timg, timg.cc
  buffered-write-sequencer.h buffered-write-sequencer.cc
  display-options.h
  framebuffer.h     framebuffer.cc
  image-display.h   image-display.cc
  image-source.h    image-source.cc
  iterm2-canvas.h   iterm2-canvas.cc
  jpeg-source.h     jpeg-source.cc
  kitty-canvas.h    kitty-canvas.cc
  renderer.h        renderer.cc
  terminal-canvas.h terminal-canvas.cc
  termutils.h       termutils.cc
  thread-pool.h
  timg-base64.h
  timg-png.h        timg-png.cc
  timg-time.h
  unicode-block-canvas.h unicode-block-canvas.cc
)

But of course, I have no idea about the consequences of this blind change, not even sure if syntax is correct, or what becomes of target_sources() with this change

[coldtobi]

But of course, I have no idea about the consequences of this blind change, not even sure if syntax is correct, or what becomes of target_sources() with this change

add_executable(timg timg.cc)
add_target_sources(timg PRIVATE
 (remaining source files)

will work. $Old CMake needs 1 file in add_executable, the rest can stay in add_target_sources.

[MestreLion]

(Coffee starts working… Managed to compile timg with CMake 3.7)

Thanks for taking your time to see this! I appreciate!

I'll try some of your suggestions, hold on...

[MestreLion]

add_executable(timg timg.cc)
add_target_sources(timg PRIVATE
 (remaining source files)

No add_target_sources, do you mean target_sources? I tried, and...

It worked. Flawleslly! Didn't bump into any other issues. Well, I didn't try make install yet (how can I change the prefix using CMake?), but at least compiling went smooth, and I have the executable at build/src/timg

Given that, would you guys mind doing this small change to CMakeLists.txt to allow other poor souls with half-aged (but still LTS) distros to run timg out of the box? I would gladly do a PR if you want

[MestreLion]

However, you will run into more issues:
target_compile_features(timg PRIVATE cxx_std_14)
needs a newer CMake as well; However, if you have a recent gcc (>6.3, IIRC), C++14 is already the default, so dropping that line should work there. (I wouldn't drop the line generally, as cxx_std_14 is supposed to be compiler agnostic…)

I didn't run into that... target_compile_features() is available since CMake 3.1 (not 3.11), and my GCC is 7.5, so std_14-ready.

Finally, install(TARGETS timg), will also need additional parameters. If you are only for a local build and do not want to install it, you can remove that line or specify the missing parameter (CMake will tell you. I didnt record the exact message.) This is likely quite easy to fix with include(GNUInstallDirs) and using the CMAKE_INSTALL_BIN variable, but I need to investigate, before I should say so…)

The acording to CMake docs, the only change from 3.10 to 3.11 is this

New in version 3.11: Many of the install() variants implicitly create the directories containing the installed files. If CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS is set, these directories will be created with the permissions specified. Otherwise, they will be created according to the uname rules on Unix-like platforms. Windows platforms are unaffected.

No new parameters, so good news! Does this note about implicit creation have any impact on timg's current install()?

[coldtobi]

Note: I had only CMake 3.7 at hand. My differences are possibly caused by that.
But it is good news, so reducing to CMake 3.10 is an option :)

Regarding the prefix: The default should be fine. as you don't want to install to /usr directly, only to /usr/local…
Said that, there's CMAKE_INSTALL_PREFIX as well…

[MestreLion]

Ok, tried installing using a home prefix and... man, this is beautiful:

$ cmake ../ \
    -DWITH_VIDEO_DECODING=Off \
    -DWITH_VIDEO_DEVICE=Off \
    -DWITH_OPENSLIDE_SUPPORT=Off \
    -DCMAKE_INSTALL_PREFIX:PATH=$HOME/.local && make && make install

-- The CXX compiler identification is GNU 7.5.0
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.29.1") 
-- Found Git: /usr/bin/git (found version "2.17.1") 
-- Checking for module 'GraphicsMagick++'
--   Found GraphicsMagick++, version 1.3.28
-- Checking for module 'libturbojpeg'
--   Found libturbojpeg, version 1.5.2
-- Checking for module 'zlib'
--   Found zlib, version 1.2.11
-- Checking for module 'libexif'
--   Found libexif, version 0.6.21
-- Checking for module 'libavutil'
--   Found libavutil, version 55.78.100
-- Checking for module 'libswscale'
--   Found libswscale, version 4.8.100
-- Looking for C++ include pthread.h
-- Looking for C++ include pthread.h - found
-- Looking for pthread_create
-- Looking for pthread_create - not found
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE  
-- Configuring done
-- Generating done
-- Build files have been written to: /home/rodrigo/install/timg/build
Scanning dependencies of target timg
[  6%] Building CXX object src/CMakeFiles/timg.dir/timg.cc.o
[ 13%] Building CXX object src/CMakeFiles/timg.dir/buffered-write-sequencer.cc.o
[ 20%] Building CXX object src/CMakeFiles/timg.dir/framebuffer.cc.o
[ 26%] Building CXX object src/CMakeFiles/timg.dir/image-display.cc.o
[ 33%] Building CXX object src/CMakeFiles/timg.dir/image-source.cc.o
[ 40%] Building CXX object src/CMakeFiles/timg.dir/iterm2-canvas.cc.o
[ 46%] Building CXX object src/CMakeFiles/timg.dir/jpeg-source.cc.o
[ 53%] Building CXX object src/CMakeFiles/timg.dir/kitty-canvas.cc.o
[ 60%] Building CXX object src/CMakeFiles/timg.dir/renderer.cc.o
[ 66%] Building CXX object src/CMakeFiles/timg.dir/terminal-canvas.cc.o
[ 73%] Building CXX object src/CMakeFiles/timg.dir/termutils.cc.o
[ 80%] Building CXX object src/CMakeFiles/timg.dir/timg-png.cc.o
[ 86%] Building CXX object src/CMakeFiles/timg.dir/unicode-block-canvas.cc.o
[ 93%] Linking CXX executable timg
[ 93%] Built target timg
Scanning dependencies of target man
[100%] Building manpage timg.1
[100%] Built target man
[ 93%] Built target timg
[100%] Built target man
Install the project...
-- Install configuration: ""
-- Installing: /home/rodrigo/.local/bin/timg
-- Installing: /home/rodrigo/.local/share/man/man1/timg.1

[MestreLion]

Even the man page worked... no sudo needed at all. Sweet!

So... shall I PR or no need?

[coldtobi]

So... shall I PR or no need?

Yeah, a PR would be cool.. (Afterall you did the hard work of testing according to my ramblings, so it should be your name attached to it ;-))
(Of course only if you want, if not I can follow up this evening)

[MestreLion]

I can do it... files in my local repo are already modified, so this is an easy commit...

[MestreLion]

try building timg with the old Makefile (I've provided CMake to be able to package timg for Debian**, so to have less pain there, especially if it comes to porting and maintainability.) But the old Makefile might just work well for your usecase…

Not relevant anymore, but, just to note the old Makefile is not in the repo anymore... can't cmake co-exist with autotools?

consider upgrading your installation to the current LTS supported by Canonical. While 18.04. is still supported by Canonical*, its starts showing its age.

Will do... in April 2022 :-)

It might be enough to get an container with a newer Ubuntu and just use it for compiling.

Now that was a sweet idea, thanks! It does not cover the install, that needs to be done in the real host anyway... but at least it's a viable option.

possibly you should ask them to provide a updated CMake... After all, its still supported… just kidding, they wont do it, its not how they define "supported".

"Supported" in Debian means "security updates", not "version upgrades". Even so, some hand-picked packages are elected to version upgrades, like Firefox. Not to mention the backports channel. But not CMake, obviously.

package timg for Debian [...] will happening soon(tm)

Really looking forward to that! Please try to land it before 2022! Will be sweet to sudo apt install timg without hassles!

Yeah, a PR would be cool.. (Afterall you did the hard work of testing according to my ramblings, so it should be your name attached to it ;-))

Done!

[hzeller]

Thanks @coldtobi for your guidance and @MestreLion to make it work!