From fa13b3438e199ad91560198f7fb019fd3abbc094 Mon Sep 17 00:00:00 2001 From: Carlos Panato Date: Fri, 19 Feb 2021 10:53:52 +0100 Subject: [PATCH] git: add new function to do the plain rev parse --- cmd/krel/cmd/release_notes.go | 2 +- pkg/anago/stage.go | 7 +++- pkg/anago/stage_test.go | 24 ++++++------- pkg/changelog/changelog.go | 2 +- pkg/changelog/impl.go | 5 +++ pkg/git/git.go | 45 +++++++++++++++++------ pkg/git/git_integration_test.go | 64 +++++++++++++++++++++++++++++++++ pkg/notes/options/options.go | 4 +-- pkg/release/repository.go | 18 +++++++--- pkg/release/repository_test.go | 3 +- 10 files changed, 142 insertions(+), 32 deletions(-) diff --git a/cmd/krel/cmd/release_notes.go b/cmd/krel/cmd/release_notes.go index 46a375c68e25..563728cf8e45 100644 --- a/cmd/krel/cmd/release_notes.go +++ b/cmd/krel/cmd/release_notes.go @@ -866,7 +866,7 @@ func releaseNotesJSON(repoPath, tag string) (jsonString string, err error) { } // Chech if release branch already exists - _, err = repo.RevParse(releaseBranch) + _, err = repo.RevParseTag(releaseBranch) if err == nil { logrus.Infof("Working on branch %s instead of %s", releaseBranch, git.DefaultBranch) branchName = releaseBranch diff --git a/pkg/anago/stage.go b/pkg/anago/stage.go index ddf712be9b5b..4779e47bb9b9 100644 --- a/pkg/anago/stage.go +++ b/pkg/anago/stage.go @@ -131,6 +131,7 @@ type stageImpl interface { ) (*release.Versions, error) OpenRepo(repoPath string) (*git.Repo, error) RevParse(repo *git.Repo, rev string) (string, error) + RevParseTag(repo *git.Repo, rev string) (string, error) Checkout(repo *git.Repo, rev string, args ...string) error CurrentBranch(repo *git.Repo) (string, error) CommitEmpty(repo *git.Repo, msg string) error @@ -191,6 +192,10 @@ func (d *defaultStageImpl) RevParse(repo *git.Repo, rev string) (string, error) return repo.RevParse(rev) } +func (d *defaultStageImpl) RevParseTag(repo *git.Repo, rev string) (string, error) { + return repo.RevParseTag(rev) +} + func (d *defaultStageImpl) Checkout(repo *git.Repo, rev string, args ...string) error { return repo.Checkout(rev, args...) } @@ -328,7 +333,7 @@ func (d *DefaultStage) TagRepository() error { logrus.Infof("Preparing version %s", version) // Ensure that the tag not already exists - if _, err := d.impl.RevParse(repo, version); err == nil { + if _, err := d.impl.RevParseTag(repo, version); err == nil { return errors.Errorf("tag %s already exists", version) } diff --git a/pkg/anago/stage_test.go b/pkg/anago/stage_test.go index ed0b4146dfdf..8b88afb21487 100644 --- a/pkg/anago/stage_test.go +++ b/pkg/anago/stage_test.go @@ -238,7 +238,7 @@ func TestTagRepository(t *testing.T) { }{ { // success new rc creating release branch prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) mock.CurrentBranchReturnsOnCall(0, "release-1.20", nil) }, versions: newRCVersions, @@ -248,7 +248,7 @@ func TestTagRepository(t *testing.T) { }, { // failure on CommitEmpty new rc creating release branch prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) mock.CurrentBranchReturnsOnCall(0, "release-1.20", nil) mock.CommitEmptyReturns(err) }, @@ -259,7 +259,7 @@ func TestTagRepository(t *testing.T) { }, { // failure on CurrentBranch new rc creating release branch prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) mock.CurrentBranchReturnsOnCall(0, "release-1.20", nil) mock.CurrentBranchReturns("", err) }, @@ -270,7 +270,7 @@ func TestTagRepository(t *testing.T) { }, { // failure on Checkout new rc creating release branch prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) mock.CurrentBranchReturnsOnCall(0, "release-1.20", nil) mock.CheckoutReturns(err) }, @@ -281,7 +281,7 @@ func TestTagRepository(t *testing.T) { }, { // failure on RevParse new rc creating release branch prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", nil) + mock.RevParseTagReturns("", nil) }, versions: newRCVersions, releaseBranch: "release-1.20", @@ -299,7 +299,7 @@ func TestTagRepository(t *testing.T) { }, { // success new rc checking out release branch prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) }, versions: newRCVersions, releaseBranch: "release-1.20", @@ -308,7 +308,7 @@ func TestTagRepository(t *testing.T) { }, { // failure on Checkout new rc checking out release branch prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) mock.CheckoutReturns(err) }, versions: newRCVersions, @@ -318,7 +318,7 @@ func TestTagRepository(t *testing.T) { }, { // failure on RevParse new rc checking out release branch prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", nil) + mock.RevParseTagReturns("", nil) }, versions: newRCVersions, releaseBranch: "release-1.20", @@ -327,7 +327,7 @@ func TestTagRepository(t *testing.T) { }, { // success new beta prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) }, versions: newBetaVersions, releaseBranch: git.DefaultBranch, @@ -335,7 +335,7 @@ func TestTagRepository(t *testing.T) { }, { // new beta failure on Tag prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) mock.TagReturns(err) }, versions: newBetaVersions, @@ -344,7 +344,7 @@ func TestTagRepository(t *testing.T) { }, { // new beta failure on CurrentBranch prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) mock.CurrentBranchReturns("", err) }, versions: newBetaVersions, @@ -353,7 +353,7 @@ func TestTagRepository(t *testing.T) { }, { // new beta failure on Checkout prepare: func(mock *anagofakes.FakeStageImpl) { - mock.RevParseReturns("", err) + mock.RevParseTagReturns("", err) mock.CheckoutReturns(err) }, versions: newBetaVersions, diff --git a/pkg/changelog/changelog.go b/pkg/changelog/changelog.go index 017159d0b705..eaade01c9d0d 100644 --- a/pkg/changelog/changelog.go +++ b/pkg/changelog/changelog.go @@ -93,7 +93,7 @@ func (c *Changelog) Run() error { } remoteBranch := git.Remotify(branch) - head, err := c.impl.RevParse(repo, remoteBranch) + head, err := c.impl.RevParseTag(repo, remoteBranch) if err != nil { return errors.Wrap(err, "get latest branch commit") } diff --git a/pkg/changelog/impl.go b/pkg/changelog/impl.go index 469b91f4da1b..8db205436459 100644 --- a/pkg/changelog/impl.go +++ b/pkg/changelog/impl.go @@ -47,6 +47,7 @@ type impl interface { OpenRepo(repoPath string) (*git.Repo, error) CurrentBranch(repo *git.Repo) (branch string, err error) RevParse(repo *git.Repo, rev string) (string, error) + RevParseTag(repo *git.Repo, rev string) (string, error) CreateDownloadsTable( w io.Writer, bucket, tars, prevTag, newTag string, ) error @@ -108,6 +109,10 @@ func (*defaultImpl) RevParse(repo *git.Repo, rev string) (string, error) { return repo.RevParse(rev) } +func (*defaultImpl) RevParseTag(repo *git.Repo, rev string) (string, error) { + return repo.RevParseTag(rev) +} + func (*defaultImpl) CreateDownloadsTable( w io.Writer, bucket, tars, prevTag, newTag string, ) error { diff --git a/pkg/git/git.go b/pkg/git/git.go index d65721e5f610..4b401de52139 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -438,9 +438,10 @@ func (r *Repo) Cleanup() error { return os.RemoveAll(r.dir) } -// RevParse parses a git revision and returns a SHA1 on success, otherwise an +// RevParseTag parses a git revision and returns a SHA1 on success, otherwise an // error. -func (r *Repo) RevParse(rev string) (string, error) { +// If the revision does not match a tag add the remote origin in the revision. +func (r *Repo) RevParseTag(rev string) (string, error) { matched, err := regexp.MatchString(`v\d+\.\d+\.\d+.*`, rev) if err != nil { return "", err @@ -459,7 +460,31 @@ func (r *Repo) RevParse(rev string) (string, error) { return ref.String(), nil } -// RevParseShort parses a git revision and returns a SHA1 trimmed to the length +// RevParseTag parses a git revision and returns a SHA1 on success, otherwise an +// error. +func (r *Repo) RevParse(rev string) (string, error) { + // Try to resolve the rev + ref, err := r.inner.ResolveRevision(plumbing.Revision(rev)) + if err != nil { + return "", err + } + + return ref.String(), nil +} + +// RevParseTagShort parses a git revision and returns a SHA1 trimmed to the length +// 10 on success, otherwise an error. +// If the revision does not match a tag add the remote origin in the revision. +func (r *Repo) RevParseTagShort(rev string) (string, error) { + fullRev, err := r.RevParseTag(rev) + if err != nil { + return "", err + } + + return fullRev[:10], nil +} + +// RevParseTagShort parses a git revision and returns a SHA1 trimmed to the length // 10 on success, otherwise an error. func (r *Repo) RevParseShort(rev string) (string, error) { fullRev, err := r.RevParse(rev) @@ -519,7 +544,7 @@ func (r *Repo) LatestNonPatchFinalToMinor() (DiscoverResult, error) { latestVersion := versions[0] latestVersionTag := util.SemverToTagString(latestVersion) logrus.Debugf("Latest non patch version %s", latestVersionTag) - end, err := r.RevParse(latestVersionTag) + end, err := r.RevParseTag(latestVersionTag) if err != nil { return DiscoverResult{}, err } @@ -527,7 +552,7 @@ func (r *Repo) LatestNonPatchFinalToMinor() (DiscoverResult, error) { previousVersion := versions[1] previousVersionTag := util.SemverToTagString(previousVersion) logrus.Debugf("Previous non patch version %s", previousVersionTag) - start, err := r.RevParse(previousVersionTag) + start, err := r.RevParseTag(previousVersionTag) if err != nil { return DiscoverResult{}, err } @@ -569,13 +594,13 @@ func (r *Repo) latestNonPatchFinalVersions() ([]semver.Version, error) { func (r *Repo) releaseBranchOrMainRef(major, minor uint64) (sha, rev string, err error) { relBranch := fmt.Sprintf("release-%d.%d", major, minor) - sha, err = r.RevParse(relBranch) + sha, err = r.RevParseTag(relBranch) if err == nil { logrus.Debugf("Found release branch %s", relBranch) return sha, relBranch, nil } - sha, err = r.RevParse(DefaultBranch) + sha, err = r.RevParseTag(DefaultBranch) if err == nil { logrus.Debugf("No release branch found, using %s", DefaultBranch) return sha, DefaultBranch, nil @@ -794,14 +819,14 @@ func (r *Repo) LatestPatchToPatch(branch string) (DiscoverResult, error) { logrus.Debugf("Parsing latest tag %s%v", util.TagPrefix, latestTag) latestVersionTag := util.SemverToTagString(latestTag) - end, err := r.RevParse(latestVersionTag) + end, err := r.RevParseTag(latestVersionTag) if err != nil { return DiscoverResult{}, errors.Wrapf(err, "parsing version %v", latestTag) } logrus.Debugf("Parsing previous tag %s%v", util.TagPrefix, prevTag) previousVersionTag := util.SemverToTagString(prevTag) - start, err := r.RevParse(previousVersionTag) + start, err := r.RevParseTag(previousVersionTag) if err != nil { return DiscoverResult{}, errors.Wrapf(err, "parsing previous version %v", prevTag) } @@ -830,7 +855,7 @@ func (r *Repo) LatestPatchToLatest(branch string) (DiscoverResult, error) { logrus.Debugf("Parsing latest tag %s%v", util.TagPrefix, latestTag) latestVersionTag := util.SemverToTagString(latestTag) - start, err := r.RevParse(latestVersionTag) + start, err := r.RevParseTag(latestVersionTag) if err != nil { return DiscoverResult{}, errors.Wrapf(err, "parsing version %v", latestTag) } diff --git a/pkg/git/git_integration_test.go b/pkg/git/git_integration_test.go index ac6f942262bb..428ad15c2bd0 100644 --- a/pkg/git/git_integration_test.go +++ b/pkg/git/git_integration_test.go @@ -341,6 +341,27 @@ func TestSuccessRevParse(t *testing.T) { tagRev, err := testRepo.sut.RevParse(testRepo.firstTagName) require.Nil(t, err) require.Equal(t, tagRev, testRepo.firstCommit) + + tagRev, err = testRepo.sut.RevParse(testRepo.firstCommit) + require.Nil(t, err) + require.Equal(t, tagRev, testRepo.firstCommit) +} + +func TestSuccessRevTagParse(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + mainRev, err := testRepo.sut.RevParseTag(git.DefaultBranch) + require.Nil(t, err) + require.Equal(t, mainRev, testRepo.firstCommit) + + branchRev, err := testRepo.sut.RevParseTag(testRepo.branchName) + require.Nil(t, err) + require.Equal(t, branchRev, testRepo.thirdBranchCommit) + + tagRev, err := testRepo.sut.RevParseTag(testRepo.firstTagName) + require.Nil(t, err) + require.Equal(t, tagRev, testRepo.firstCommit) } func TestFailureRevParse(t *testing.T) { @@ -351,6 +372,17 @@ func TestFailureRevParse(t *testing.T) { require.NotNil(t, err) } +func TestFailureRevParseTag(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + _, err := testRepo.sut.RevParseTag("wrong") + require.NotNil(t, err) + + _, err = testRepo.sut.RevParseTag(testRepo.firstCommit) + require.NotNil(t, err) +} + func TestSuccessRevParseShort(t *testing.T) { testRepo := newTestRepo(t) defer testRepo.cleanup(t) @@ -366,6 +398,27 @@ func TestSuccessRevParseShort(t *testing.T) { tagRev, err := testRepo.sut.RevParseShort(testRepo.firstTagName) require.Nil(t, err) require.Equal(t, tagRev, testRepo.firstCommit[:10]) + + tagRev, err = testRepo.sut.RevParseShort(testRepo.firstCommit) + require.Nil(t, err) + require.Equal(t, tagRev, testRepo.firstCommit[:10]) +} + +func TestSuccessRevParseTagShort(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + mainRev, err := testRepo.sut.RevParseTagShort(git.DefaultBranch) + require.Nil(t, err) + require.Equal(t, mainRev, testRepo.firstCommit[:10]) + + branchRev, err := testRepo.sut.RevParseTagShort(testRepo.branchName) + require.Nil(t, err) + require.Equal(t, branchRev, testRepo.thirdBranchCommit[:10]) + + tagRev, err := testRepo.sut.RevParseTagShort(testRepo.firstTagName) + require.Nil(t, err) + require.Equal(t, tagRev, testRepo.firstCommit[:10]) } func TestFailureRevParseShort(t *testing.T) { @@ -376,6 +429,17 @@ func TestFailureRevParseShort(t *testing.T) { require.NotNil(t, err) } +func TestFailureRevParseTagShort(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + _, err := testRepo.sut.RevParseTagShort("wrong") + require.NotNil(t, err) + + _, err = testRepo.sut.RevParseTagShort(testRepo.firstCommit) + require.NotNil(t, err) +} + func TestSuccessPush(t *testing.T) { testRepo := newTestRepo(t) defer testRepo.cleanup(t) diff --git a/pkg/notes/options/options.go b/pkg/notes/options/options.go index a44241b805eb..3d0dfd50ad87 100644 --- a/pkg/notes/options/options.go +++ b/pkg/notes/options/options.go @@ -210,7 +210,7 @@ func (o *Options) ValidateAndFinish() (err error) { return err } if o.StartRev != "" && o.StartSHA == "" { - sha, err := repo.RevParse(o.StartRev) + sha, err := repo.RevParseTag(o.StartRev) if err != nil { return errors.Wrapf(err, "resolving %s", o.StartRev) } @@ -218,7 +218,7 @@ func (o *Options) ValidateAndFinish() (err error) { o.StartSHA = sha } if o.EndRev != "" && o.EndSHA == "" { - sha, err := repo.RevParse(o.EndRev) + sha, err := repo.RevParseTag(o.EndRev) if err != nil { return errors.Wrapf(err, "resolving %s", o.EndRev) } diff --git a/pkg/release/repository.go b/pkg/release/repository.go index 91037d2d15e1..a88558dde398 100644 --- a/pkg/release/repository.go +++ b/pkg/release/repository.go @@ -160,10 +160,20 @@ func (r *Repo) CheckState(expOrg, expRepo, expRev string, nomock bool) error { ) logrus.Info("Verifying remote HEAD commit") - logrus.Infof("Verifying remote HEAD commit -> %s / %s / %s", foundRemote.Name(), expRev, branch) - lsRemoteOut, err := r.repo.LsRemote( - "--heads", foundRemote.Name(), "refs/heads/"+branch, - ) + args := []string{ + "--heads", + foundRemote.Name(), + fmt.Sprintf("refs/heads/%s", branch), + } + if branch == "" { + args = []string{ + "--tags", + "--heads", + foundRemote.Name(), + fmt.Sprintf("refs/tags/%s^{}", expRev), + } + } + lsRemoteOut, err := r.repo.LsRemote(args...) if err != nil { return errors.Wrap(err, "getting remote HEAD") } diff --git a/pkg/release/repository_test.go b/pkg/release/repository_test.go index 1d3f7f922658..26137c56be38 100644 --- a/pkg/release/repository_test.go +++ b/pkg/release/repository_test.go @@ -95,8 +95,9 @@ func TestCheckStateSuccess(t *testing.T) { sut.mock.RemotesReturns([]*git.Remote{ git.NewRemote("origin", []string{"github.com:org/repo"}), }, nil) - sut.mock.LsRemoteReturns("dbade8e refs/heads/master", nil) sut.mock.HeadReturns("dbade8e", nil) + sut.mock.RevParseReturns("dbade8e", nil) + sut.mock.LsRemoteReturns("dbade8e refs/heads/master", nil) // When err := sut.repo.CheckState("org", "repo", "branch", false)