74 git submodule support #485
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add support for initializing and updating git submodules during repository cloning.
Added GitCloneSubmodules option to support recursive submodule initialization.
Resolved issues with git submodule handling for better cloning and URL resolution.
Resolved issues with git submodule handling to ensure robust cloning and URL resolution without relying on git binary calls.
*test.go file addendum
Adding tests for Submodules and two missing
Author
|
I have read the CLA Document and I hereby sign the CLA |
johnstcn
reviewed
Jan 2, 2026
Member
johnstcn
left a comment
johnstcn
left a comment
There was a problem hiding this comment.
Thanks for working on this @bjornrobertsson !
I have some comments below, and I think it would also be worthwhile adding an integration test (see: integration/integration_test.go and testutil/gittest/gittest.go).
Comment on lines
+436
to
+438
| // resolveSubmoduleURL resolves a potentially relative submodule URL against the parent repository URL | ||
| // ResolveSubmoduleURLForTest is exported for testing resolveSubmoduleURL logic | ||
| func ResolveSubmoduleURLForTest(parentURL, submoduleURL string) (string, error) { |
Member
There was a problem hiding this comment.
nit: the naming of this function implies that it is only used in tests, which is false. suggestion: ResolveSubmoduleURL.
Comment on lines
+505
to
+528
| { | ||
| name: "absolute", | ||
| parentURL: "https://example.com/org/main.git", | ||
| subURL: "https://github.com/other/repo.git", | ||
| expect: "https://github.com/other/repo.git", | ||
| }, | ||
| { | ||
| name: "relativeSibling", | ||
| parentURL: "https://example.com/org/main.git", | ||
| subURL: "../deps/lib.git", | ||
| expect: "https://example.com/org/deps/lib.git", | ||
| }, | ||
| { | ||
| name: "relativeChild", | ||
| parentURL: "https://example.com/org/main.git", | ||
| subURL: "./extras/tool.git", | ||
| expect: "https://example.com/org/main.git/extras/tool.git", | ||
| }, | ||
| { | ||
| name: "badParent", | ||
| parentURL: "://bad", | ||
| subURL: "./child", | ||
| expectErr: "parse parent URL", | ||
| }, |
Member
There was a problem hiding this comment.
Please also test with the URL schemes from TestSetupRepoAuth:
- SSH with scheme (
ssh://host.tld/repo) - SSH without scheme (
[email protected]:repo/path) - Git scheme (
git://[email protected]:repo/path) - Absolute local path (
/path/to/repo) - Relative paths without
./..(path/to/repo)
Comment on lines
+145
to
+150
| if opts.Submodules { | ||
| err = initSubmodules(ctx, logf, repo, opts) | ||
| if err != nil { | ||
| return true, fmt.Errorf("init submodules: %w", err) | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
We can detect the presence/absence of submodules via .gitmodules after cloning the remote. This would remove the need to have a separate config option.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add submodule cloning support via native Go implementation
Summary
This PR resolves/mitigates #74
Changes
Since the existing submodule support in
git.godepends on execution of the git binary, this implementation circumvents that limitation by traversing the tree to clone submodules natively in Go.Behavior