From 24a228cadb121301f1a4f05b52c96fc368583cde Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 14 Nov 2022 11:24:17 +0000 Subject: [PATCH 1/4] git: change Commit#String format This changes the string format of `Commit` to use an `@` separator instead of `/`, and drops the usage of "HEAD" as a virtual named reference for commits without a named pointer (e.g. a Git branch and/or tag). Matching the (revision) format proposed in RFC-0005. Signed-off-by: Hidde Beydals --- git/git.go | 8 ++++---- git/git_test.go | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/git/git.go b/git/git.go index 2a534dbe..c737973e 100644 --- a/git/git.go +++ b/git/git.go @@ -67,9 +67,9 @@ type Commit struct { // for a "tag-1" tag. func (c *Commit) String() string { if short := strings.SplitAfterN(c.Reference, "/", 3); len(short) == 3 { - return fmt.Sprintf("%s/%s", short[2], c.Hash) + return fmt.Sprintf("%s@sha1:%s", short[2], c.Hash) } - return fmt.Sprintf("HEAD/%s", c.Hash) + return fmt.Sprintf("sha1:%s", c.Hash) } // Verify the Signature of the commit with the given key rings. @@ -121,8 +121,8 @@ var ( ) // IsConcreteCommit returns if a given commit is a concrete commit. Concrete -// commits have most of commit metadata and commit content. In contrast, a -// partial commit may only have some metadata and no commit content. +// commits have most of the commit metadata and content. In contrast, a partial +// commit may only have some metadata and no commit content. func IsConcreteCommit(c Commit) bool { if c.Hash != nil && c.Encoded != nil { return true diff --git a/git/git_test.go b/git/git_test.go index 4001b179..3650d21c 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -131,7 +131,7 @@ func TestCommit_String(t *testing.T) { Hash: []byte("commit"), Reference: "refs/heads/main", }, - want: "main/commit", + want: "main@sha1:commit", }, { name: "Reference with slash and commit", @@ -139,14 +139,14 @@ func TestCommit_String(t *testing.T) { Hash: []byte("commit"), Reference: "refs/heads/feature/branch", }, - want: "feature/branch/commit", + want: "feature/branch@sha1:commit", }, { - name: "No reference", + name: "No name reference", commit: &Commit{ Hash: []byte("commit"), }, - want: "HEAD/commit", + want: "sha1:commit", }, } for _, tt := range tests { From b0976866892698c97c80a86bbb402ee4a76afab7 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 14 Nov 2022 11:47:48 +0000 Subject: [PATCH 2/4] git: align tests and code with commit fmt change Signed-off-by: Hidde Beydals --- git/gogit/clone.go | 19 +++++++++---------- git/gogit/clone_test.go | 20 ++++++++++---------- git/libgit2/clone.go | 6 +++--- git/libgit2/clone_test.go | 14 +++++++------- 4 files changed, 29 insertions(+), 30 deletions(-) diff --git a/git/gogit/clone.go b/git/gogit/clone.go index 9ed96c07..02141fb7 100644 --- a/git/gogit/clone.go +++ b/git/gogit/clone.go @@ -58,13 +58,13 @@ func (g *Client) cloneBranch(ctx context.Context, url, branch string, opts repos if head != "" && head == opts.LastObservedCommit { // Construct a non-concrete commit with the existing information. // Split the revision and take the last part as the hash. - // Example revision: main/43d7eb9c49cdd49b2494efd481aea1166fc22b67 + // Example revision: main@sha1:43d7eb9c49cdd49b2494efd481aea1166fc22b67 var hash git.Hash - ss := strings.Split(head, "/") + ss := strings.Split(head, "@") if len(ss) > 1 { - hash = git.Hash(ss[len(ss)-1]) + hash = git.Hash(strings.TrimPrefix(ss[len(ss)-1], "sha1:")) } else { - hash = git.Hash(ss[0]) + hash = git.Hash(strings.TrimPrefix(ss[0], "sha1:")) } c := &git.Commit{ Hash: hash, @@ -148,13 +148,13 @@ func (g *Client) cloneTag(ctx context.Context, url, tag string, opts repository. if head != "" && head == opts.LastObservedCommit { // Construct a non-concrete commit with the existing information. // Split the revision and take the last part as the hash. - // Example revision: 6.1.4/bf09377bfd5d3bcac1e895fa8ce52dc76695c060 + // Example revision: 6.1.4@sha1:bf09377bfd5d3bcac1e895fa8ce52dc76695c060 var hash git.Hash - ss := strings.Split(head, "/") + ss := strings.Split(head, "@") if len(ss) > 1 { - hash = git.Hash(ss[len(ss)-1]) + hash = git.Hash(strings.TrimPrefix(ss[len(ss)-1], "sha1:")) } else { - hash = git.Hash(ss[0]) + hash = git.Hash(strings.TrimPrefix(ss[0], "sha1:")) } c := &git.Commit{ Hash: hash, @@ -406,10 +406,9 @@ func getRemoteHEAD(ctx context.Context, url string, ref plumbing.ReferenceName, func filterRefs(refs []*plumbing.Reference, currentRef plumbing.ReferenceName) string { for _, ref := range refs { if ref.Name().String() == currentRef.String() { - return fmt.Sprintf("%s/%s", currentRef.Short(), ref.Hash().String()) + return fmt.Sprintf("%s@sha1:%s", currentRef.Short(), ref.Hash().String()) } } - return "" } diff --git a/git/gogit/clone_test.go b/git/gogit/clone_test.go index de2b4e3e..0aeacaa3 100644 --- a/git/gogit/clone_test.go +++ b/git/gogit/clone_test.go @@ -96,7 +96,7 @@ func TestClone_cloneBranch(t *testing.T) { name: "skip clone if LastRevision hasn't changed", branch: "master", filesCreated: map[string]string{"branch": "init"}, - lastRevision: fmt.Sprintf("master/%s", firstCommit.String()), + lastRevision: fmt.Sprintf("master@sha1:%s", firstCommit.String()), expectedCommit: firstCommit.String(), expectedConcreteCommit: false, }, @@ -104,7 +104,7 @@ func TestClone_cloneBranch(t *testing.T) { name: "Other branch - revision has changed", branch: "test", filesCreated: map[string]string{"branch": "second"}, - lastRevision: fmt.Sprintf("master/%s", firstCommit.String()), + lastRevision: fmt.Sprintf("master@sha1:%s", firstCommit.String()), expectedCommit: secondCommit.String(), expectedConcreteCommit: true, }, @@ -157,7 +157,7 @@ func TestClone_cloneBranch(t *testing.T) { } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit)) + g.Expect(cc.String()).To(Equal(tt.branch + "@sha1:" + tt.expectedCommit)) g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectedConcreteCommit)) if tt.expectedConcreteCommit { @@ -258,7 +258,7 @@ func TestClone_cloneTag(t *testing.T) { // If last revision is provided, configure it. if tt.lastRevTag != "" { lc := tagCommits[tt.lastRevTag] - opts.LastObservedCommit = fmt.Sprintf("%s/%s", tt.lastRevTag, lc) + opts.LastObservedCommit = fmt.Sprintf("%s@sha1:%s", tt.lastRevTag, lc) } cc, err := ggc.Clone(context.TODO(), path, opts) @@ -274,7 +274,7 @@ func TestClone_cloneTag(t *testing.T) { g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectConcreteCommit)) targetTagHash := tagCommits[tt.checkoutTag] g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.checkoutTag + "/" + targetTagHash)) + g.Expect(cc.String()).To(Equal(tt.checkoutTag + "@sha1:" + targetTagHash)) // Check file content only when there's an actual checkout. if tt.lastRevTag != tt.checkoutTag { @@ -314,14 +314,14 @@ func TestClone_cloneCommit(t *testing.T) { { name: "Commit", commit: firstCommit.String(), - expectCommit: "HEAD/" + firstCommit.String(), + expectCommit: "sha1:" + firstCommit.String(), expectFile: "init", }, { name: "Commit in specific branch", commit: secondCommit.String(), branch: "other-branch", - expectCommit: "other-branch/" + secondCommit.String(), + expectCommit: "other-branch@sha1:" + secondCommit.String(), expectFile: "second", }, { @@ -470,7 +470,7 @@ func TestClone_cloneSemVer(t *testing.T) { } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.expectTag + "/" + refs[tt.expectTag])) + g.Expect(cc.String()).To(Equal(tt.expectTag + "@sha1:" + refs[tt.expectTag])) g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.expectTag)) }) @@ -970,7 +970,7 @@ func Test_getRemoteHEAD(t *testing.T) { ref := plumbing.NewBranchReferenceName(git.DefaultBranch) head, err := getRemoteHEAD(context.TODO(), path, ref, &git.AuthOptions{}, nil) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(head).To(Equal(fmt.Sprintf("%s/%s", git.DefaultBranch, cc))) + g.Expect(head).To(Equal(fmt.Sprintf("%s@sha1:%s", git.DefaultBranch, cc))) cc, err = commitFile(repo, "test", "testing current head tag", time.Now()) g.Expect(err).ToNot(HaveOccurred()) @@ -980,7 +980,7 @@ func Test_getRemoteHEAD(t *testing.T) { ref = plumbing.NewTagReferenceName("v0.1.0") head, err = getRemoteHEAD(context.TODO(), path, ref, &git.AuthOptions{}, nil) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(head).To(Equal(fmt.Sprintf("%s/%s", "v0.1.0", cc))) + g.Expect(head).To(Equal(fmt.Sprintf("%s@sha1:%s", "v0.1.0", cc))) } func TestClone_CredentialsOverHttp(t *testing.T) { diff --git a/git/libgit2/clone.go b/git/libgit2/clone.go index 30eb62a7..5cd5d4e5 100644 --- a/git/libgit2/clone.go +++ b/git/libgit2/clone.go @@ -56,7 +56,7 @@ func (l *Client) cloneBranch(ctx context.Context, url, branch string, opts repos } if len(heads) > 0 { hash := heads[0].Id.String() - remoteHead := fmt.Sprintf("%s/%s", branch, hash) + remoteHead := fmt.Sprintf("%s@sha1:%s", branch, hash) if remoteHead == opts.LastObservedCommit { // Construct a non-concrete commit with the existing information. c := &git.Commit{ @@ -173,13 +173,13 @@ func (l *Client) cloneTag(ctx context.Context, url, tag string, opts repository. } if len(heads) > 0 { hash := heads[0].Id.String() - remoteHEAD := fmt.Sprintf("%s/%s", tag, hash) + remoteHEAD := fmt.Sprintf("%s@sha1:%s", tag, hash) var same bool if remoteHEAD == opts.LastObservedCommit { same = true } else if len(heads) > 1 { hash = heads[1].Id.String() - remoteAnnotatedHEAD := fmt.Sprintf("%s/%s", tag, hash) + remoteAnnotatedHEAD := fmt.Sprintf("%s@sha1:%s", tag, hash) if remoteAnnotatedHEAD == opts.LastObservedCommit { same = true } diff --git a/git/libgit2/clone_test.go b/git/libgit2/clone_test.go index 4d1d263a..b89955b2 100644 --- a/git/libgit2/clone_test.go +++ b/git/libgit2/clone_test.go @@ -114,7 +114,7 @@ func TestClone_cloneBranch(t *testing.T) { name: "skip clone - lastRevision hasn't changed", branch: defaultBranch, filesCreated: map[string]string{"branch": "second"}, - lastRevision: fmt.Sprintf("%s/%s", defaultBranch, secondCommit.String()), + lastRevision: fmt.Sprintf("%s@sha1:%s", defaultBranch, secondCommit.String()), expectedCommit: secondCommit.String(), expectedConcreteCommit: false, }, @@ -122,7 +122,7 @@ func TestClone_cloneBranch(t *testing.T) { name: "lastRevision is different", branch: defaultBranch, filesCreated: map[string]string{"branch": "second"}, - lastRevision: fmt.Sprintf("%s/%s", defaultBranch, firstCommit.String()), + lastRevision: fmt.Sprintf("%s@sha1:%s", defaultBranch, firstCommit.String()), expectedCommit: secondCommit.String(), expectedConcreteCommit: true, }, @@ -152,7 +152,7 @@ func TestClone_cloneBranch(t *testing.T) { return } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit)) + g.Expect(cc.String()).To(Equal(tt.branch + "@sha1:" + tt.expectedCommit)) g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectedConcreteCommit)) if tt.expectedConcreteCommit { @@ -272,7 +272,7 @@ func TestClone_cloneTag(t *testing.T) { // If last revision is provided, configure it. if tt.lastRevTag != "" { lc := tagCommits[tt.lastRevTag] - cloneOpts.LastObservedCommit = fmt.Sprintf("%s/%s", tt.lastRevTag, lc.Id().String()) + cloneOpts.LastObservedCommit = fmt.Sprintf("%s@sha1:%s", tt.lastRevTag, lc.Id().String()) } cc, err := lgc.Clone(context.TODO(), repoURL, cloneOpts) @@ -286,7 +286,7 @@ func TestClone_cloneTag(t *testing.T) { // Check successful checkout results. targetTagCommit := tagCommits[tt.checkoutTag] g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.checkoutTag + "/" + targetTagCommit.Id().String())) + g.Expect(cc.String()).To(Equal(tt.checkoutTag + "@sha1:" + targetTagCommit.Id().String())) g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectConcreteCommit)) // Check file content only when there's an actual checkout. @@ -348,7 +348,7 @@ func TestClone_cloneCommit(t *testing.T) { }) g.Expect(err).ToNot(HaveOccurred()) g.Expect(cc).ToNot(BeNil()) - g.Expect(cc.String()).To(Equal("HEAD/" + c.String())) + g.Expect(cc.String()).To(Equal("sha1:" + c.String())) g.Expect(filepath.Join(tmpDir, "commit")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "commit"))).To(BeEquivalentTo("init")) @@ -500,7 +500,7 @@ func TestClone_cloneSemVer(t *testing.T) { } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.expectTag + "/" + refs[tt.expectTag])) + g.Expect(cc.String()).To(Equal(tt.expectTag + "@sha1:" + refs[tt.expectTag])) g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.expectTag)) }) From 3fb1b65b2b54a2d7020a2744ed7a624300f8e7a5 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 16 Nov 2022 10:01:45 +0000 Subject: [PATCH 3/4] git: tidy code around digests Signed-off-by: Hidde Beydals --- git/git.go | 36 ++++++++++++++-- git/git_test.go | 89 ++++++++++++++++++++++++++++++++++++--- git/gogit/clone.go | 26 ++---------- git/gogit/clone_test.go | 20 ++++----- git/libgit2/clone.go | 6 +-- git/libgit2/clone_test.go | 14 +++--- git/utils.go | 20 +++++++++ git/utils_test.go | 46 ++++++++++++++++++++ 8 files changed, 204 insertions(+), 53 deletions(-) diff --git a/git/git.go b/git/git.go index c737973e..74c26b51 100644 --- a/git/git.go +++ b/git/git.go @@ -26,8 +26,36 @@ import ( "github.com/ProtonMail/go-crypto/openpgp" ) +const ( + // HashTypeSHA1 is the SHA1 hash algorithm. + HashTypeSHA1 = "sha1" + // HashTypeUnknown is an unknown hash algorithm. + HashTypeUnknown = "" +) + +// Hash is the (non-truncated) SHA-1 or SHA-256 hash of a Git commit. type Hash []byte +// Algorithm returns the algorithm of the hash based on its length. +// This is a heuristic, and may not be accurate for truncated user constructed +// hashes. The library itself does not produce truncated hashes. +func (h Hash) Algorithm() string { + switch len(h) { + case 40: + return HashTypeSHA1 + default: + return HashTypeUnknown + } +} + +// Digest returns a digest of the commit, in the format of ":". +func (h Hash) Digest() string { + if len(h) == 0 { + return "" + } + return fmt.Sprintf("%s:%s", h.Algorithm(), h) +} + // String returns the Hash as a string. func (h Hash) String() string { return string(h) @@ -43,7 +71,7 @@ type Signature struct { // Commit contains all possible information about a Git commit. type Commit struct { - // Hash is the SHA1 hash of the commit. + // Hash is the hash of the commit. Hash Hash // Reference is the original reference of the commit, for example: // 'refs/tags/foo'. @@ -57,7 +85,7 @@ type Commit struct { Signature string // Encoded is the encoded commit, without any signature. Encoded []byte - // Message is the commit message, contains arbitrary text. + // Message is the commit message, containing arbitrary text. Message string } @@ -67,9 +95,9 @@ type Commit struct { // for a "tag-1" tag. func (c *Commit) String() string { if short := strings.SplitAfterN(c.Reference, "/", 3); len(short) == 3 { - return fmt.Sprintf("%s@sha1:%s", short[2], c.Hash) + return fmt.Sprintf("%s@%s", short[2], c.Hash.Digest()) } - return fmt.Sprintf("sha1:%s", c.Hash) + return c.Hash.Digest() } // Verify the Signature of the commit with the given key rings. diff --git a/git/git_test.go b/git/git_test.go index 3650d21c..f5dace89 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -119,6 +119,83 @@ Bg2WzDuLKvQBi9tFSwnUbQoFFlOeiGW8G/bdkoJDWeS1oYgSD3nkmvXvrVESCrbT ` ) +func TestHash_Algorithm(t *testing.T) { + tests := []struct { + name string + hash Hash + want string + }{ + { + name: "SHA-1", + hash: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + want: HashTypeSHA1, + }, + { + name: "SHA-256", + hash: Hash("6ee9a7ade2ca791bc1bf9d133ef6ddaa9097cf521e6a19be92dbcc3f2e82f6d8"), + want: HashTypeUnknown, + }, + { + name: "MD5", + hash: Hash("dba535cd50b291777a055338572e4a4b"), + want: HashTypeUnknown, + }, + { + name: "Empty", + hash: Hash(""), + want: HashTypeUnknown, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(tt.hash.Algorithm()).To(Equal(tt.want)) + }) + } +} + +func TestHash_Digest(t *testing.T) { + tests := []struct { + name string + hash Hash + want string + }{ + { + name: "With a SHA-1 hash", + hash: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + want: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + }, + { + name: "With an unknown (MD5) hash", + hash: Hash("dba535cd50b291777a055338572e4a4b"), + want: ":dba535cd50b291777a055338572e4a4b", + }, + { + name: "With an unknown (SHA-256) hash", + hash: Hash("6ee9a7ade2ca791bc1bf9d133ef6ddaa9097cf521e6a19be92dbcc3f2e82f6d8"), + want: ":6ee9a7ade2ca791bc1bf9d133ef6ddaa9097cf521e6a19be92dbcc3f2e82f6d8", + }, + { + name: "With a nil hash", + hash: nil, + want: "", + }, + { + name: "With an empty hash", + hash: Hash(""), + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(tt.hash.Digest()).To(Equal(tt.want)) + }) + } +} + func TestCommit_String(t *testing.T) { tests := []struct { name string @@ -128,25 +205,25 @@ func TestCommit_String(t *testing.T) { { name: "Reference and commit", commit: &Commit{ - Hash: []byte("commit"), + Hash: []byte("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), Reference: "refs/heads/main", }, - want: "main@sha1:commit", + want: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", }, { name: "Reference with slash and commit", commit: &Commit{ - Hash: []byte("commit"), + Hash: []byte("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), Reference: "refs/heads/feature/branch", }, - want: "feature/branch@sha1:commit", + want: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", }, { name: "No name reference", commit: &Commit{ - Hash: []byte("commit"), + Hash: []byte("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), }, - want: "sha1:commit", + want: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", }, } for _, tt := range tests { diff --git a/git/gogit/clone.go b/git/gogit/clone.go index 02141fb7..9a0fc23b 100644 --- a/git/gogit/clone.go +++ b/git/gogit/clone.go @@ -56,18 +56,8 @@ func (g *Client) cloneBranch(ctx context.Context, url, branch string, opts repos } if head != "" && head == opts.LastObservedCommit { - // Construct a non-concrete commit with the existing information. - // Split the revision and take the last part as the hash. - // Example revision: main@sha1:43d7eb9c49cdd49b2494efd481aea1166fc22b67 - var hash git.Hash - ss := strings.Split(head, "@") - if len(ss) > 1 { - hash = git.Hash(strings.TrimPrefix(ss[len(ss)-1], "sha1:")) - } else { - hash = git.Hash(strings.TrimPrefix(ss[0], "sha1:")) - } c := &git.Commit{ - Hash: hash, + Hash: git.ExtractHashFromRevision(head), Reference: plumbing.NewBranchReferenceName(branch).String(), } return c, nil @@ -146,18 +136,8 @@ func (g *Client) cloneTag(ctx context.Context, url, tag string, opts repository. } if head != "" && head == opts.LastObservedCommit { - // Construct a non-concrete commit with the existing information. - // Split the revision and take the last part as the hash. - // Example revision: 6.1.4@sha1:bf09377bfd5d3bcac1e895fa8ce52dc76695c060 - var hash git.Hash - ss := strings.Split(head, "@") - if len(ss) > 1 { - hash = git.Hash(strings.TrimPrefix(ss[len(ss)-1], "sha1:")) - } else { - hash = git.Hash(strings.TrimPrefix(ss[0], "sha1:")) - } c := &git.Commit{ - Hash: hash, + Hash: git.ExtractHashFromRevision(head), Reference: ref.String(), } return c, nil @@ -406,7 +386,7 @@ func getRemoteHEAD(ctx context.Context, url string, ref plumbing.ReferenceName, func filterRefs(refs []*plumbing.Reference, currentRef plumbing.ReferenceName) string { for _, ref := range refs { if ref.Name().String() == currentRef.String() { - return fmt.Sprintf("%s@sha1:%s", currentRef.Short(), ref.Hash().String()) + return fmt.Sprintf("%s@%s:%s", currentRef.Short(), git.HashTypeSHA1, ref.Hash().String()) } } return "" diff --git a/git/gogit/clone_test.go b/git/gogit/clone_test.go index 0aeacaa3..a70ed4ef 100644 --- a/git/gogit/clone_test.go +++ b/git/gogit/clone_test.go @@ -96,7 +96,7 @@ func TestClone_cloneBranch(t *testing.T) { name: "skip clone if LastRevision hasn't changed", branch: "master", filesCreated: map[string]string{"branch": "init"}, - lastRevision: fmt.Sprintf("master@sha1:%s", firstCommit.String()), + lastRevision: fmt.Sprintf("master@%s:%s", git.HashTypeSHA1, firstCommit.String()), expectedCommit: firstCommit.String(), expectedConcreteCommit: false, }, @@ -104,7 +104,7 @@ func TestClone_cloneBranch(t *testing.T) { name: "Other branch - revision has changed", branch: "test", filesCreated: map[string]string{"branch": "second"}, - lastRevision: fmt.Sprintf("master@sha1:%s", firstCommit.String()), + lastRevision: fmt.Sprintf("master@%s:%s", git.HashTypeSHA1, firstCommit.String()), expectedCommit: secondCommit.String(), expectedConcreteCommit: true, }, @@ -157,7 +157,7 @@ func TestClone_cloneBranch(t *testing.T) { } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.branch + "@sha1:" + tt.expectedCommit)) + g.Expect(cc.String()).To(Equal(tt.branch + "@" + git.HashTypeSHA1 + ":" + tt.expectedCommit)) g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectedConcreteCommit)) if tt.expectedConcreteCommit { @@ -258,7 +258,7 @@ func TestClone_cloneTag(t *testing.T) { // If last revision is provided, configure it. if tt.lastRevTag != "" { lc := tagCommits[tt.lastRevTag] - opts.LastObservedCommit = fmt.Sprintf("%s@sha1:%s", tt.lastRevTag, lc) + opts.LastObservedCommit = fmt.Sprintf("%s@%s:%s", tt.lastRevTag, git.HashTypeSHA1, lc) } cc, err := ggc.Clone(context.TODO(), path, opts) @@ -274,7 +274,7 @@ func TestClone_cloneTag(t *testing.T) { g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectConcreteCommit)) targetTagHash := tagCommits[tt.checkoutTag] g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.checkoutTag + "@sha1:" + targetTagHash)) + g.Expect(cc.String()).To(Equal(tt.checkoutTag + "@" + git.HashTypeSHA1 + ":" + targetTagHash)) // Check file content only when there's an actual checkout. if tt.lastRevTag != tt.checkoutTag { @@ -314,14 +314,14 @@ func TestClone_cloneCommit(t *testing.T) { { name: "Commit", commit: firstCommit.String(), - expectCommit: "sha1:" + firstCommit.String(), + expectCommit: git.HashTypeSHA1 + ":" + firstCommit.String(), expectFile: "init", }, { name: "Commit in specific branch", commit: secondCommit.String(), branch: "other-branch", - expectCommit: "other-branch@sha1:" + secondCommit.String(), + expectCommit: "other-branch@" + git.HashTypeSHA1 + ":" + secondCommit.String(), expectFile: "second", }, { @@ -470,7 +470,7 @@ func TestClone_cloneSemVer(t *testing.T) { } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.expectTag + "@sha1:" + refs[tt.expectTag])) + g.Expect(cc.String()).To(Equal(tt.expectTag + "@" + git.HashTypeSHA1 + ":" + refs[tt.expectTag])) g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.expectTag)) }) @@ -970,7 +970,7 @@ func Test_getRemoteHEAD(t *testing.T) { ref := plumbing.NewBranchReferenceName(git.DefaultBranch) head, err := getRemoteHEAD(context.TODO(), path, ref, &git.AuthOptions{}, nil) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(head).To(Equal(fmt.Sprintf("%s@sha1:%s", git.DefaultBranch, cc))) + g.Expect(head).To(Equal(fmt.Sprintf("%s@%s:%s", git.DefaultBranch, git.HashTypeSHA1, cc))) cc, err = commitFile(repo, "test", "testing current head tag", time.Now()) g.Expect(err).ToNot(HaveOccurred()) @@ -980,7 +980,7 @@ func Test_getRemoteHEAD(t *testing.T) { ref = plumbing.NewTagReferenceName("v0.1.0") head, err = getRemoteHEAD(context.TODO(), path, ref, &git.AuthOptions{}, nil) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(head).To(Equal(fmt.Sprintf("%s@sha1:%s", "v0.1.0", cc))) + g.Expect(head).To(Equal(fmt.Sprintf("%s@%s:%s", "v0.1.0", git.HashTypeSHA1, cc))) } func TestClone_CredentialsOverHttp(t *testing.T) { diff --git a/git/libgit2/clone.go b/git/libgit2/clone.go index 5cd5d4e5..ea60d51d 100644 --- a/git/libgit2/clone.go +++ b/git/libgit2/clone.go @@ -56,7 +56,7 @@ func (l *Client) cloneBranch(ctx context.Context, url, branch string, opts repos } if len(heads) > 0 { hash := heads[0].Id.String() - remoteHead := fmt.Sprintf("%s@sha1:%s", branch, hash) + remoteHead := fmt.Sprintf("%s@%s:%s", branch, git.HashTypeSHA1, hash) if remoteHead == opts.LastObservedCommit { // Construct a non-concrete commit with the existing information. c := &git.Commit{ @@ -173,13 +173,13 @@ func (l *Client) cloneTag(ctx context.Context, url, tag string, opts repository. } if len(heads) > 0 { hash := heads[0].Id.String() - remoteHEAD := fmt.Sprintf("%s@sha1:%s", tag, hash) + remoteHEAD := fmt.Sprintf("%s@%s:%s", tag, git.HashTypeSHA1, hash) var same bool if remoteHEAD == opts.LastObservedCommit { same = true } else if len(heads) > 1 { hash = heads[1].Id.String() - remoteAnnotatedHEAD := fmt.Sprintf("%s@sha1:%s", tag, hash) + remoteAnnotatedHEAD := fmt.Sprintf("%s@%s:%s", tag, git.HashTypeSHA1, hash) if remoteAnnotatedHEAD == opts.LastObservedCommit { same = true } diff --git a/git/libgit2/clone_test.go b/git/libgit2/clone_test.go index b89955b2..89de1a38 100644 --- a/git/libgit2/clone_test.go +++ b/git/libgit2/clone_test.go @@ -114,7 +114,7 @@ func TestClone_cloneBranch(t *testing.T) { name: "skip clone - lastRevision hasn't changed", branch: defaultBranch, filesCreated: map[string]string{"branch": "second"}, - lastRevision: fmt.Sprintf("%s@sha1:%s", defaultBranch, secondCommit.String()), + lastRevision: fmt.Sprintf("%s@%s:%s", defaultBranch, git.HashTypeSHA1, secondCommit.String()), expectedCommit: secondCommit.String(), expectedConcreteCommit: false, }, @@ -122,7 +122,7 @@ func TestClone_cloneBranch(t *testing.T) { name: "lastRevision is different", branch: defaultBranch, filesCreated: map[string]string{"branch": "second"}, - lastRevision: fmt.Sprintf("%s@sha1:%s", defaultBranch, firstCommit.String()), + lastRevision: fmt.Sprintf("%s@%s:%s", defaultBranch, git.HashTypeSHA1, firstCommit.String()), expectedCommit: secondCommit.String(), expectedConcreteCommit: true, }, @@ -152,7 +152,7 @@ func TestClone_cloneBranch(t *testing.T) { return } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.branch + "@sha1:" + tt.expectedCommit)) + g.Expect(cc.String()).To(Equal(tt.branch + "@" + git.HashTypeSHA1 + ":" + tt.expectedCommit)) g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectedConcreteCommit)) if tt.expectedConcreteCommit { @@ -272,7 +272,7 @@ func TestClone_cloneTag(t *testing.T) { // If last revision is provided, configure it. if tt.lastRevTag != "" { lc := tagCommits[tt.lastRevTag] - cloneOpts.LastObservedCommit = fmt.Sprintf("%s@sha1:%s", tt.lastRevTag, lc.Id().String()) + cloneOpts.LastObservedCommit = fmt.Sprintf("%s@%s:%s", tt.lastRevTag, git.HashTypeSHA1, lc.Id().String()) } cc, err := lgc.Clone(context.TODO(), repoURL, cloneOpts) @@ -286,7 +286,7 @@ func TestClone_cloneTag(t *testing.T) { // Check successful checkout results. targetTagCommit := tagCommits[tt.checkoutTag] g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.checkoutTag + "@sha1:" + targetTagCommit.Id().String())) + g.Expect(cc.String()).To(Equal(tt.checkoutTag + "@" + git.HashTypeSHA1 + ":" + targetTagCommit.Id().String())) g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectConcreteCommit)) // Check file content only when there's an actual checkout. @@ -348,7 +348,7 @@ func TestClone_cloneCommit(t *testing.T) { }) g.Expect(err).ToNot(HaveOccurred()) g.Expect(cc).ToNot(BeNil()) - g.Expect(cc.String()).To(Equal("sha1:" + c.String())) + g.Expect(cc.String()).To(Equal(git.HashTypeSHA1 + ":" + c.String())) g.Expect(filepath.Join(tmpDir, "commit")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "commit"))).To(BeEquivalentTo("init")) @@ -500,7 +500,7 @@ func TestClone_cloneSemVer(t *testing.T) { } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(cc.String()).To(Equal(tt.expectTag + "@sha1:" + refs[tt.expectTag])) + g.Expect(cc.String()).To(Equal(tt.expectTag + "@" + git.HashTypeSHA1 + ":" + refs[tt.expectTag])) g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.expectTag)) }) diff --git a/git/utils.go b/git/utils.go index c7654cd1..f6764e51 100644 --- a/git/utils.go +++ b/git/utils.go @@ -19,6 +19,7 @@ package git import ( "os" "path/filepath" + "strings" securejoin "github.com/cyphar/filepath-securejoin" ) @@ -44,3 +45,22 @@ func SecurePath(path string) (string, error) { return joined, nil } + +// ExtractHashFromRevision extracts the hash from a revision string. It accepts +// the following formats: +// +// - main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - 5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +func ExtractHashFromRevision(rev string) Hash { + if i := strings.LastIndex(rev, ":"); i != -1 { + return Hash(rev[i+1:]) + } + if ss := strings.Split(rev, "/"); len(ss) > 1 { + return Hash(ss[len(ss)-1]) + } + return Hash(rev) +} diff --git a/git/utils_test.go b/git/utils_test.go index 6359e121..282dfc98 100644 --- a/git/utils_test.go +++ b/git/utils_test.go @@ -45,3 +45,49 @@ func TestSecurePath(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) g.Expect(securePath).To(Equal(filepath.Join(wd, "outside"))) } + +func TestExtractHashFromRevision(t *testing.T) { + tests := []struct { + name string + rev string + want Hash + }{ + { + name: "revision with branch and digest", + rev: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "revision with digest", + rev: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "revision with slash branch and digest", + rev: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "legacy revision with branch and hash", + rev: "main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "legacy revision with slash branch and hash", + rev: "feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "legacy revision with hash", + rev: "5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(ExtractHashFromRevision(tt.rev)).To(Equal(tt.want)) + }) + } +} From db0daab1f8440f700501fbd4a0520eb0c685ea31 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 13 Dec 2022 22:27:59 +0000 Subject: [PATCH 4/4] git: make LastObservedCommit backwards compatible This ensures the Git implementations transform any LastObservedCommit value into the new format before comparing it to what they constructed from the remote state. Signed-off-by: Hidde Beydals --- git/git.go | 2 +- git/gogit/clone.go | 7 +- git/gogit/clone_test.go | 26 ++++-- git/libgit2/clone.go | 16 ++-- git/libgit2/clone_test.go | 22 +++++- git/utils.go | 69 ++++++++++++++++ git/utils_test.go | 161 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 282 insertions(+), 21 deletions(-) diff --git a/git/git.go b/git/git.go index 74c26b51..fca22cd2 100644 --- a/git/git.go +++ b/git/git.go @@ -37,7 +37,7 @@ const ( type Hash []byte // Algorithm returns the algorithm of the hash based on its length. -// This is a heuristic, and may not be accurate for truncated user constructed +// This is heuristic, and may not be accurate for truncated user constructed // hashes. The library itself does not produce truncated hashes. func (h Hash) Algorithm() string { switch len(h) { diff --git a/git/gogit/clone.go b/git/gogit/clone.go index 9a0fc23b..ffa07c5d 100644 --- a/git/gogit/clone.go +++ b/git/gogit/clone.go @@ -49,13 +49,12 @@ func (g *Client) cloneBranch(ctx context.Context, url, branch string, opts repos ref := plumbing.NewBranchReferenceName(branch) // check if previous revision has changed before attempting to clone - if opts.LastObservedCommit != "" { + if lastObserved := git.TransformRevision(opts.LastObservedCommit); lastObserved != "" { head, err := getRemoteHEAD(ctx, url, ref, g.authOpts, authMethod) if err != nil { return nil, err } - - if head != "" && head == opts.LastObservedCommit { + if head != "" && head == lastObserved { c := &git.Commit{ Hash: git.ExtractHashFromRevision(head), Reference: plumbing.NewBranchReferenceName(branch).String(), @@ -386,7 +385,7 @@ func getRemoteHEAD(ctx context.Context, url string, ref plumbing.ReferenceName, func filterRefs(refs []*plumbing.Reference, currentRef plumbing.ReferenceName) string { for _, ref := range refs { if ref.Name().String() == currentRef.String() { - return fmt.Sprintf("%s@%s:%s", currentRef.Short(), git.HashTypeSHA1, ref.Hash().String()) + return fmt.Sprintf("%s@%s", currentRef.Short(), git.Hash(ref.Hash().String()).Digest()) } } return "" diff --git a/git/gogit/clone_test.go b/git/gogit/clone_test.go index a70ed4ef..394f112c 100644 --- a/git/gogit/clone_test.go +++ b/git/gogit/clone_test.go @@ -96,7 +96,15 @@ func TestClone_cloneBranch(t *testing.T) { name: "skip clone if LastRevision hasn't changed", branch: "master", filesCreated: map[string]string{"branch": "init"}, - lastRevision: fmt.Sprintf("master@%s:%s", git.HashTypeSHA1, firstCommit.String()), + lastRevision: fmt.Sprintf("master@%s", git.Hash(firstCommit.String()).Digest()), + expectedCommit: firstCommit.String(), + expectedConcreteCommit: false, + }, + { + name: "skip clone if LastRevision hasn't changed (legacy)", + branch: "master", + filesCreated: map[string]string{"branch": "init"}, + lastRevision: fmt.Sprintf("master/%s", firstCommit.String()), expectedCommit: firstCommit.String(), expectedConcreteCommit: false, }, @@ -104,7 +112,15 @@ func TestClone_cloneBranch(t *testing.T) { name: "Other branch - revision has changed", branch: "test", filesCreated: map[string]string{"branch": "second"}, - lastRevision: fmt.Sprintf("master@%s:%s", git.HashTypeSHA1, firstCommit.String()), + lastRevision: fmt.Sprintf("master@%s", git.Hash(firstCommit.String()).Digest()), + expectedCommit: secondCommit.String(), + expectedConcreteCommit: true, + }, + { + name: "Other branch - revision has changed (legacy)", + branch: "test", + filesCreated: map[string]string{"branch": "second"}, + lastRevision: fmt.Sprintf("master/%s", firstCommit.String()), expectedCommit: secondCommit.String(), expectedConcreteCommit: true, }, @@ -258,7 +274,7 @@ func TestClone_cloneTag(t *testing.T) { // If last revision is provided, configure it. if tt.lastRevTag != "" { lc := tagCommits[tt.lastRevTag] - opts.LastObservedCommit = fmt.Sprintf("%s@%s:%s", tt.lastRevTag, git.HashTypeSHA1, lc) + opts.LastObservedCommit = fmt.Sprintf("%s@%s", tt.lastRevTag, git.Hash(lc).Digest()) } cc, err := ggc.Clone(context.TODO(), path, opts) @@ -970,7 +986,7 @@ func Test_getRemoteHEAD(t *testing.T) { ref := plumbing.NewBranchReferenceName(git.DefaultBranch) head, err := getRemoteHEAD(context.TODO(), path, ref, &git.AuthOptions{}, nil) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(head).To(Equal(fmt.Sprintf("%s@%s:%s", git.DefaultBranch, git.HashTypeSHA1, cc))) + g.Expect(head).To(Equal(fmt.Sprintf("%s@%s", git.DefaultBranch, git.Hash(cc.String()).Digest()))) cc, err = commitFile(repo, "test", "testing current head tag", time.Now()) g.Expect(err).ToNot(HaveOccurred()) @@ -980,7 +996,7 @@ func Test_getRemoteHEAD(t *testing.T) { ref = plumbing.NewTagReferenceName("v0.1.0") head, err = getRemoteHEAD(context.TODO(), path, ref, &git.AuthOptions{}, nil) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(head).To(Equal(fmt.Sprintf("%s@%s:%s", "v0.1.0", git.HashTypeSHA1, cc))) + g.Expect(head).To(Equal(fmt.Sprintf("%s@%s", "v0.1.0", git.Hash(cc.String()).Digest()))) } func TestClone_CredentialsOverHttp(t *testing.T) { diff --git a/git/libgit2/clone.go b/git/libgit2/clone.go index ea60d51d..a9a8d749 100644 --- a/git/libgit2/clone.go +++ b/git/libgit2/clone.go @@ -49,15 +49,15 @@ func (l *Client) cloneBranch(ctx context.Context, url, branch string, opts repos // When the last observed revision is set, check whether it is still the // same at the remote branch. If so, short-circuit the clone operation here. - if opts.LastObservedCommit != "" { + if lastObserved := git.TransformRevision(opts.LastObservedCommit); lastObserved != "" { heads, err := l.remote.Ls(branch) if err != nil { return nil, fmt.Errorf("unable to remote ls for '%s': %w", url, libGit2Error(err)) } if len(heads) > 0 { hash := heads[0].Id.String() - remoteHead := fmt.Sprintf("%s@%s:%s", branch, git.HashTypeSHA1, hash) - if remoteHead == opts.LastObservedCommit { + remoteHead := fmt.Sprintf("%s@%s", branch, git.Hash(hash).Digest()) + if remoteHead == lastObserved { // Construct a non-concrete commit with the existing information. c := &git.Commit{ Hash: git.Hash(hash), @@ -166,21 +166,21 @@ func (l *Client) cloneTag(ctx context.Context, url, tag string, opts repository. // When the last observed revision is set, check whether it is still the // same at the remote branch. If so, short-circuit the clone operation here. - if opts.LastObservedCommit != "" { + if lastObserved := git.TransformRevision(opts.LastObservedCommit); lastObserved != "" { heads, err := l.remote.Ls(tag) if err != nil { return nil, fmt.Errorf("unable to remote ls for '%s': %w", url, libGit2Error(err)) } if len(heads) > 0 { hash := heads[0].Id.String() - remoteHEAD := fmt.Sprintf("%s@%s:%s", tag, git.HashTypeSHA1, hash) + remoteHEAD := fmt.Sprintf("%s@%s", tag, git.Hash(hash).Digest()) var same bool - if remoteHEAD == opts.LastObservedCommit { + if remoteHEAD == lastObserved { same = true } else if len(heads) > 1 { hash = heads[1].Id.String() - remoteAnnotatedHEAD := fmt.Sprintf("%s@%s:%s", tag, git.HashTypeSHA1, hash) - if remoteAnnotatedHEAD == opts.LastObservedCommit { + remoteAnnotatedHEAD := fmt.Sprintf("%s@%s", tag, git.Hash(hash).Digest()) + if remoteAnnotatedHEAD == lastObserved { same = true } } diff --git a/git/libgit2/clone_test.go b/git/libgit2/clone_test.go index 89de1a38..d768ad4e 100644 --- a/git/libgit2/clone_test.go +++ b/git/libgit2/clone_test.go @@ -114,7 +114,15 @@ func TestClone_cloneBranch(t *testing.T) { name: "skip clone - lastRevision hasn't changed", branch: defaultBranch, filesCreated: map[string]string{"branch": "second"}, - lastRevision: fmt.Sprintf("%s@%s:%s", defaultBranch, git.HashTypeSHA1, secondCommit.String()), + lastRevision: fmt.Sprintf("%s@%s", defaultBranch, git.Hash(secondCommit.String()).Digest()), + expectedCommit: secondCommit.String(), + expectedConcreteCommit: false, + }, + { + name: "skip clone - lastRevision hasn't changed (legacy)", + branch: defaultBranch, + filesCreated: map[string]string{"branch": "second"}, + lastRevision: fmt.Sprintf("%s/%s", defaultBranch, secondCommit.String()), expectedCommit: secondCommit.String(), expectedConcreteCommit: false, }, @@ -122,7 +130,15 @@ func TestClone_cloneBranch(t *testing.T) { name: "lastRevision is different", branch: defaultBranch, filesCreated: map[string]string{"branch": "second"}, - lastRevision: fmt.Sprintf("%s@%s:%s", defaultBranch, git.HashTypeSHA1, firstCommit.String()), + lastRevision: fmt.Sprintf("%s@%s", defaultBranch, git.Hash(firstCommit.String()).Digest()), + expectedCommit: secondCommit.String(), + expectedConcreteCommit: true, + }, + { + name: "lastRevision is different (legacy)", + branch: defaultBranch, + filesCreated: map[string]string{"branch": "second"}, + lastRevision: fmt.Sprintf("%s/%s", defaultBranch, firstCommit.String()), expectedCommit: secondCommit.String(), expectedConcreteCommit: true, }, @@ -272,7 +288,7 @@ func TestClone_cloneTag(t *testing.T) { // If last revision is provided, configure it. if tt.lastRevTag != "" { lc := tagCommits[tt.lastRevTag] - cloneOpts.LastObservedCommit = fmt.Sprintf("%s@%s:%s", tt.lastRevTag, git.HashTypeSHA1, lc.Id().String()) + cloneOpts.LastObservedCommit = fmt.Sprintf("%s@%s", tt.lastRevTag, git.Hash(lc.Id().String()).Digest()) } cc, err := lgc.Clone(context.TODO(), repoURL, cloneOpts) diff --git a/git/utils.go b/git/utils.go index f6764e51..8bcc0fce 100644 --- a/git/utils.go +++ b/git/utils.go @@ -46,6 +46,71 @@ func SecurePath(path string) (string, error) { return joined, nil } +// TransformRevision transforms a "legacy" revision string into a "new" +// revision string. It accepts the following formats: +// +// - main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - HEAD/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// +// Which are transformed into the following formats respectively: +// +// - main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// +// NOTE: This function is only intended to be used for backwards compatibility +// with the old revision format. It may be removed in a future release. +func TransformRevision(rev string) string { + if rev == "" || strings.LastIndex(rev, ":") >= 0 { + return rev + } + p, h := SplitRevision(rev) + if p == "" { + return h.Digest() + } + return p + "@" + h.Digest() +} + +// SplitRevision splits a revision string into it's named pointer and hash +// components. It accepts the following formats: +// +// - main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - HEAD/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - 5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// +// If the revision string does not contain a named pointer, the returned +// string will be empty. +func SplitRevision(rev string) (string, Hash) { + return ExtractNamedPointerFromRevision(rev), ExtractHashFromRevision(rev) +} + +// ExtractNamedPointerFromRevision extracts the named pointer from a revision +// string. It accepts the following formats: +// +// - main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// +// If the revision string does not contain a named pointer, the returned string +// is empty. +func ExtractNamedPointerFromRevision(rev string) string { + if i := strings.LastIndex(rev, "@"); i != -1 { + return rev[:i] + } + if i := strings.LastIndex(rev, "/"); i != -1 { + if s := rev[:i]; s != "HEAD" { + return s + } + } + return "" +} + // ExtractHashFromRevision extracts the hash from a revision string. It accepts // the following formats: // @@ -54,8 +119,12 @@ func SecurePath(path string) (string, error) { // - sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 // - main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 // - feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 +// - HEAD/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 // - 5394cb7f48332b2de7c17dd8b8384bbc84b7e738 func ExtractHashFromRevision(rev string) Hash { + if rev == "" { + return nil + } if i := strings.LastIndex(rev, ":"); i != -1 { return Hash(rev[i+1:]) } diff --git a/git/utils_test.go b/git/utils_test.go index 282dfc98..aa1cd0b1 100644 --- a/git/utils_test.go +++ b/git/utils_test.go @@ -46,6 +46,167 @@ func TestSecurePath(t *testing.T) { g.Expect(securePath).To(Equal(filepath.Join(wd, "outside"))) } +func TestTransformRevision(t *testing.T) { + tests := []struct { + name string + rev string + want string + }{ + { + name: "revision with branch and digest", + rev: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + }, + { + name: "revision with digest", + rev: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + }, + { + name: "revision with slash branch and digest", + rev: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + }, + { + name: "legacy revision with branch and hash", + rev: "main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + }, + { + name: "legacy revision with slash branch and hash", + rev: "feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + }, + { + name: "legacy revision with hash", + rev: "5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + }, + { + name: "legacy revision with HEAD named pointer and hash", + rev: "HEAD/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + }, + { + name: "empty revision", + rev: "", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := TransformRevision(tt.rev) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func TestSplitRevision(t *testing.T) { + tests := []struct { + name string + rev string + wantPointer string + wantHash Hash + }{ + { + name: "revision with branch and digest", + rev: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + wantPointer: "main", + wantHash: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "revision with digest", + rev: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + wantHash: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "revision with slash branch and digest", + rev: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + wantPointer: "feature/branch", + wantHash: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "legacy revision with branch and hash", + rev: "main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + wantPointer: "main", + wantHash: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "legacy revision with hash", + rev: "5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + wantHash: Hash("5394cb7f48332b2de7c17dd8b8384bbc84b7e738"), + }, + { + name: "empty revision", + rev: "", + wantPointer: "", + wantHash: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + p, h := SplitRevision(tt.rev) + g.Expect(p).To(Equal(tt.wantPointer)) + g.Expect(h).To(Equal(tt.wantHash)) + }) + } +} + +func TestExtractNamedPointerFromRevision(t *testing.T) { + tests := []struct { + name string + rev string + want string + }{ + { + name: "revision with branch and digest", + rev: "main@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "main", + }, + { + name: "revision with digest", + rev: "sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "", + }, + { + name: "revision with slash branch and digest", + rev: "feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "feature/branch", + }, + { + name: "legacy revision with branch and hash", + rev: "main/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "main", + }, + { + name: "legacy revision with slash branch and hash", + rev: "feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "feature/branch", + }, + { + name: "legacy revision with hash", + rev: "5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "", + }, + { + name: "legacy revision with HEAD named pointer and hash", + rev: "HEAD/5394cb7f48332b2de7c17dd8b8384bbc84b7e738", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(ExtractNamedPointerFromRevision(tt.rev)).To(Equal(tt.want)) + }) + } +} + func TestExtractHashFromRevision(t *testing.T) { tests := []struct { name string