CMake refactoring: add CMakeLists.txt and function add_max_target()

[Mc-Zen]

Changes by adding a CMakeLists.txt are:

• max-sdk-base gets a CMakeLists.txt file and provides an interface library so that when a target is linked against it, the include paths for c74support, jit-includes, max-includes and msp-includes are inherited.
• A global property is used to store max-sdk-base's path which is necessary for some steps when an external is created in order to remain independent from the callers directory (which may be min-api or some day even max-sdk (?)).
• The max-sdk-base library gets a folder (C74_Libraries) for possible organisation in an IDE.

Changes in comparison to the scripts that are currently used:

• A function add_max_target() is used to create a library target (i.e. an external) with the necessary configuration that is otherwise done by the pre- and post-scripts. This function will be called by a new function add_min_target() from the min-api. A MODULE library is already created here.
• The targets output path is NOT set to ../../../externals (as done in the script) because this only works when used in the exact same setup as in the min-devkit. Instead, a corresponding function in the min-devkit will then do this.

Note:
jit, max and msp could also have their own interface libraries but I'm not sure if it would be useful to include just some instead of all of them.

Outlook:
A CMakeLists.txt file that is located in the min-devkits projects folder could later look like this:

set(target mc.min.info_tilde)
add_project(${target} SOURCES mc.min.info_tilde.cpp)	

[isabelgk]

Hi @Mc-Zen !

Would you be able to reduce the amount of duplication between script/add_max_target.cmake and script/max-pretarget.cmake without breaking backward compatibility of the pretarget script?

[Mc-Zen]

Hi,

I did try that but it is nothing less then trivial and I'm not sure 100% backwards compatibility can be guaranteed.

