Update submodules and do recursive clone

[stv0g]

This will not cover all edge cases like changes in the submodules.
But we have it in production for over a year now and are happy with it.

[manics]

Have you thought about adding git submodule sync, or is that going to cause problems? (Edit: Maybe it'll confuse things... if someone changes the submodule locally we probably shouldn't change it back. Could be worth adding to https://jupyterhub.github.io/nbgitpuller/topic/automatic-merging.html#topic-automatic-merging ?)

Also any chance of adding a test? I've got a love-hate relationship with submodules, they're great when they work, incredibly frustrating when there's a change, so I think a test would be useful to ensure we don't modify a git option somewhere in future and inadvertently break it.

[jacobtomlinson]

I would support this being merged. Anything I can do to help unblock?

[yuvipanda]

@jacobtomlinson mostly I'm not sure how the merging functionality would work for files inside the submodules. Will it preserve current automerge behavior? Probably not, so we should very clearly document that. A test is also needed, along with resolving the merge conflict.

[jacobtomlinson]

Naively I assumed the same merging behaviour would still apply. What makes you think it should behave differently?

[yuvipanda]

git submodules just contain a pointer to a repo + ref to check out. So any merging behavior will only refer to those two pieces of information, and nothing more. So if the ref had changed because the user committed something inside the submodule, and the upstream has too, the merging behavior will work out to simply preserving the user's commit - no merging will happen.

To do proper merging, we'll have to recursively dive into each submodule and do our merging there too.

[jacobtomlinson]

To do proper merging, we'll have to recursively dive into each submodule and do our merging there too.

Ah right I see what you mean. Yeah that's kinda what I meant, the merging logic would be the same but it would need to be performed recursively on submodules.

Perhaps the pointer itself makes this trickier.

[yuvipanda]

Yeah, you'd have to do

nbgitpuller/nbgitpuller/pull.py

Line 189 in 22e2e27

def update(self):

recursively. The ref pointer will make things difficult, since you'll probably always diverge from the upstream ref. How to make sure you still get changes from upstream ref moving while keeping the merge behavior? Doable, just tricky