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.

Sign up for GitHub

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

Fix regression in recurse_submodules #585

Merged
merged 6 commits into from
Sep 23, 2021
Merged

Fix regression in recurse_submodules #585

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aed3625
Fix submodules
robbertvanginkel Sep 23, 2021
e214692
Refactor slice to extra return arg
robbertvanginkel Sep 23, 2021
941c21e
Log git version
robbertvanginkel Sep 23, 2021
b5467b0
Add explicit destination subdir
robbertvanginkel Sep 23, 2021
1af7986
Add comment, add changelog note, remove debugging log
robbertvanginkel Sep 23, 2021
62bf937
remove unnecessary ws change
robbertvanginkel Sep 23, 2021
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
39 changes: 26 additions & 13 deletions private/pkg/git/cloner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (c *cloner) CloneToBucket(
}
gitConfigAuthArgs = append(gitConfigAuthArgs, extraArgs...)
}
fetchRef, checkoutRefs := getRefspecsForName(options.Name)
fetchRef, worktreeRef, checkoutRef := getRefspecsForName(options.Name)
fetchArgs := append(
gitConfigAuthArgs,
"--git-dir="+bareDir.AbsPath(),
Expand All @@ -150,7 +150,6 @@ func (c *cloner) CloneToBucket(
return err
}
}

buffer.Reset()
cmd = exec.CommandContext(ctx, "git", fetchArgs...)
cmd.Env = app.Environ(envContainer)
Expand All @@ -159,16 +158,31 @@ func (c *cloner) CloneToBucket(
return newGitCommandError(err, buffer, bareDir)
}

