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

Support updating git submodules #59

Closed
HaukurPall opened this issue Oct 25, 2018 · 10 comments · May be fixed by #123
Closed

Support updating git submodules #59

HaukurPall opened this issue Oct 25, 2018 · 10 comments · May be fixed by #123

Comments

@HaukurPall
Copy link

I think it would be a useful feature support for git submodules in nbgitpuller.

The support I had in mind was simply calling git submodule init and then git submodule update when updating a repo.

When a repo is updated which uses git submodules, these functions will most likely need to be called. Expecting the not-tech-savvy user to call these functions will cause confusion.

@choldgraf
Copy link
Member

I agree this would be useful as well! I'm not sure how much technical complexity it would add (I'm trying to think of ways that this could error in unexpected ways). If nothing glaring comes up as a possible downside, I'd be +1. Thanks for the idea!

@betatim
Copy link
Member

betatim commented Nov 2, 2018

In repo2docker we use --recursive as a flag when cloning which seems to "just work":

https://github.com/jupyter/repo2docker/blob/cc27548621cbad15a9b3d1725a891045b510ea6f/repo2docker/contentproviders/git.py#L22-L29

This is what a github blog post recommends to use. I don't use submodules at all so not sure what the state of the art is though.

@HaukurPall
Copy link
Author

The --recursive flag would be a sufficient solution to my problem and sounds like a fairly simple implementation.

@betatim
Copy link
Member

betatim commented Nov 2, 2018

Would you like to tackle this as a PR? Would be super welcome.

I think we can make it the default, like in repo2docker, as it seems to be harmless for repos that don't have submodules.

@HaukurPall
Copy link
Author

Yes, I can take on this PR. Not sure when I can do it though.

@yuvipanda
Copy link
Contributor

Hey @HaukurPall. Have you had any time since to work on this? :)

@yuvipanda
Copy link
Contributor

Thinking about this some more, what happens if the user has edited files inside the submodule? We would basically need to apply our merging logic recursively as well, rather than just run the submodule commands.

@HaukurPall
Copy link
Author

Hi @yuvipanda,
No, I have not worked on this.
I am unsure how git-submodules work in general and especially how they behave if the source repo refers to a different commit of a submodule, with uncommitted changes. [Opinion]: That being said, I think adding the merging logic to the submodules should be done separately, and potentially just when someone asks for it.

@yuvipanda
Copy link
Contributor

Thanks for responding, @HaukurPall.

Submodules are generally pretty tricky. Every time I thought I had understood them I have been wrong...

The automatic, 'no merge conflicts' invariant is a core part of nbgitpuller. Unless there are major reasons to not, I'd say we must support that before we can merge submodule support.

@stv0g
Copy link

stv0g commented Jun 6, 2019

I am also interested in this feature.

git submodule update has three different modes: --merge, --rebase, --checkout.

I am not sure which one we should use. @yuvipanda what do you think? Which one is closest to nbgitpullers strategy?

@manics manics linked a pull request Oct 3, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants