Cycling74 / min-devkit

Tools, documentation, and reference implementation of a Max Package built using the Min-API.
MIT License
161 stars 31 forks source link

CMake build system modification #204

Closed Mc-Zen closed 1 year ago

Mc-Zen commented 2 years ago

Hi there,

In the documentation you say that you are generally open to pull requests. Does that also apply to the CMake build system?

I noticed it is hard to create a structure that is more than just a single and simple external. Especially, the C++ version cannot be changed for an external in a straightforward way and linked libraries don't behave quite as they should. Also, the projects need to lie in the projects folder which makes it a bit tiresome to use version control and still keep updated with the min-devkit while also linking to other libraries.

If you were open to accept a modification I could maybe offer to propose a new backwards-compatible solution in form of a pull request. This would most likely also affect the submodules, naming min-api, max-sdk and min-lib. The rough idea is to

Some advantages would be:

Maybe some target exports could also be added, so the entire sdk could be found via find_package() but I'm not sure yet if this is feasible or useful.

Let me know if this would be desired, Cheers

isabelgk commented 2 years ago

Hi @Mc-Zen !

In the documentation you say that you are generally open to pull requests. Does that also apply to the CMake build system?

We'd be happy to review a PR for the CMake build system!

I noticed it is hard to create a structure that is more than just a single and simple external. Especially, the C++ version cannot be changed for an external in a straightforward way and linked libraries don't behave quite as they should. Also, the projects need to lie in the projects folder which makes it a bit tiresome to use version control and still keep updated with the min-devkit while also linking to other libraries.

I'm curious about the issues you've run into with linked libraries not behaving properly. Also, what have you found tiring about the projects folder with regard to version control?

propose a new backwards-compatible solution

Great -- backward compatibility would be important to us! The advantages of the rough idea you've proposed sounds promising and likely useful for users.

Maybe some target exports could also be added, so the entire sdk could be found via find_package() but I'm not sure yet if this is feasible or useful.

A few things to note here:

All this said, I can't guarantee we'll accept the PR, but this sounds promising and we'd be happy to review the work. 😃

Mc-Zen commented 2 years ago

Hi @isabelgk ! Thanks for your thorough answer.

I'm curious about the issues you've run into with linked libraries not behaving properly. Also, what have you found tiring about the projects folder with regard to version control?

Actually, I think I need to take back my statement about linked libraries not behaving properly. We tested this particular issue again and it seems there were some cache issues when the library was added. The C++ standard or other flags would need to be changed after the postscript which is not obvious to find out and the modern approach using CMAKE_CXX_STANDARD and co of course does not work. As for version control, it is firstly discouraged (though not quite impossible) to use nested git repositories without submodules but also it seems a little bit laborious and unsuitable to us to insert and maintain an entire project with submodules etc. of its own inside the projects directory. On this note, maybe a better option for us would be just to include the min-api and min-lib as submodules instead of using the min-devkit repository at all.

The find_package() integration might make more sense in the min-api rather than the min-devkit since the devkit is essentially an example package using the min-api. (Analogously, the max-sdk is an example package using the max-sdk-base.) Since the changes to CMake you're proposing would be necessary throughout the different repositories, you might already know this distinction. We have an example of using the conan package manager with the min-api here, if this is of interest for you. https://github.com/Cycling74/min-api/blob/main/Conan.md

Yes definitely, find_package() would be more useful in the min-api. I'm aware of it and thanks for the hint with conan :)

I'd have one question beforehand: currently, the max-sdk-base has no CMakeLists.txt - is it desired to be kept this way? Otherwise, a max-sdk-base target could be added there to simplify appending c74support, jit, max and msp include paths as currently done by the max_pretarget.cmake script.

isabelgk commented 2 years ago

On this note, maybe a better option for us would be just to include the min-api and min-lib as submodules instead of using the min-devkit repository at all.

Yes, this is what I'd recommend! The min-devkit repository is more of an example of what a package made with min would look like rather than a repository you'd want to consume in your own package.

I'd have one question beforehand: currently, the max-sdk-base has no CMakeLists.txt - is it desired to be kept this way? Otherwise, a max-sdk-base target could be added there to simplify appending c74support, jit, max and msp include paths as currently done by the max_pretarget.cmake script.

I don't see why adding one would hurt! This made me wonder why we don't already have one, actually. I would guess it's the fact that max-sdk-base is a newer addition since the transition to CMake with Max 8.2. Internally we consume those headers in a different CMake setup so we wouldn't have run into this. That's all to say, the omission is not intentional as far as I'm aware!

Mc-Zen commented 2 years ago

Alright, thank you for your quick reply!

I have created something like a draft and I would start by creating a pull request for max-sdk-base adding a CMakeLists.txt as well as a cmake file that includes a function which performs the setup that is otherwise done by max-pretarget.cmake and min-pretarget.cmake

scblakely commented 1 year ago

I would certainly appreciate some better (or simpler) guidance on how to use CMake to link an external library (on an arbitrary path) with a min project.

I don't understand CMake, and trying to link my project to an arbitrary .lib file already compiled from a solution is doing my head in. I have managed the include file, but getting the subsequent project to link has proved impossible.

isabelgk commented 1 year ago

Hi @scblakely , I'm going to move your request to a new ticket and follow up on it there since I'm going to close this issue for now.

isabelgk commented 1 year ago

See #208