Skip to content

Commit

Permalink
Support git master rename to main
Browse files Browse the repository at this point in the history
  • Loading branch information
phanimarupaka committed Oct 14, 2020
1 parent 0ccdcde commit a3a194f
Show file tree
Hide file tree
Showing 13 changed files with 253 additions and 28 deletions.
3 changes: 0 additions & 3 deletions internal/cmddiff/cmddiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,6 @@ func (r *Runner) preRunE(c *cobra.Command, args []string) error {
} else {
r.DiffType = diff.DiffType(r.diffType)
}
if version == "" {
version = "master"
}

r.Path = dir
r.Ref = version
Expand Down
56 changes: 55 additions & 1 deletion internal/cmdget/cmdget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/GoogleContainerTools/kpt/internal/cmdget"
"github.com/GoogleContainerTools/kpt/internal/gitutil"
"github.com/GoogleContainerTools/kpt/internal/testutil"
"github.com/GoogleContainerTools/kpt/pkg/kptfile"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -68,6 +69,56 @@ func TestCmd_execute(t *testing.T) {
})
}

// TestCmdMainBranch_execute tests that get is correctly invoked if default branch
// is main and master branch doesn't exist
func TestCmdMainBranch_execute(t *testing.T) {
// set up git repository with master and main branches
g, dir, clean := testutil.SetupDefaultRepoAndWorkspace(t)
defer clean()
dest := filepath.Join(dir, g.RepoName)
err := g.CheckoutBranch("main", false)
if !assert.NoError(t, err) {
t.FailNow()
}
err = g.DeleteBranch("master")
if !assert.NoError(t, err) {
t.FailNow()
}

r := cmdget.NewRunner("kpt")
r.Command.SetArgs([]string{"file://" + g.RepoDirectory + ".git/", "./"})
err = r.Command.Execute()

assert.NoError(t, err)
g.AssertEqual(t, filepath.Join(g.DatasetDirectory, testutil.Dataset1), dest)

commit, err := g.GetCommit()
assert.NoError(t, err)
g.AssertKptfile(t, dest, kptfile.KptFile{
ResourceMeta: yaml.ResourceMeta{
ObjectMeta: yaml.ObjectMeta{
NameMeta: yaml.NameMeta{
Name: g.RepoName,
},
},
TypeMeta: yaml.TypeMeta{
APIVersion: kptfile.TypeMeta.APIVersion,
Kind: kptfile.TypeMeta.Kind},
},
PackageMeta: kptfile.PackageMeta{},
Upstream: kptfile.Upstream{
Type: "git",
Git: kptfile.Git{
Directory: "/",
Repo: "file://" + g.RepoDirectory,
Ref: "main",
Commit: commit, // verify the commit matches the repo
},
},
})

}