The file script/max-pretarget.cmake mostly sets variables which makes it hard to use functions as they are scoped. Using macros solves this issue mainly but there needs to be a common CMake file containing these functions. This file (let's say macros.cmake) will then be included in both script/max-pretarget.cmake and script/add_max_target.cmake.

The issue with that is that include() does not set the relative directory and nested includes are more than messy. As an example take the min-api directory:

• max-sdk-base
  • script
    • macros.cmake
    • max-pretarget.cmake
    • add_max_target.cmake
  • CMakeLists.txt
• script
  • min-pretarget.cmake

For add_max_target.cmake this is no problem because it is only ever included by the CMakeLists.txt and when including macros.cmake from CMakeLists.txt we can just use include(script/macros.cmake).

However it is not clear how max-pretarget.cmake should include macros.cmake because that depends on the relative path of the file including min-pretarget.cmake. Given this only occurs in the min-devkit projects folder, that's okay, but shouldn't the max-sdk-base better not depend on its child repositories? At least, this would need to be handled with a lot of care, especially in the future.

Another option is to include(source/min-api/max-sdk-base/script/macros.cmake) in the min-devkits CMakeLists.txt file but again then max-pretarget.cmake and even min-pretarget.cmake do not work on their own. Admittedly, you might say this is not a problem because they do not work on their own anyway - depending on some other files that need to be included breforehand (e.g. git-rev.cmake etc).

[Mc-Zen]

Oh and by the way: i have some questions

• Is the missing linking to MaxAPI on the apple platform (in contrast to windows) intentional/this library not needed on mac?
  

  max-sdk-base/script/max-posttarget.cmake

  Lines 23 to 41 in b6d635c

  	find_library(
  	MSP_LIBRARY "MaxAudioAPI"
  	REQUIRED
  	PATHS "${MAX_SDK_MSP_INCLUDES}"
  	NO_DEFAULT_PATH
  	#only use the specific path above, don't look in system root
  	#this enables cross compilation to provide an alternative root
  	#but also find this specific path
  	NO_CMAKE_FIND_ROOT_PATH
  	)
  	target_link_libraries(${PROJECT_NAME} PUBLIC ${MSP_LIBRARY})
  	find_library(
  	JITTER_LIBRARY "JitterAPI"
  	REQUIRED
  	PATHS "${MAX_SDK_JIT_INCLUDES}"
  	NO_DEFAULT_PATH
  	NO_CMAKE_FIND_ROOT_PATH
  	)
  	target_link_libraries(${PROJECT_NAME} PUBLIC ${JITTER_LIBRARY})

  I just noticed it when moving the linking to the interface target max-sdk-base and wasn't sure about that.
• I think, officially CMAKE_OSX_ARCHITECTURES and CMAKE_OSX_DEPLOYMENT_TARGET should be set before project() see here

[Mc-Zen]

Actually, I found a solution for the issue above with including included CMake files and most of script/max-pretarget.cmake can be moved to macros which also can be used from the new function.

However: for one step, it would be necessary to swap the add_subdirectorys for min-lib and min-api needs in min-devkit. This is because min-lib refers to max-sdk-base scripts in the unit tests although it is not a submodule. So, the min-api and thus max-sdk-base should be initialized before min-lib is.

I pushed the draft. It may need some testing for other projects than the ones in the min-devkit but there is seems to work well now!

[isabelgk]

@Mc-Zen This is looking good so far!

it would be necessary to swap the add_subdirectorys for min-lib and min-api needs in min-devkit. This is because min-lib refers to max-sdk-base scripts in the unit tests although it is not a submodule. So, the min-api and thus max-sdk-base should be initialized before min-lib is.

In that case, I would like to wait on approving/merging/etc. this PR before seeing proposed changes to other repositories as well.

[isabelgk]

Hi @Mc-Zen,

Just checking in on this -- thanks for patience. Are the other PRs on min-api and min-devkit also ready? I didn't want to add any comments until all repositories' PRs were ready.

[Mc-Zen]

Hi @isabelgk

Yes, they are ready - at least I thought so. Last week I tried to set up some extended unit testing for a max external and had some problems with a more complicated setup. A part of it is a typo made by me in the PR and another is a global CMAKE parameter that is set but here an elegant solution exists. I will add these two fixes very soon.

The other issue is a bit harder. Whenever an external uses functions from MaxApi, MaxAudioApi or Jitter (and most would), a test fails to compile because of missing linkage to those libraries. This was already the case before I made changes to the build system. We can add the missing library targets as we do for the external itself. However, all of these libraries have a dynamic library component which needs to be loaded at runtime. I believe Max 8 usually takes care of this - and for Windows the required dll files are found in the Max 8 installation. But of course the test runner will not find them.

Do you have a suggestion for that? It would be great to be able to write proper unit tests for max externals. What does work is copying the dlls into the test folder ... but either each "cloner" of the min-devkit needs to do this on their own or the files would need to be added to the repository :/ both don't seem great.

But the latter issue does not need to be solved now, just wanted to throw it in for consideration.

[Mc-Zen]

A part of it is a typo made by me in the PR and another is a global CMAKE parameter that is set but here an elegant solution exists. I will add these two fixes very soon.

Done. Everything ready so far

[isabelgk]

Thanks for all the work @Mc-Zen ! I'll need a bit of time to review the changes thoroughly before being able to let you know how we'll proceed.

Whenever an external uses functions from MaxApi, MaxAudioApi or Jitter (and most would), a test fails to compile because of missing linkage to those libraries.

I don't have a suggestion about this at the moment, but since it sounds like it is unrelated to your changeset, perhaps a separate issue thread would be appropriate and we can take a peek later on.

[isabelgk]

Hi @Mc-Zen ,

As a first test of these changes, I used the percolate package which just uses the max-sdk-base as a dependency.

Without changing any CMake files in the existing project, I get the following errors when configuring:

❯ cmake -G Ninja ..
-- The C compiler identification is AppleClang 13.0.0.13000029
-- The CXX compiler identification is AppleClang 13.0.0.13000029
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at source/max-sdk-base/script/macros.cmake:62 (file):
  file STRINGS file "/script/max-linker-flags.txt" cannot be read.
Call Stack (most recent call first):
  source/max-sdk-base/script/max-pretarget.cmake:13 (c74_max_post_project_calls)
  source/projects/JC_rev~/CMakeLists.txt:1 (include)


CMake Error at source/max-sdk-base/script/max-posttarget.cmake:23 (find_library):
  Could not find MSP_LIBRARY using the following names: MaxAudioAPI
Call Stack (most recent call first):
  source/projects/JC_rev~/CMakeLists.txt:34 (include)


-- Configuring incomplete, errors occurred!

[Mc-Zen]

Hi @isabelgk ,
max-sdk-base is now intended to be add_subdirectory'd*! I should have mentioned that but didn't think of other repos using max-sdk-base.

Adding add_subdirectory(source/max-sdk-base) somewhere early in percolate's CMakeLists.txt file (e.g. after the project() call) should be enough to solve the issue.

*this was necessary in order to reduce the code duplications

[isabelgk]

Thanks for the explanation! Unfortunately, this requirement (of needing add_subdirectory(source/max-sdk-base)) is a breaking change that we wouldn't want to introduce. I see why it's required to reduce duplication, though I still don't want to introduce code duplication either.

[Mc-Zen]

Mmh alright, I understand that. It would have been nice however to review the changes for max-sdk-base before I invested the work into min-api and min-devkit.

There is certainly a way to make it work without the CMakeLists.txt but this would be a very tedious job because the built system was designed in a way that many hardcoded, relative paths go to a lot of places which does not scale too well with the multirepo/submodules approach. The main issue is related to things like getting max-linker-flags.txt from other directories. With functions, targets etc. this is possible but the existing scripts rely on the calling/including file to be at exactly the right level of nesting relative to the file (e.g. max-linker-flags.txt)

[isabelgk]

I agree that such a refactoring could be quite time-intensive and require a lot of testing to ensure we don't break folks' current workflows (including internal ones within Cycling '74), though the requirement of not breaking older workflows at the moment is a priority.

Also you're right that it would have been better to have had testing on these changes before having you invest time into min-api and min-devkit. I apologize for that.

[Mc-Zen]

Well, it is how it is and of course not breaking compatibility is important.

I even looked into the thing again but I'm afraid the issue sits quite deep :/

Anyhow, I wish you a nice weekend!