Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
81b6fd7
Implement submodule initialization in CloneRepo
bjornrobertsson Dec 9, 2025
5de553d
Add GitCloneSubmodules option for cloning
bjornrobertsson Dec 9, 2025
adbdccf
Fix typo in environment variables documentation entry
bjornrobertsson Dec 9, 2025
18384d5
Fix submodule handling in git
bjornrobertsson Dec 9, 2025
4d3e529
Fix submodule handling in README
bjornrobertsson Dec 9, 2025
e183aff
Update README.md
bjornrobertsson Dec 9, 2025
2d1df11
Add ResolveSubmoduleURLForTest function
bjornrobertsson Dec 9, 2025
5407a5b
Add tests for ResolveSubmoduleURL and CloneOptions
bjornrobertsson Dec 9, 2025
463fd91
Add tests for new environment variables in CLI
bjornrobertsson Dec 9, 2025
04f798b
make gen
bjornrobertsson Dec 11, 2025
db5e40d
Rename ResolveSubmoduleURLForTest to ResolveSubmoduleURL
bjornrobertsson Jan 22, 2026
223d3e4
Add integration test for git submodules
bjornrobertsson Jan 22, 2026
5c59408
Change GitCloneSubmodules to accept depth (true/false/integer)
bjornrobertsson Jan 23, 2026
8134964
Add tests for submodule depth values
bjornrobertsson Jan 23, 2026
442477e
Remove submodule section from README (belongs in release notes)
bjornrobertsson Jan 23, 2026
fb31cac
Fix typo: dev.zaure.com -> dev.azure.com
bjornrobertsson Jan 23, 2026
68ace4b
Fix formatting (gofmt alignment)
bjornrobertsson Jan 23, 2026
149ae9a
Fix git_test.go: use SubmoduleDepth int instead of Submodules bool
bjornrobertsson Jan 23, 2026
3f81838
Update golden file for CLI output test
bjornrobertsson Jan 23, 2026
ea25253
Merge branch 'main' into 74-git-submodule-support
bjornrobertsson Feb 3, 2026
cb11759
Fix: assign repo from CloneContext (was discarded after merge)
bjornrobertsson Feb 3, 2026
76138b0
Merge branch 'main' into 74-git-submodule-support
johnstcn Feb 6, 2026
79d1fc5
Merge branch 'main' into 74-git-submodule-support
johnstcn Feb 6, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,10 @@ On macOS or Windows systems, we recommend using a VM or the provided `.devcontai
- `test`: Runs tests.
- `test-registry`: Stands up a local registry for caching images used in tests.
- `docs/env-variables.md`: Updated the [environment variables documentation](./docs/env-variables.md).

**Submodule Handling Fix**

An issue concerning git's submodule handling has been resolved through iterative refinements. This fix ensures robust submodule cloning and URL resolution without relying on the calls to the git binary (current fallback).
```
ENVBUILDER_GIT_CLONE_SUBMODULES=true
```
1 change: 1 addition & 0 deletions docs/env-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
| `--git-clone-depth` | `ENVBUILDER_GIT_CLONE_DEPTH` | | The depth to use when cloning the Git repository. |
| `--git-clone-single-branch` | `ENVBUILDER_GIT_CLONE_SINGLE_BRANCH` | | Clone only a single branch of the Git repository. |
| `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | `true` | Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated,it will not be turned on for the domain dev.zaure.com. |
| `--git-clone-submodules` | `ENVBUILDER_GIT_CLONE_SUBMODULES` | | Recursively clone Git submodules after cloning the repository. |
Copy link
Member

Choose a reason for hiding this comment

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

We may also want to add --git-clone-submodules-depth to limit recursion depth.

Copy link
Member

Choose a reason for hiding this comment

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

WDYT about --git-clone-submodules-depth=0 as "don't clone submodules (default)"?

Copy link
Member

Choose a reason for hiding this comment

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

Works for me 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just add conditional string eval, default, and 0 is 'false' and a number is Depth?
Transitional, it might be nicer if 'true' opens up a sub-menu for Depth in the template, poc:

  validation {
    regex = "^(true|false|[0-9]|10)$"
    error = "Must be 'true', 'false', or a number from 0-10."
  }
