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 dc5fccb7..1050f19f 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