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

New attempt at fetching submodules #3957

Merged
merged 4 commits into from
Nov 15, 2017
Merged

New attempt at fetching submodules #3957

merged 4 commits into from
Nov 15, 2017

Conversation

tresf
Copy link
Member

@tresf tresf commented Nov 8, 2017

The new technique parses the .gitmodules file and clones each submodule by path, unless the module was specified in SKIP_SUBMODULES.

To test, just clone this branch without the --recursive option and try to run cmake.

Advanced usage:

# Clones everything except plugins/LadspaEffect/swh/swh
SET(SKIP_SUBMODULES swh)
INCLUDE(CheckSubmodules)

Or via CLI:

# Clones everything except zynaddsubfx and rpmalloc
cmake -DSKIP_SUBMODULES=zynaddsubfx;rpmalloc

Most of the time, users shouldn't even know this module exists. It'll just be part of the codebase to improve productivity by reducing the number of build and configure errors directly related to submodule cloning.

  • Eventually we may want to bind the ADD_SUBDIRECTORY(...) to be cognizant of the submodule status, but that is out of scope for this PR.
  • Eventually we may want to check if a submodule is out of date and warn the user. This may happen if a branch hasn't recently been pulled, or if switching to a branch with a different submodule commit. This should be rare and is out of scope for this PR.

Once tested and merged to master, this PR should be safe to cherry-pick back to stable-1.2.

(Editorial)

Submodules are annoying.

As the author of a few submodule PRs, I'm continuously forgetting --recurse or I'm switching branches without re-running git submodule update --init --recursive. Sometimes cmake puts files in the directories and makes them non-clean so I have to clean them before running the commmands again.

To make things worse, I still haven't memorized if it's init --update or update --init (it's the latter, of course).

If I'm this frustrated after a week of working on submodules, newcomers will have an even harder time.

That said, submodules are great! They allow us to pull directly from upstream without the mess of cluttering our codebase and risk not submitting patches to the right area.

So we have to live with them. This is my second attempt to make that happen. :) -Tres

if (INCLUDE_DIRS)
TARGET_INCLUDE_DIRECTORIES(lmmsobjs PRIVATE ${INCLUDE_DIRS})
ENDIF()
IF(TARGET ${LIB})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already did this in 0dbdafc (albeit with inconsistent indentation...), it just hasn't been merged back to master yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think I'll keep it here and we can deal with the minor merge conflict when rebasing.

@tresf
Copy link
Member Author

tresf commented Nov 15, 2017

It's been 8 days without any testing feedback, but in that time, we've merged quite a bit of submodules making this needed more and more each day. Calf, for example, is huge and the only way I've found to reduce that size is to reduce the size of the database using the --depth parameter, added in 8801ded.

I'm merging this to master, but there's a good chance it will need modifications and if we cherry-pick it to stable, we'll need to be cognizant of syncing them.

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

Successfully merging this pull request may close these issues.

2 participants