func TestCmd_stdin(t *testing.T) {
d, err := ioutil.TempDir("", "kpt")
if !assert.NoError(t, err) {
Expand Down Expand Up @@ -111,7 +162,7 @@ func TestCmd_fail(t *testing.T) {
if !assert.Error(t, err) {
return
}
assert.Contains(t, err.Error(), "failed to clone git repo: trouble fetching")
assert.Contains(t, err.Error(), "failed to lookup master(or main) branch")
}

// NoOpRunE is a noop function to replace the run function of a command. Useful for testing argument parsing.
Expand All @@ -131,6 +182,9 @@ func (t NoOpFailRunE) runE(cmd *cobra.Command, args []string) error {
// TestCmd_Execute_flagAndArgParsing verifies that the flags and args are parsed into the correct Command fields
func TestCmd_Execute_flagAndArgParsing(t *testing.T) {
failRun := NoOpFailRunE{t: t}.runE
gitutil.DefaultRef = func(repo string) (string, error) {
return "master", nil
}

r := cmdget.NewRunner("kpt")
r.Command.SilenceErrors = true
Expand Down
58 changes: 58 additions & 0 deletions internal/cmdinit/cmdinit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"testing"

"github.com/GoogleContainerTools/kpt/internal/cmdinit"
"github.com/GoogleContainerTools/kpt/internal/gitutil"
"github.com/GoogleContainerTools/kpt/internal/testutil"
"github.com/GoogleContainerTools/kpt/internal/util/man"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -136,3 +138,59 @@ func TestCmd_failNotExists(t *testing.T) {
assert.Contains(t, err.Error(), "does not exist")
}
}

func TestGitUtil_DefaultRef(t *testing.T) {
// set up git repo with both main and master branches
g, _, clean := testutil.SetupDefaultRepoAndWorkspace(t)
defer clean()

// check if master is picked as default if both main and master branches exist
defaultRef, err := gitutil.DefaultRef("file://" + g.RepoDirectory)
if !assert.NoError(t, err) {
t.FailNow()
}
if !assert.Equal(t, "master", defaultRef) {
t.FailNow()
}
if !assert.Equal(t, "master", defaultRef) {
t.FailNow()
}

err = g.CheckoutBranch("main", false)
if !assert.NoError(t, err) {
t.FailNow()
}

// delete master branch and check if main is selected as default
err = g.DeleteBranch("master")
if !assert.NoError(t, err) {
t.FailNow()
}

defaultRef, err = gitutil.DefaultRef("file://" + g.RepoDirectory)
if !assert.NoError(t, err) {
t.FailNow()
}
if !assert.Equal(t, "main", defaultRef) {
t.FailNow()
}

err = g.CheckoutBranch("master", true)
if !assert.NoError(t, err) {
t.FailNow()
}

// delete main branch and check if master is selected as default
err = g.DeleteBranch("main")
if !assert.NoError(t, err) {
t.FailNow()
}

defaultRef, err = gitutil.DefaultRef("file://" + g.RepoDirectory)
if !assert.NoError(t, err) {
t.FailNow()
}
if !assert.Equal(t, "master", defaultRef) {
t.FailNow()
}
}
66 changes: 59 additions & 7 deletions internal/gitutil/gitutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,57 @@ import (
// for remote repos. Defaults to UserHomeDir/.kpt/repos if unspecified.
const RepoCacheDirEnv = "KPT_CACHE_DIR"

// DefaultRef returns the DefaultRef to "master" if master branch exists in
// remote repository, falls back to "main" if master branch doesn't exist
// Making it a var so that it can be overridden for local testing
var DefaultRef = func(repo string) (string, error) {
masterRef := "master"
mainRef := "main"
masterExists, err := branchExists(repo, masterRef)
if err != nil {
return "", err
}
mainExists, err := branchExists(repo, mainRef)
if err != nil {
return "", err
}
if masterExists {
return masterRef, nil
} else if mainExists {
return mainRef, nil
}
return masterRef, nil
}

// BranchExists checks if branch is present in the input repo
func branchExists(repo, branch string) (bool, error) {
gitProgram, err := exec.LookPath("git")
if err != nil {
return false, errors.Wrap(err)
}
stdOut := bytes.Buffer{}
stdErr := bytes.Buffer{}
cmd := exec.Command(gitProgram, "ls-remote", repo, branch)
cmd.Stderr = &stdErr
cmd.Stdout = &stdOut
err = cmd.Run()
if err != nil {
// stdErr contains the error message for os related errors, git permission errors
// and if repo doesn't exist
return false, errors.Errorf("failed to lookup master(or main) branch %v: %s", err, strings.TrimSpace(stdErr.String()))
}
// stdOut contains the branch information if the branch is present in remote repo
// stdOut is empty if the repo doesn't have the input branch
if strings.TrimSpace(stdOut.String()) != "" {
return true, nil
}
return false, nil
}

// NewUpstreamGitRunner returns a new GitRunner for an upstream package.
//
// The upstream package repo will be fetched to a local cache directory under $HOME/.kpt
// and hard reset to origin/master.
// and hard reset to origin/main.
// The refs will also be fetched so they are available locally.
func NewUpstreamGitRunner(uri, dir string, required []string, optional []string) (*GitRunner, error) {
g := &GitRunner{}
Expand Down Expand Up @@ -195,16 +242,21 @@ func (g *GitRunner) cacheRepo(uri, dir string,
"please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", err)
}

defaultRef, err := DefaultRef(uri)
if err != nil {
return "", errors.Errorf("%v, please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", err)
}

// reset the repo state
if err = gitRunner.Run("checkout", "master"); err != nil {
return "", errors.Errorf("failed to clone repo: trouble checking out master: %v, "+
"please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", err)
if err = gitRunner.Run("checkout", defaultRef); err != nil {
return "", errors.Errorf("failed to clone repo: trouble checking out %s: %v, "+
"please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", defaultRef, err)
}

// TODO: make this safe for concurrent operations
if err = gitRunner.Run("reset", "--hard", "origin/master"); err != nil {
return "", errors.Errorf("failed to clone repo: trouble reset to master: %v, "+
"please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", err)
if err = gitRunner.Run("reset", "--hard", "origin/"+defaultRef); err != nil {
return "", errors.Errorf("failed to clone repo: trouble reset to %s: %v, "+
"please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", defaultRef, err)
}
gitRunner.Dir = filepath.Join(repoCacheDir, dir)
return repoCacheDir, nil
Expand Down
28 changes: 24 additions & 4 deletions internal/testutil/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,21 @@ func (g *TestGitRepo) CheckoutBranch(branch string, create bool) error {
// checkout the branch
cmd := exec.Command("git", args...)
cmd.Dir = g.RepoDirectory
stdoutStderr, err := cmd.CombinedOutput()
_, err := cmd.CombinedOutput()
if err != nil {
return err
}

return nil
}

// DeleteBranch deletes the git branch in the repo
func (g *TestGitRepo) DeleteBranch(branch string) error {
// checkout the branch
cmd := exec.Command("git", []string{"branch", "-D", branch}...)
cmd.Dir = g.RepoDirectory
_, err := cmd.Output()
if err != nil {
fmt.Fprintf(os.Stderr, "%s", stdoutStderr)
return err
}

Expand Down Expand Up @@ -312,9 +324,8 @@ func (g *TestGitRepo) CopyAddData(data string) error {

cmd := exec.Command("git", "add", ".")
cmd.Dir = g.RepoDirectory
stdoutStderr, err := cmd.CombinedOutput()
_, err = cmd.CombinedOutput()
if err != nil {
fmt.Fprintf(os.Stderr, "%s", stdoutStderr)
return err
}

Expand Down Expand Up @@ -506,6 +517,15 @@ func SetupDefaultRepoAndWorkspace(t *testing.T) (*TestGitRepo, string, func()) {
assert.FailNowf(t, "%s %s", gr.Stdout.String(), gr.Stderr.String())
}

// make sure that both master and main branches are created in the test repo
// do not error if they already exist or
_ = g.CheckoutBranch("master", true)
_ = g.CheckoutBranch("main", true)

// checkout to master branch
err = g.CheckoutBranch("master", false)
assert.NoError(t, err)

return g, dir, func() {
// ignore cleanup failures
_ = g.RemoveAll()
Expand Down
8 changes: 8 additions & 0 deletions internal/util/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"path/filepath"
"strings"

"github.com/GoogleContainerTools/kpt/internal/gitutil"
"github.com/GoogleContainerTools/kpt/internal/util/get"
"github.com/GoogleContainerTools/kpt/pkg/kptfile"
"github.com/GoogleContainerTools/kpt/pkg/kptfile/kptfileutil"
Expand Down Expand Up @@ -132,6 +133,13 @@ func (c *Command) Run() error {

var upstreamTargetPkg string

if c.Ref == "" {
c.Ref, err = gitutil.DefaultRef(kptFile.Upstream.Git.Repo)
if err != nil {
return err
}
}

if c.DiffType == DiffTypeRemote ||
c.DiffType == DiffTypeCombined ||
c.DiffType == DiffType3Way {
Expand Down
17 changes: 13 additions & 4 deletions internal/util/get/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"path/filepath"
"strings"

"github.com/GoogleContainerTools/kpt/internal/gitutil"
"github.com/GoogleContainerTools/kpt/internal/util/git"
"github.com/GoogleContainerTools/kpt/pkg/kptfile"
"github.com/GoogleContainerTools/kpt/pkg/kptfile/kptfileutil"
Expand Down Expand Up @@ -67,9 +68,14 @@ func (c Command) Run() error {
// define where we are going to clone the package from
r := &git.RepoSpec{OrgRepo: c.Repo, Path: c.Directory, Ref: c.Ref}

defaultRef, err := gitutil.DefaultRef(c.Repo)
if err != nil {
return err
}

// clone the repo to a tmp directory.
// delete the tmp directory later.
err := ClonerUsingGitExec(r)
err = ClonerUsingGitExec(r, defaultRef)
if err != nil {
return errors.Errorf("failed to clone git repo: %v", err)
}
Expand Down Expand Up @@ -103,7 +109,7 @@ type Cloner func(repoSpec *git.RepoSpec) error
// ClonerUsingGitExec uses a local git install, as opposed
// to say, some remote API, to obtain a local clone of
// a remote repo.
func ClonerUsingGitExec(repoSpec *git.RepoSpec) error {
func ClonerUsingGitExec(repoSpec *git.RepoSpec, defaultRef string) error {
// look for a tag with the directory as a prefix for versioning
// subdirectories independently
originalRef := repoSpec.Ref
Expand All @@ -123,7 +129,7 @@ func ClonerUsingGitExec(repoSpec *git.RepoSpec) error {
if err != nil {
if strings.HasPrefix(repoSpec.Path, "blob/") {
return errors.Errorf("failed to clone git repo containing /blob/, "+
"you may need to remove /blob/master from the url:\n%v", err)
"you may need to remove /blob/%s from the url:\n%v", defaultRef, err)
}
return errors.Errorf("failed to clone git repo: %v", err)
}
Expand Down Expand Up @@ -165,7 +171,10 @@ func clonerUsingGitExec(repoSpec *git.RepoSpec) error {
repoSpec.CloneSpec())
}
if repoSpec.Ref == "" {
repoSpec.Ref = "master"
repoSpec.Ref, err = gitutil.DefaultRef(repoSpec.Dir)
if err != nil {
return err
}
}

err = func() error {
Expand Down
2 changes: 1 addition & 1 deletion internal/util/get/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ func TestCommand_Run_failInvalidRepo(t *testing.T) {
if !assert.Error(t, err) {
t.FailNow()
}
if !assert.Contains(t, err.Error(), "trouble fetching") {
if !assert.Contains(t, err.Error(), "failed to lookup master(or main) branch") {
t.FailNow()
}
}
Expand Down

0 comments on commit a3a194f

Please sign in to comment.