Cmake

[NaLiJa]

Bring macos demo fixes to new cmake files

[mikke89]

Our cmake branch doesn't really have any changes right now, since it's waiting for #446. Do you perhaps mean to apply these changes on top of that PR instead? Would be interesting to see how that goes. Otherwise, it makes more sense to put this on master in the meantime I think.

[NaLiJa]

Yes, this was meant for the modernised cmake file to carry over the fixes that we recently applied to the current cmake file. It is actually the same fix, but since the modernised version puts more stuff into separate files, I brought the helper function to a external file as well.

[hobyst]

If this is meant to be on top of the modernized CMake PR, then here's a few comments:

• That method of adding resources is a big no no. It uses file(GLOB_RECURSE ...), which not only isn't recommended by the Modern CMake guide (see "CMake Antipatterns") but it also makes CMake search those directories for files every time the project is re-configured even if no new files were added or modified.
• The code has very little comments, making it difficult to understand.

Since CPack already supports creating macOS bundles, and these bundles are nothing but structured directories, it should be possible to declare the samples as non-MACOS_BUNDLE targets and have them behave the same way as they do on Linux and Windows for development setups. Then, if desired by the consumer, run CPack to generate a self-contained macOS bundle. This way it should be possible to still generate macOS bundles using the same workflow as the rest of platforms and without file(GLOB_RECURSE ...). It should be possible to add the necessary asset files to each bundle calling something along the lines of install(DIRECTORY "<assets_dir>" DESTINATION "Contents/Resources/...").

If CPack were found not to be a proper solution for this, it could be possible to add a call to cmake -E copy_directory ... or similar at build time using add_custom_command() only when the MACOS_BUNDLE property of the target is set to TRUE.

[NaLiJa]

Well, I must confess, I am somewhat irritated anout this comment as the above change is the same change we applied to the current cmake file.

The point is to make the samples immediately work. Without those changes, you compile the demos and when trying to run they fail without further notice because of the missing assets. And IMHO this is a very bad first impression. The Windows/Linux builds are not the same as the executable of the mac version which is deeper inside the bundle folder structure. So you either copy the assets into the bundle or you add custom code to the find the assets.

A self-contained bundle is yet another challenge as you would need to add all the dependencies like sdl, glfw whatever to the bundle as well. And even if you manage to do this, you will probabaly need to run post-copy tasks to fix the LC_PATH in the libs.

If you follow discussions on the net you will notice many people reporting that the cmake bundles tools are not working as expected. It might be necessary either way to post-provess the bundle files to fix things up.

To my impressions, the GLOB discussion is usually the other way around, but I might be wrong. It might be problematic that you need to re-configure if contents in the glob'bed folders have changed to be recognized. I is hard to understand that the time penalty during re-configuration is critical as re-configuration is presumably a task that is rarely done once a project is set up.

You could replace the dirctory scanning by explicitely lising all asset files individually if this really deems necessary. And you could throw the bundles out and build single executables. Then, you do not have to deal with the bundle stuff, but it is not really macos-like anymore. I havn't considered this, because I tried to keep the existing build process intact and just fix non-working stuff. Well, and of course after all, you can simply ignore the PR if you don't see it fit.

[mikke89]

First of all, I appreciate the continued efforts here. I believe both of you have completely valid concerns. I hope we can make it easy for macOS users to integrate the library, and simultaneously keep our new CMake code clean with modern practices.

With that said, this PR itself is a very minor refactoring. The cmake branch currently doesn't actually contain anything yet and this code would have to be implemented / refactored again for #446. It would be more valuable to try to make bundling work on top of the changes in #446. For these reasons, I am closing this one. But I want to encourage you to keep contributing toward our goals here.