Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove .gitmodules and use CPM to fetch dependencies instead #139

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

multivac61
Copy link

We are already using CPM for fetching JUCE in the examples. Much nicer to use them for everything instead of .gitmodules that just add needless complexity, confusion and extra step when building 😊

@multivac61 multivac61 force-pushed the migrate-from-git-submodules-to-cpm branch 2 times, most recently from 6f04bb6 to 64a7ca8 Compare November 19, 2023 00:43
@baconpaul
Copy link
Collaborator

baconpaul commented Nov 19, 2023

So the reason we didn’t do this is because quite a few of our target environments have network free build stages. If we are going to do cpm we need a bailout which lets you specific a pre downloaded location (which is the strategy we tok with the clap wrapper)

the examples are built in ci and by developers so the constraints are more relaxed, but having those have a juce root would also be lovely.

But i don’t think we can merge this change as is. It will break some of our client users.

@baconpaul
Copy link
Collaborator

Also it seems the projucer workflows failed to work with the change.

The strategies folks seems to use are

  1. copy the deps. This is how juce deals with vst3 and AU
  2. Submodule so the network access is at checkout time
  3. Cpm with configurable bailout. So do if target checks and look for things like clap root and obey that if set - check out the guarantee functions in the clap wrapper for this

all of those allow a build of the core lib in an un networked env. Moving from 2 to 3 will break some of our users though since they will have to reconfigure their builds to do the checkouts at source assembly time.

@baconpaul
Copy link
Collaborator

baconpaul commented Nov 19, 2023

Oh another alternative I just thought of. You could use File in cmake to check if the submodule stage has been run. If it has not you could print a warning and run Cpm. That would retain the submodules for current workflow but also self heal with Cpm if you don’t do the proper check out. That’s the least disruptive strategy.

Need to make sure we sync the Cpm and git module versions then so a small workflow cost.

alternately we could have the same check do a clear error message with a fatal error which is lowest complexity

@multivac61
Copy link
Author

Thank you for getting back to me @baconpaul. Love the project!

You can tell CPM to rely solely on local dependencies, no problem. It will prioritise local packages and revert to download if it doesn't find them

    "-DCPM_USE_LOCAL_PACKAGES=ON"
    "-DCPM_LOCAL_PACKAGES_ONLY=ON

@baconpaul
Copy link
Collaborator

Thank you for getting back to me @baconpaul. Love the project!

You can tell CPM to rely solely on local dependencies, no problem. It will prioritise local packages and revert to download if it doesn't find them

    "-DCPM_USE_LOCAL_PACKAGES=ON"
    "-DCPM_LOCAL_PACKAGES_ONLY=ON

Yeah but then it uses findoackage right? Which isn’t set up with clap helpers and clap

the more I think about it the idea of if the submodule are empty then try cpm seems most appropriate

we can even set up a ci test of that to either not clone or to remove the submodule before running

(If this was an end product I would totally be fine with cpm and we use it for all our test cases but libraries and included things I’ve generally been a bit more careful)

@jatinchowdhury18
Copy link
Collaborator

It is also possible to use CPM's "local package override" to try CPM to use a locally downloaded copy of the dependency repo rather downloading it. Not sure exactly where this fits into the conversation, but it felt worth mentioning.

https://github.com/cpm-cmake/CPM.cmake#local-package-override

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants