-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Add a 'pull submodules' option to Repositories #3757
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| alter table `project__repository` drop column `pull_submodules`; | ||
|
stith marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| alter table `project__repository` add `pull_submodules` boolean not null DEFAULT true; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two issues here:
alter table `project__repository` add `pull_submodules` boolean not null default true;
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. who are you |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,9 +100,17 @@ func (c CmdGitClient) Clone(r GitRepository) error { | |
| dirName = r.TmpDirName | ||
| } | ||
|
|
||
| if r.Repository.PullSubmodules { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The args := []string{"clone", "--branch", r.Repository.GitBranch, r.Repository.GetGitURL(false), dirName}
if r.Repository.PullSubmodules {
args = append([]string{"clone", "--recursive", "--branch", r.Repository.GitBranch, r.Repository.GetGitURL(false), dirName}, []string{}...)
// or build args slice and insert --recursive at position 1
}
return c.run(r, GitRepositoryTmpPath, args...)A cleaner approach: cloneArgs := []string{"clone"}
if r.Repository.PullSubmodules {
cloneArgs = append(cloneArgs, "--recursive")
}
cloneArgs = append(cloneArgs, "--branch", r.Repository.GitBranch, r.Repository.GetGitURL(false), dirName)
return c.run(r, GitRepositoryTmpPath, cloneArgs...) |
||
| return c.run(r, GitRepositoryTmpPath, | ||
| "clone", | ||
| "--recursive", | ||
| "--branch", | ||
| r.Repository.GitBranch, | ||
| r.Repository.GetGitURL(false), | ||
| dirName) | ||
| } | ||
| return c.run(r, GitRepositoryTmpPath, | ||
| "clone", | ||
| "--recursive", | ||
| "--branch", | ||
| r.Repository.GitBranch, | ||
| r.Repository.GetGitURL(false), | ||
|
|
@@ -116,7 +124,11 @@ func (c CmdGitClient) Pull(r GitRepository) error { | |
| if err != nil { | ||
| return err | ||
| } | ||
| return c.run(r, GitRepositoryFullPath, "submodule", "update", "--init", "--recursive") | ||
|
|
||
| if r.Repository.PullSubmodules { | ||
| return c.run(r, GitRepositoryFullPath, "submodule", "update", "--init", "--recursive") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func (c CmdGitClient) Checkout(r GitRepository, target string) error { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,10 +102,15 @@ func (c GoGitClient) Clone(r GitRepository) error { | |
| return authErr | ||
| } | ||
|
|
||
| submoduleDepth := git.DefaultSubmoduleRecursionDepth | ||
| if !r.Repository.PullSubmodules { | ||
| submoduleDepth = git.NoRecurseSubmodules | ||
| } | ||
|
|
||
| cloneOpt := &git.CloneOptions{ | ||
| URL: r.Repository.GetGitURL(true), | ||
| Progress: ProgressWrapper{r.Logger}, | ||
| RecurseSubmodules: git.DefaultSubmoduleRecursionDepth, | ||
| RecurseSubmodules: submoduleDepth, | ||
| ReferenceName: plumbing.NewBranchReferenceName(r.Repository.GitBranch), | ||
| Auth: authMethod, | ||
| } | ||
|
|
@@ -137,9 +142,16 @@ func (c GoGitClient) Pull(r GitRepository) error { | |
| } | ||
|
|
||
| // Pull the latest changes from the origin remote and merge into the current branch | ||
| err = wt.Pull(&git.PullOptions{RemoteName: "origin", | ||
| pullOpts := &git.PullOptions{ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor redundancy: pullOpts := &git.PullOptions{
RemoteName: "origin",
Auth: authMethod,
RecurseSubmodules: git.NoRecurseSubmodules,
}
if r.Repository.PullSubmodules {
pullOpts.RecurseSubmodules = git.DefaultSubmoduleRecursionDepth
} |
||
| RemoteName: "origin", | ||
| Auth: authMethod, | ||
| RecurseSubmodules: git.DefaultSubmoduleRecursionDepth}) | ||
| RecurseSubmodules: git.DefaultSubmoduleRecursionDepth, | ||
| } | ||
| if !r.Repository.PullSubmodules { | ||
| pullOpts.RecurseSubmodules = git.NoRecurseSubmodules | ||
| } | ||
|
|
||
| err = wt.Pull(pullOpts) | ||
| if err != nil && err != git.NoErrAlreadyUpToDate { | ||
| r.Logger.Log("Unable to pull latest changes") | ||
| return err | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The zero value of
boolin Go isfalse, so any code path that constructs aRepositorystruct without explicitly settingPullSubmodules(e.g. direct API calls, backup restore, or internal tooling) will silently disable submodule pulling. The database default of1only applies to rows inserted without that column, not to the application layer.Consider whether the API handler that reads the request body into a
Repositorystruct needs to default this field totruewhen it is absent from the JSON payload. A JSONomitemptytag would also prevent the frontend from explicitly sendingfalsewhen unchecked. At minimum, this edge case is worth a comment.