# and #
    "ENVBUILDER_GIT_CLONE_SUBMODULES" : tostring(local.submodule_depth),

| `--git-username` | `ENVBUILDER_GIT_USERNAME` | | The username to use for Git authentication. This is optional. |
| `--git-password` | `ENVBUILDER_GIT_PASSWORD` | | The password to use for Git authentication. This is optional. |
| `--git-ssh-private-key-path` | `ENVBUILDER_GIT_SSH_PRIVATE_KEY_PATH` | | Path to an SSH private key to be used for Git authentication. If this is set, then GIT_SSH_PRIVATE_KEY_BASE64 cannot be set. |
Expand Down
284 changes: 283 additions & 1 deletion git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@ import (
"fmt"
"io"
"net"
"net/url"
"os"
"path"
"strings"

"github.com/coder/envbuilder/options"

giturls "github.com/chainguard-dev/git-urls"
"github.com/go-git/go-billy/v5"
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/config"
"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/cache"
"github.com/go-git/go-git/v5/plumbing/protocol/packp/capability"
Expand All @@ -41,6 +44,7 @@ type CloneRepoOptions struct {
Depth int
CABundle []byte
ProxyOptions transport.ProxyOptions
Submodules bool
}

// CloneRepo will clone the repository at the given URL into the given path.
Expand Down Expand Up @@ -119,7 +123,7 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt
return false, nil
}

