Skip to content

Commit

Permalink
git: make LastObservedCommit backwards compatible
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@hiddeco
hiddeco committed Jan 12, 2023
1 parent 1130245 commit 1ac5d56
Show file tree
Hide file tree
Showing 7 changed files with 282 additions and 21 deletions.
2 changes: 1 addition & 1 deletion git/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 3 additions & 4 deletions git/gogit/clone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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 ""
Expand Down
26 changes: 21 additions & 5 deletions git/gogit/clone_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,31 @@ 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,
},
{
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,
},
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand All @@ -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) {
Expand Down
16 changes: 8 additions & 8 deletions git/libgit2/clone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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
}
}
Expand Down
22 changes: 19 additions & 3 deletions git/libgit2/clone_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,31 @@ 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,
},
{
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,
},
Expand Down Expand Up @@ -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)
Expand Down
69 changes: 69 additions & 0 deletions git/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
//
Expand All @@ -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:])
}
Expand Down
Loading

0 comments on commit 1ac5d56

Please sign in to comment.