for _, checkoutRef := range checkoutRefs {
buffer.Reset()
args := append(
gitConfigAuthArgs,
"--git-dir="+bareDir.AbsPath(),
"worktree",
"add",
worktreeDir.AbsPath(),
worktreeRef,
)
cmd = exec.CommandContext(ctx, "git", args...)
cmd.Env = app.Environ(envContainer)
cmd.Stderr = buffer
if err := cmd.Run(); err != nil {
return newGitCommandError(err, buffer, worktreeDir)
}

if checkoutRef != "" {
buffer.Reset()
args := append(
gitConfigAuthArgs,
"--git-dir="+bareDir.AbsPath(),
"--work-tree="+worktreeDir.AbsPath(),
"checkout",
checkoutRef,
)
cmd = exec.CommandContext(ctx, "git", args...)
cmd.Dir = worktreeDir.AbsPath()
cmd.Env = app.Environ(envContainer)
cmd.Stderr = buffer
if err := cmd.Run(); err != nil {
Expand All @@ -179,8 +193,6 @@ func (c *cloner) CloneToBucket(
if options.RecurseSubmodules {
submoduleArgs := append(
gitConfigAuthArgs,
"--git-dir="+bareDir.AbsPath(),
"--work-tree="+worktreeDir.AbsPath(),
"submodule",
"update",
"--init",
Expand All @@ -190,6 +202,7 @@ func (c *cloner) CloneToBucket(
)
buffer.Reset()
cmd = exec.CommandContext(ctx, "git", submoduleArgs...)
cmd.Dir = worktreeDir.AbsPath()
cmd.Env = app.Environ(envContainer)
cmd.Stderr = buffer
if err := cmd.Run(); err != nil {
Expand Down Expand Up @@ -302,9 +315,9 @@ func getSSHKnownHostsFilePaths(sshKnownHostsFiles string) []string {
return filePaths
}

func getRefspecsForName(gitName Name) (string, []string) {
func getRefspecsForName(gitName Name) (string, string, string) {
robbertvanginkel marked this conversation as resolved.
Show resolved Hide resolved
if gitName == nil {
return "HEAD", []string{"FETCH_HEAD"}
return "HEAD", "FETCH_HEAD", ""
}
if gitName.cloneBranch() != "" && gitName.checkout() != "" {
// When doing branch/tag clones, make sure we use a
Expand All @@ -313,19 +326,19 @@ func getRefspecsForName(gitName Name) (string, []string) {
// for example:
// branch=origin/main,ref=origin/main~1
fetchRefSpec := gitName.cloneBranch() + ":" + gitName.cloneBranch()
return fetchRefSpec, []string{"FETCH_HEAD", gitName.checkout()}
return fetchRefSpec, "FETCH_HEAD", gitName.checkout()
} else if gitName.cloneBranch() != "" {
return gitName.cloneBranch(), []string{"FETCH_HEAD"}
return gitName.cloneBranch(), "FETCH_HEAD", ""
} else if gitName.checkout() != "" {
// After fetch we won't have checked out any refs. This
// will cause `refs=` containing "HEAD" to fail, as HEAD
// is a special case that is not fetched into a ref but
// instead refers to the current commit checked out. By
// checking out "FETCH_HEAD" before checking out the
// user supplied ref, we behave similarly to git clone.
return "HEAD", []string{"FETCH_HEAD", gitName.checkout()}
return "HEAD", "FETCH_HEAD", gitName.checkout()
} else {
return "HEAD", []string{"FETCH_HEAD"}
return "HEAD", "FETCH_HEAD", ""
}
}

Expand Down
62 changes: 48 additions & 14 deletions private/pkg/git/git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,44 @@ import (

func TestGitCloner(t *testing.T) {
t.Parallel()

// Add git version to log for easier context of ci failures.
gitVersion, err := exec.Command("git", "--version").CombinedOutput()
require.NoError(t, err)
t.Log(string(gitVersion))

originDir, workDir := createGitDirs(t)

t.Run("default", func(t *testing.T) {
t.Parallel()
readBucket := readBucketForName(t, workDir, 1, nil)
readBucket := readBucketForName(t, workDir, 1, nil, false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
assert.Equal(t, "// commit 2", string(content), "expected the commit on local-branch to be checked out")
_, err = readBucket.Stat(context.Background(), "nonexistent")
assert.True(t, storage.IsNotExist(err))
_, err = storage.ReadPath(context.Background(), readBucket, "submodule/test.proto")
assert.True(t, storage.IsNotExist(err))
})

t.Run("default_submodule", func(t *testing.T) {
t.Parallel()
readBucket := readBucketForName(t, workDir, 1, nil, true)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
assert.Equal(t, "// commit 2", string(content), "expected the commit on local-branch to be checked out")
_, err = readBucket.Stat(context.Background(), "nonexistent")
assert.True(t, storage.IsNotExist(err))
content, err = storage.ReadPath(context.Background(), readBucket, "submodule/test.proto")
require.NoError(t, err)
assert.Equal(t, "// submodule", string(content))
})

t.Run("main", func(t *testing.T) {
t.Parallel()
readBucket := readBucketForName(t, workDir, 1, NewBranchName("main"))
readBucket := readBucketForName(t, workDir, 1, NewBranchName("main"), false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
Expand All @@ -60,7 +82,7 @@ func TestGitCloner(t *testing.T) {

t.Run("origin/main", func(t *testing.T) {
t.Parallel()
readBucket := readBucketForName(t, workDir, 1, NewBranchName("origin/main"))
readBucket := readBucketForName(t, workDir, 1, NewBranchName("origin/main"), false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
Expand All @@ -71,7 +93,7 @@ func TestGitCloner(t *testing.T) {

t.Run("origin/remote-branch", func(t *testing.T) {
t.Parallel()
readBucket := readBucketForName(t, workDir, 1, NewBranchName("origin/remote-branch"))
readBucket := readBucketForName(t, workDir, 1, NewBranchName("origin/remote-branch"), false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
Expand All @@ -82,7 +104,7 @@ func TestGitCloner(t *testing.T) {

t.Run("remote-tag", func(t *testing.T) {
t.Parallel()
readBucket := readBucketForName(t, workDir, 1, NewTagName("remote-tag"))
readBucket := readBucketForName(t, workDir, 1, NewTagName("remote-tag"), false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
Expand All @@ -93,7 +115,7 @@ func TestGitCloner(t *testing.T) {

t.Run("branch_and_main_ref", func(t *testing.T) {
t.Parallel()
readBucket := readBucketForName(t, workDir, 2, NewRefNameWithBranch("HEAD~", "main"))
readBucket := readBucketForName(t, workDir, 2, NewRefNameWithBranch("HEAD~", "main"), false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
Expand All @@ -104,7 +126,7 @@ func TestGitCloner(t *testing.T) {

t.Run("branch_and_ref", func(t *testing.T) {
t.Parallel()
readBucket := readBucketForName(t, workDir, 2, NewRefNameWithBranch("local-branch~", "local-branch"))
readBucket := readBucketForName(t, workDir, 2, NewRefNameWithBranch("local-branch~", "local-branch"), false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
Expand All @@ -115,7 +137,7 @@ func TestGitCloner(t *testing.T) {

t.Run("HEAD", func(t *testing.T) {
t.Parallel()
readBucket := readBucketForName(t, workDir, 1, NewRefName("HEAD"))
readBucket := readBucketForName(t, workDir, 1, NewRefName("HEAD"), false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
Expand All @@ -128,7 +150,7 @@ func TestGitCloner(t *testing.T) {
t.Parallel()
revParseBytes, err := exec.Command("git", "-C", workDir, "rev-parse", "HEAD~").Output()
require.NoError(t, err)
readBucket := readBucketForName(t, workDir, 2, NewRefName(strings.TrimSpace(string(revParseBytes))))
readBucket := readBucketForName(t, workDir, 2, NewRefName(strings.TrimSpace(string(revParseBytes))), false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
Expand All @@ -141,7 +163,7 @@ func TestGitCloner(t *testing.T) {
t.Parallel()
revParseBytes, err := exec.Command("git", "-C", originDir, "rev-parse", "remote-branch~").Output()
require.NoError(t, err)
readBucket := readBucketForName(t, workDir, 2, NewRefNameWithBranch(strings.TrimSpace(string(revParseBytes)), "origin/remote-branch"))
readBucket := readBucketForName(t, workDir, 2, NewRefNameWithBranch(strings.TrimSpace(string(revParseBytes)), "origin/remote-branch"), false)

content, err := storage.ReadPath(context.Background(), readBucket, "test.proto")
require.NoError(t, err)
Expand All @@ -151,7 +173,7 @@ func TestGitCloner(t *testing.T) {
})
}

func readBucketForName(t *testing.T, path string, depth uint32, name Name) storage.ReadBucket {
func readBucketForName(t *testing.T, path string, depth uint32, name Name, recurseSubmodules bool) storage.ReadBucket {
storageosProvider := storageos.NewProvider(storageos.ProviderWithSymlinks())
cloner := NewCloner(zap.NewNop(), storageosProvider, ClonerOptions{})
envContainer, err := app.NewEnvContainerForOS()
Expand All @@ -165,8 +187,9 @@ func readBucketForName(t *testing.T, path string, depth uint32, name Name) stora
depth,
readBucketBuilder,
CloneToBucketOptions{
Mapper: storage.MatchPathExt(".proto"),
Name: name,
Mapper: storage.MatchPathExt(".proto"),
Name: name,
RecurseSubmodules: recurseSubmodules,
},
)
require.NoError(t, err)
Expand All @@ -177,8 +200,18 @@ func readBucketForName(t *testing.T, path string, depth uint32, name Name) stora

func createGitDirs(t *testing.T) (string, string) {
tmpDir := t.TempDir()
originPath := filepath.Join(tmpDir, "origin")

submodulePath := filepath.Join(tmpDir, "submodule")
require.NoError(t, os.MkdirAll(submodulePath, os.ModePerm))
runCommand(t, "git", "-C", submodulePath, "init")
runCommand(t, "git", "-C", submodulePath, "config", "user.email", "[email protected]")
runCommand(t, "git", "-C", submodulePath, "config", "user.name", "Buf go tests")
runCommand(t, "git", "-C", submodulePath, "checkout", "-b", "main")
require.NoError(t, os.WriteFile(filepath.Join(submodulePath, "test.proto"), []byte("// submodule"), 0600))
runCommand(t, "git", "-C", submodulePath, "add", "test.proto")
runCommand(t, "git", "-C", submodulePath, "commit", "-m", "commit 0")

originPath := filepath.Join(tmpDir, "origin")
require.NoError(t, os.MkdirAll(originPath, 0777))
runCommand(t, "git", "-C", originPath, "init")
runCommand(t, "git", "-C", originPath, "config", "user.email", "[email protected]")
Expand All @@ -187,6 +220,7 @@ func createGitDirs(t *testing.T) (string, string) {
require.NoError(t, os.WriteFile(filepath.Join(originPath, "test.proto"), []byte("// commit 0"), 0600))
runCommand(t, "git", "-C", originPath, "add", "test.proto")
runCommand(t, "git", "-C", originPath, "commit", "-m", "commit 0")
runCommand(t, "git", "-C", originPath, "submodule", "add", submodulePath)
require.NoError(t, os.WriteFile(filepath.Join(originPath, "test.proto"), []byte("// commit 1"), 0600))
runCommand(t, "git", "-C", originPath, "add", "test.proto")
runCommand(t, "git", "-C", originPath, "commit", "-m", "commit 1")
Expand Down