_, err = git.CloneContext(ctx, gitStorage, fs, &git.CloneOptions{
repo, err = git.CloneContext(ctx, gitStorage, fs, &git.CloneOptions{
URL: parsed.String(),
Auth: opts.RepoAuth,
Progress: opts.Progress,
Expand All @@ -136,6 +140,15 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt
if err != nil {
return false, fmt.Errorf("clone %q: %w", opts.RepoURL, err)
}

// Initialize submodules if requested
if opts.Submodules {
err = initSubmodules(ctx, logf, repo, opts)
if err != nil {
return true, fmt.Errorf("init submodules: %w", err)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This seems like the preferable option to me vs bespoke implementation. 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That function breaks because of the relative path, and reason for handling this manually (it was confounding and took some time).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, and v6 renames it, fixes the typo but we're not at v6 yet.

Copy link
Member

Choose a reason for hiding this comment

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

I take it you're referencing this commit? We could fork and backport this fix to v5 until v6 comes out. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is nothing concrete that the relative path issues are resolved, more they're not. Checks indicate that there are known bugs with:

  • HTTPS URLs losing a slash
  • Multiple relative paths (../../submodule.git) not working


return true, nil
}

Expand Down Expand Up @@ -361,6 +374,7 @@ func CloneOptionsFromOptions(logf func(string, ...any), options options.Options)
ThinPack: options.GitCloneThinPack,
Depth: int(options.GitCloneDepth),
CABundle: caBundle,
Submodules: options.GitCloneSubmodules,
}

cloneOpts.RepoAuth = SetupRepoAuth(logf, &options)
Expand Down Expand Up @@ -418,3 +432,271 @@ func ProgressWriter(write func(line string, args ...any)) io.WriteCloser {
done: done,
}
}

// 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) {
// If the submodule URL is absolute (contains ://) or doesn't start with ./ or ../, return it as-is
if strings.Contains(submoduleURL, "://") || (!strings.HasPrefix(submoduleURL, "../") && !strings.HasPrefix(submoduleURL, "./")) {
return submoduleURL, nil
}

// Parse the parent URL
parentParsed, err := url.Parse(parentURL)
if err != nil {
return "", fmt.Errorf("parse parent URL: %w", err)
}

// For relative URLs, we need to resolve them against the parent's path
// The parent path represents a repository (like a file in filesystem terms)
// So ../something means "sibling repository"
parentPath := strings.TrimSuffix(parentParsed.Path, "/")

// Split the submodule URL into components
// and manually walk up the directory tree for each ../
currentPath := parentPath
relativeParts := strings.Split(submoduleURL, "/")

for _, part := range relativeParts {
if part == ".." {
// Go up one directory
currentPath = path.Dir(currentPath)
} else if part == "." {
// Stay in current directory
continue
} else if part != "" {
// Add this component to the path
currentPath = currentPath + "/" + part
}
}

// Clean the final path
resolvedPath := path.Clean(currentPath)

// Construct the absolute URL
resolvedParsed := &url.URL{
Scheme: parentParsed.Scheme,
User: parentParsed.User,
Host: parentParsed.Host,
Path: resolvedPath,
}

return resolvedParsed.String(), nil
}

// initSubmodules recursively initializes and updates all submodules in the repository.
func initSubmodules(ctx context.Context, logf func(string, ...any), repo *git.Repository, opts CloneRepoOptions) error {
Copy link
Member

Choose a reason for hiding this comment

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

I'm always a bit wary about recursive functions. I'd feel better about this if we had a maximum depth of which to recurse. I'd wager that most repos won't need more than 2 iterations. If ENVBUILDER_GIT_CLONE_SUBMODULES were changed to accept a positive integer instead, this could function as the number of times to recurse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the acceptance to 0-10, true, false - and added a Depth testing

logf("🔗 Initializing git submodules...")

w, err := repo.Worktree()
if err != nil {
return fmt.Errorf("get worktree: %w", err)
}

subs, err := w.Submodules()
if err != nil {
return fmt.Errorf("get submodules: %w", err)
}

if len(subs) == 0 {
logf("No submodules found")
return nil
}

logf("Found %d submodule(s)", len(subs))

// Get the parent repository URL for resolving relative submodule URLs
cfg, err := repo.Config()
if err != nil {
return fmt.Errorf("get repo config: %w", err)
}

parentURL := opts.RepoURL
if origin, hasOrigin := cfg.Remotes["origin"]; hasOrigin && len(origin.URLs) > 0 {
parentURL = origin.URLs[0]
}
logf("Parent repository URL: %s", parentURL)

for _, sub := range subs {
subConfig := sub.Config()
logf("📦 Initializing submodule: %s", subConfig.Name)
logf(" Submodule path: %s", subConfig.Path)
logf(" Submodule URL (from .gitmodules): %s", subConfig.URL)

// Get the expected commit hash
subStatus, err := sub.Status()
if err != nil {
return fmt.Errorf("get submodule status for %q: %w", subConfig.Name, err)
}
logf(" Expected commit: %s", subStatus.Expected)

// Resolve the submodule URL
resolvedURL, err := ResolveSubmoduleURLForTest(parentURL, subConfig.URL)
if err != nil {
return fmt.Errorf("resolve submodule URL for %q: %w", subConfig.Name, err)
}
logf(" Resolved URL: %s", resolvedURL)

// Clone the submodule manually
err = cloneSubmodule(ctx, logf, w, subConfig, subStatus.Expected, resolvedURL, opts)
if err != nil {
return fmt.Errorf("clone submodule %q: %w", subConfig.Name, err)
}

logf("✓ Submodule initialized: %s", subConfig.Name)

// Recursively handle nested submodules
subRepo, err := sub.Repository()
if err != nil {
logf(" ⚠ Could not open submodule repository %s: %v", subConfig.Name, err)
continue
}

// Check for nested submodules
subWorktree, err := subRepo.Worktree()
if err == nil {
nestedSubs, err := subWorktree.Submodules()
if err == nil && len(nestedSubs) > 0 {
logf(" Found %d nested submodule(s) in %s", len(nestedSubs), subConfig.Name)
// Create new opts with the submodule's URL as the parent
nestedOpts := opts
nestedOpts.RepoURL = resolvedURL
err = initSubmodules(ctx, logf, subRepo, nestedOpts)
if err != nil {
return fmt.Errorf("init nested submodules in %q: %w", subConfig.Name, err)
}
}
}
}

logf("✓ All submodules initialized successfully")
return nil
}

// cloneSubmodule manually clones a submodule repository
func cloneSubmodule(ctx context.Context, logf func(string, ...any), parentWorktree *git.Worktree, subConfig *config.Submodule, expectedHash plumbing.Hash, resolvedURL string, opts CloneRepoOptions) error {
// Get the submodule directory within the parent worktree
submodulePath := subConfig.Path

// Create the submodule directory
subFS, err := parentWorktree.Filesystem.Chroot(submodulePath)
if err != nil {
return fmt.Errorf("chroot to submodule path: %w", err)
}

// Check if already cloned
_, err = subFS.Stat(".git")
if err == nil {
logf(" Submodule already cloned, checking out expected commit...")
// Open the existing repository
subRepo, err := git.Open(
filesystem.NewStorage(subFS, cache.NewObjectLRU(cache.DefaultMaxSize)),
subFS,
)
if err != nil {
return fmt.Errorf("open existing submodule: %w", err)
}

subWorktree, err := subRepo.Worktree()
if err != nil {
return fmt.Errorf("get submodule worktree: %w", err)
}

// Checkout the expected commit
err = subWorktree.Checkout(&git.CheckoutOptions{
Hash: expectedHash,
})
if err != nil {
return fmt.Errorf("checkout expected commit: %w", err)
}
return nil
}

// Clone the submodule
logf(" Cloning submodule from: %s", resolvedURL)

// Create .git directory for the submodule
err = subFS.MkdirAll(".git", 0o755)
if err != nil {
return fmt.Errorf("create .git directory: %w", err)
}

subGitDir, err := subFS.Chroot(".git")
if err != nil {
return fmt.Errorf("chroot to .git: %w", err)
}

gitStorage := filesystem.NewStorage(subGitDir, cache.NewObjectLRU(cache.DefaultMaxSize*10))

// Clone the submodule repository
// Use SingleBranch=false to fetch all branches so we can find the commit
subRepo, err := git.CloneContext(ctx, gitStorage, subFS, &git.CloneOptions{
URL: resolvedURL,
Auth: opts.RepoAuth,
Progress: opts.Progress,
InsecureSkipTLS: opts.Insecure,
CABundle: opts.CABundle,
ProxyOptions: opts.ProxyOptions,
SingleBranch: false, // Fetch all branches
NoCheckout: true, // Don't checkout yet, we'll do it manually
})
if err != nil && !errors.Is(err, git.ErrRepositoryAlreadyExists) {
return fmt.Errorf("clone submodule repository: %w", err)
}

// Verify the commit exists
logf(" Verifying commit exists: %s", expectedHash)
_, err = subRepo.CommitObject(expectedHash)
if err != nil {
// Commit not found, try fetching with the specific hash
logf(" Commit not found, attempting to fetch it directly...")
err = subRepo.FetchContext(ctx, &git.FetchOptions{
RemoteName: "origin",
RefSpecs: []config.RefSpec{
config.RefSpec("+" + expectedHash.String() + ":" + expectedHash.String()),
},
Auth: opts.RepoAuth,
Progress: opts.Progress,
InsecureSkipTLS: opts.Insecure,
CABundle: opts.CABundle,
ProxyOptions: opts.ProxyOptions,
})
if err != nil && err != git.NoErrAlreadyUpToDate {
// If that fails, try fetching all refs
logf(" Direct fetch failed, fetching all refs...")
err = subRepo.FetchContext(ctx, &git.FetchOptions{
RemoteName: "origin",
Auth: opts.RepoAuth,
Progress: opts.Progress,
InsecureSkipTLS: opts.Insecure,
CABundle: opts.CABundle,
ProxyOptions: opts.ProxyOptions,
})
if err != nil && err != git.NoErrAlreadyUpToDate {
return fmt.Errorf("fetch commit %s: %w", expectedHash, err)
}
}

// Verify again
_, err = subRepo.CommitObject(expectedHash)
if err != nil {
return fmt.Errorf("commit %s still not found after fetch: %w", expectedHash, err)
}
}

// Checkout the specific commit expected by the parent repository
logf(" Checking out commit: %s", expectedHash)
subWorktree, err := subRepo.Worktree()
if err != nil {
return fmt.Errorf("get submodule worktree: %w", err)
}

err = subWorktree.Checkout(&git.CheckoutOptions{
Hash: expectedHash,
})
if err != nil {
return fmt.Errorf("checkout expected commit %s: %w", expectedHash, err)
}

return nil
}
Loading
Loading