CMake Refactor

[swagtoy]

This is a PR that tries to refactor the CMakeFiles.txt, splitting some things into other files, cleaning up the "Provides" nonsense, and... making the game actually build.

For example: libphysfs requires manually copying the DLL on Windows (as well on Linux), but I've personally had linking issues on OpenSUSE. I've unfortunately just never succeeded in building this game.

It was planned to be a rewrite, and it kind of is, but it's mostly just axing stuff from the current files. It will need plenty of testing.

TODO

• Remove Supertux2_lib and Supertux2_c
• Switch to find_package for things that can be a find_package
• Use find_package for external stuff when needed
• Linux support with PkgConfig/find_package
• Switch from GoogleTest to CMakeTest
• "Cherrypick" Display branch name and commit hash in window title #3001
• MacOS (need someone to test this all)
• Windows installer

It (will soon) also removes the Supertux_lib split, making the tests instead depend on the specific files being tested, as well as using CMake's built-in testing functionality instead of GoogleTest.

[swagtoy]

I hacked PkgConfig to work for just my system temporarily (as CMake files for many libraries actually already exist, except for Ogg which seemed finnicky), but how should I tackle find_package for non-existant things and then external being used as a fallback? I guess I could provide my own FindXXXXX.cmake files? OR what about those XXXX-config.cmake files?

[MatusGuy]

My suggestion would be to only support vcpkg but if you really need that you can use the XXX_FOUND variable to check if the find package command was successful.

[MatusGuy]

I guess a modified FindXXXX.cmake could work. Basically the better ProvideXXXX.cmake

[swagtoy]

My suggestion would be to only support vcpkg but if you really need that you can use the XXX_FOUND variable to check if the find package command was successful.

Well, I'm building on Linux now, and find_package actually works for that too (for most things). If you take a look at your Linux /usr/lib{64,}/cmake directory you will find a ton of libraries that provide CMakefiles, as well as /usr/share/cmake which has some more generic ones for stuff like CURL and all that.

But I think I can do a fallback for if a package doesn't get found, then use pkg_config, then use external/asdfghjk.

Even better than custom FindXXXX.cmake files, I could even write a fancy macro like add_package, which basically does what the Ogg hack does, but it takes more arguments such as the pkg-config fallback packages, as well as an external directory if it exists. Then, for example, say that the CMake files provided break on someones system (but they DO exist), we could have a setting you could pass to just prefer external or prefer PkgConfig).

The macro would accomplish this without duplicating a lot of files, I think.

[MatusGuy]

You should 100% support pkg config but if you do you should add an option to disable it because if i recall correctly @mrkubax10 would want that? Also its just handy in general.

[swagtoy]

pkgconfig will generally speaking be a fallback option if a FindXXXX.cmake doesn't exist in the System cmake directory, but again; using a macro, I could probably have some variables to basically ignore the pkg-config fallback

[swagtoy]

A quick reference for anyone curious about the add_package command, it's very similar to the find_package command, but you can also specify the Provides files (not implemented as of typing this) or the PkgConfig fallbacks. It essentially lets me define a package in one line.

EX:

add_package(SDL2              # <-- the "output" target
    PKG SDL2                  # <-- The find_package package, in this case it gets SDL2 (might look similar to the "output" target)
    CONFIG                    # <-- (optional) Passed to find_package if its a CONFIG
    REQUIRED                  # <-- (optional) NOT passed to find_package, just a check to throw an error if nothing is found
    PKG_CONFIG sdl2 sdl2_ttf  # <-- (optional, recommended) List of packages for PkgConfig
    PROVIDES ProvideSDL2      # <-- (optional) Fallback to just look at the provided file. Undecided if I should fall back to a FindXXXX.cmake yet
)   

[MatusGuy]

Remember to include these packages in the vcpkg example!

[tobbi]

I'm getting a couple errors on OS X (I tried out the branch in a new worktree):

tobiasmarkus@Tobiass-MBP-3 cmake-refactor-supertux % ./configure
-- Size of void* is 8
CMake Error at mk/cmake/SuperTux/AddPackage.cmake:62 (get_target_property):
  get_target_property() called with non-existent target "SDL2_ttf::SDL2_ttf".
Call Stack (most recent call first):
  CMakeLists.txt:139 (add_package)


CMake Error at mk/cmake/SuperTux/AddPackage.cmake:64 (add_library):
  add_library cannot create ALIAS target "SDL2_ttf" because target
  "SDL2_ttf::SDL2_ttf" does not already exist.
Call Stack (most recent call first):
  CMakeLists.txt:139 (add_package)


-- Could NOT find Ogg (missing: Ogg_DIR)
-- Checking for one of the modules 'ogg'
-- Could NOT find Vorbis (missing: Vorbis_DIR)
-- Checking for one of the modules 'vorbis'
-- Could NOT find Vorbis (missing: Vorbis_DIR)
-- Could NOT find glm (missing: glm_DIR)
-- Checking for one of the modules 'glm'
CMake Error at /usr/local/share/cmake/Modules/FindPkgConfig.cmake:938 (message):
  None of the required 'glm' found
Call Stack (most recent call first):
  mk/cmake/SuperTux/AddPackage.cmake:77 (pkg_search_module)
  CMakeLists.txt:145 (add_package)


-- Could NOT find fmt (missing: fmt_DIR)
-- Checking for one of the modules 'fmt'
CMake Error at /usr/local/share/cmake/Modules/FindPkgConfig.cmake:938 (message):
  None of the required 'fmt' found
Call Stack (most recent call first):
  mk/cmake/SuperTux/AddPackage.cmake:77 (pkg_search_module)
  CMakeLists.txt:146 (add_package)


