don't depend on submodules for clap and clap-helpers #38
Conversation
| @@ -1,3 +1,6 @@ | |||
| # OS's | |||
| .DS_Store | |||
There was a problem hiding this comment.
these are all over the place on macOS, hope you don't mind
There was a problem hiding this comment.
I personally don't mind.
Though in the best practice, you could add those to your personal gitignore (global git config).
| @@ -1,10 +1,8 @@ | |||
| cmake_minimum_required(VERSION 3.17) | |||
| cmake_minimum_required(VERSION 3.24) | |||
There was a problem hiding this comment.
needed for the behavior that I want in FetchContent, I'll explain on another comment
There was a problem hiding this comment.
3.24 is OK, it is quite old already.
| @@ -40,6 +37,8 @@ else() | |||
| find_package(RtMidi CONFIG REQUIRED) | |||
| find_package(RtAudio CONFIG REQUIRED) | |||
| endif() | |||
| find_package(clap CONFIG REQUIRED) | |||
| find_package(clap-helpers CONFIG REQUIRED) | |||
There was a problem hiding this comment.
just wanted to get all the compiler flag stuff before all the find_package stuff
cmake/Dependencies.cmake
Outdated
| FetchContent_Declare( | ||
| clap | ||
| GIT_REPOSITORY https://github.com/free-audio/clap.git | ||
| GIT_TAG main | ||
| SYSTEM | ||
| # 'FIND_PACKAGE_ARGS' will skip download if | ||
| # the target is already available in the system | ||
| FIND_PACKAGE_ARGS NAMES clap | ||
| ) | ||
|
|
||
| FetchContent_Declare( | ||
| clap-helpers | ||
| GIT_REPOSITORY https://github.com/free-audio/clap-helpers.git | ||
| GIT_TAG main | ||
| SYSTEM | ||
| FIND_PACKAGE_ARGS NAMES clap-helpers | ||
| ) | ||
|
|
||
|
|
||
| FetchContent_MakeAvailable(clap clap-helpers) |
There was a problem hiding this comment.
This is the interesting bit: with FIND_PACKAGE_ARGS, if find_package is called and the package is already somewhere in the system, then FetchContent won't download (and build) the dependency.
I like it because it's very non intrusive: you can get your dependencies however it works for you (distro package manager, conan, homebrew, some custom cmake scripts, whatever) and, if none of that is available, FetchContent is used as a fallback to make things JustWork.
There was a problem hiding this comment.
I don't see the benefit.
clap-host and clap-plugin aren't meant to be submodules of other projects.
They need a precise version of clap and clap-helpers to be built with.
There was a problem hiding this comment.
Sorry, I think I didn't make myself clear: this is about adding clap and clap-helpers as dependencies of clap-host using native cmake features only. FetchContent can be thought of as a cmake native CPM, even if a little more lower level.
We can tie them to specific versions (that need to be git tags or branches in the fetched repos) using the GIT_TAG property, I'll update the PR.
There was a problem hiding this comment.
added 1.2.0 as the version for the clap dependency at https://github.com/free-audio/clap-host/pull/38/files#diff-12e8125164bbfc7556b1781a8ed516e333cc0bf058acb7197f7415be44606c72R6 (clap-helpers doesn't have numbered versions yet).
There was a problem hiding this comment.
Sorry, I think I didn't make myself clear: this is about adding
clapandclap-helpersas dependencies ofclap-hostusing native cmake features only.FetchContentcan be thought of as a cmake nativeCPM, even if a little more lower level.We can tie them to specific versions (that need to be git tags or branches in the fetched repos) using the
GIT_TAGproperty, I'll update the PR.
But having them as submodules does almost the same (tied to a version, can be fetched and so on.). And they are both not very large repos so it's no problem to have them on the drive twice. So I think it's fine to have them as submodules
There was a problem hiding this comment.
the biggest advantage is that, if you have a system wide installation of clap and clap-helpers, then FetchContent will use that instead of downloading their sources (fixing issues like #29).
Same would work if you call, say, cmake .. -DCMAKE_PREFIX_PATH=/some/folder/with/clap/and/clap-helpers/installed: if cmake can find the packages ready to use it'll use them, and fallback to downloading their sources only if needed (as opposed to the git submodules in which they always have to be downloaded).
There was a problem hiding this comment.
also, I'm probably not the only one who constantly forgets the --recurse-submodules on git clone and/or git submodule update --init, but that's on me.
|
Could it be that you want to add the clap-host as a submodule of your own project, for having simpler launchers? |
not really, I really just see |
|
no problem, folks, I'd personally prefer a consistent way of dealing with the dependencies but what's here now makes sense too. Cheers! |
Hi folks!
I'm not that sure about this one, but thought it would be better to propose a PR directly instead of discussing different approaches beforehand, so feel free to give any feedback or even close it if you already have anything else in mind.
This solves #29 (by removing the submodules), and with some work on the
RtAudioandRtMidirepos (to make them moreFetchContentfriendly) could also in the future solve #28 (RtAudioversion mismatch), #27 (whatever problempkg-configis having), #17 (old versions packaged on distro) and #2 (depend onvcpkg).Actually with a little hacking even a small, static
Qtcould be made to work: I almost made this PR withQt,RtAudioandRtMidi, but couldn't get them to behave consistently on all three platforms (yet)... 😢I've seen you're using
CPMon clap-helpers. I like it, but to me it looks like for simpler use cases like these just vanillacmakeis enough, that's why I wanted to check this out with you folks.Thanks for the great work on clap!