-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY - Success
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY - Success
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
-- Performing Test STD_C__0X_CXX_FLAG_FOUND
-- Performing Test STD_C__0X_CXX_FLAG_FOUND - Success
-- Performing Test O3_CXX_FLAG_FOUND
-- Performing Test O3_CXX_FLAG_FOUND - Success
-- Performing Test WALL_CXX_FLAG_FOUND
-- Performing Test WALL_CXX_FLAG_FOUND - Success
-- Performing Test WEXTRA_CXX_FLAG_FOUND
-- Performing Test WEXTRA_CXX_FLAG_FOUND - Success
-- Performing Test WEFFC___CXX_FLAG_FOUND
-- Performing Test WEFFC___CXX_FLAG_FOUND - Success
-- Performing Test PEDANTIC_CXX_FLAG_FOUND
-- Performing Test PEDANTIC_CXX_FLAG_FOUND - Success
-- Checking for OpenGL
--   OPENGL_LINK_LIBRARIES: /Volumes/Development/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk/System/Library/Frameworks/OpenGL.framework;/Volumes/Development/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk/System/Library/Frameworks/OpenGL.framework;GLEW::GLEW
--   OPENGL_INCLUDE_DIRECTORIES: /Volumes/Development/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk/System/Library/Frameworks/OpenGL.framework;/usr/local/include
--   OPENGL_COMPILE_DEFINITIONS:
-- Configuring incomplete, errors occurred!

[swagtoy]

x_x

fatal: No names found, cannot describe anything.
-- "/usr/bin/git describe;--tags;--abbrev=0" failed with result "128".
fatal: ambiguous argument '_tag-NOTFOUND..HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
-- "/usr/bin/git rev-list;_tag-NOTFOUND..HEAD;--count" failed with result "128".

[swagtoy]

This still builds on my machine which means my machine is superior. I'm now not sure how we work around weird cases like Vankatas. The only thing that comes to mind is using one of CMake's LSB (or whatever its called) variables and just adding a "WORKAROUND" option for specific packages, so WORKAROUND Arch would skip the CMake (typically the culprit) and instead rely on pkg-config + any built-in files for said distribution until Arch developers decide to be competent one day. Is this okay @MatusGuy ? Or do you have a better solution in mind?

[swagtoy]

@tobbi brew install sdl2_ttf libvorbis glm

[swagtoy]

-- Could NOT find Ogg (missing: Ogg_DIR)
-- Checking for one of the modules 'ogg'
CMake Error at mk/cmake/SuperTux/AddPackage.cmake:88 (message):
  Package "Ogg" couldn't be found with pkg-config, but it's required.

  I don't know what to do.  Is it installed?

  Tried: ogg
Call Stack (most recent call first):
  CMakeLists.txt:148 (add_package)

x_x

[swagtoy]

ogg is also installed on my machine. @MatusGuy I think Vankata had a similar issue. There is a regression with the pkg-config stuff (or it never even worked regardless.) I can take a stab at it after work

[MatusGuy]

Aw snap! Found the issue. PKG_FOUND is a leftover from my drafts 😅

supertux/mk/cmake/SuperTux/AddPackage.cmake

Line 87 in dd18bf0

if(NOT PKG_FOUND AND addpackage_args_REQUIRED)

[swagtoy]

Ha, thank you!

[tobbi]

CMake Error at mk/cmake/SuperTux/AddPackage.cmake:67 (get_target_property):
  get_target_property() called with non-existent target "SDL2_ttf::SDL2_ttf".
Call Stack (most recent call first):
  CMakeLists.txt:142 (add_package)


-- Package "SDL2_ttf" is an alias. Realiasing it.
CMake Error at mk/cmake/SuperTux/AddPackage.cmake:73 (get_target_property):
  get_target_property() called with non-existent target "SDL2_ttf::SDL2_ttf".
Call Stack (most recent call first):
  CMakeLists.txt:142 (add_package)

It seems like the fallback modules in external/ are not taken into account.

[MatusGuy]

That's because we're actually removing them

[swagtoy]

@tobbi I think we considered removing SDL2 TTF because we dont believe we needed to maintain a fork. Its a decision we probably shouldn't take lightly, but native builds of sdl2-ttf seem to work fine so far.

Can you try building with perhaps a native install of SDL ttf? This should eliminate the error.

[tobbi]

@tobbi I think we considered removing SDL2 TTF because we dont believe we needed to maintain a fork. Its a decision we probably shouldn't take lightly, but native builds of sdl2-ttf seem to work fine so far.

Can you try building with perhaps a native install of SDL ttf? This should eliminate the error.

Native SDL_TTF has no support for RAQM for Arabic language suppport.

[tobbi]

I think string_view should be passed by reference (even though it's not as expensive as std::string)

[MatusGuy]

string_view provides read-only access to an existing string, it does not represent the string itself. Therefore you can pass it as value

[tobbi]

I recently got a new color printer, so, color me surprised. I didn't know that.

[swagtoy]

Actually I kind of took note with Tobbi on one thing, I dont even need to use string view here. I think I can just use a good old const char* since im just printing the value out.

[MatusGuy]

Native SDL_TTF has no support for RAQM for Arabic language suppport

... because it's not needed. You can find examples and API docs on people using arabic script with sdl2_ttf.

Also, is it really worth it to make half-baked arabic language support without supporting RTL first?

[swagtoy]

I would really prefer if we tested arabic scripts instead of kinda throwing knives. Lets see if we can live without any custom patches again with visual proof. I'd imagine the team up around SDL2 TTF would actually have such a thing sorted out (i skimmed our fork and it does look like we just applied weird patches overtop of it...), especially since this uses Truetype; i feel like we (me & Marty) are too hasty to conclude on this.