pkg/git: introduce concrete and partial commit

Introduce concrete and partial commits. Concrete commits have all the
information from remote including the hash and commit content. Partial
commits are based on locally available copy of a repo, they may only
contain the commit hash and reference.

IsConcreteCommit() can be used to find out if a given commit is based on
local information or full remote repo information.

Update go-git and libgit2 branch/tag clone optimization to return a
partial commit and no error.

Update and simplify the go-git and libgit2 tests for the same.

Signed-off-by: Sunny <[email protected]>

Author: darkowlzz

pkg/git/git.go

@@ -118,3 +118,13 @@ type NoChangesError struct {
 func (e NoChangesError) Error() string {
 	return fmt.Sprintf("%s: observed revision '%s'", e.Message, e.ObservedRevision)
 }
+
+// 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.
+func IsConcreteCommit(c Commit) bool {
+	if c.Hash != nil && c.Encoded != nil {
+		return true
+	}
+	return false
+}

pkg/git/git_test.go

@@ -18,6 +18,7 @@ package git
 
 import (
 	"testing"
+	"time"
 
 	. "github.com/onsi/gomega"
 )
@@ -263,3 +264,41 @@ of the commit`,
 		})
 	}
 }
+
+func TestIsConcreteCommit(t *testing.T) {
+	tests := []struct {
+		name   string
+		commit Commit
+		result bool
+	}{
+		{
+			name: "concrete commit",
+			commit: Commit{
+				Hash:      Hash("foo"),
+				Reference: "refs/tags/main",
+				Author: Signature{
+					Name: "user", Email: "[email protected]", When: time.Now(),
+				},
+				Committer: Signature{
+					Name: "user", Email: "[email protected]", When: time.Now(),
+				},
+				Signature: "signature",
+				Encoded:   []byte("commit-content"),
+				Message:   "commit-message",
+			},
+			result: true,
+		},
+		{
+			name:   "partial commit",
+			commit: Commit{Hash: Hash("foo")},
+			result: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			g.Expect(IsConcreteCommit(tt.commit)).To(Equal(tt.result))
+		})
+	}
+}

pkg/git/gogit/checkout.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"io"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/Masterminds/semver/v3"
@@ -78,10 +79,21 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g
 		}
 
 		if currentRevision != "" && currentRevision == c.LastRevision {
-			return nil, git.NoChangesError{
-				Message:          "no changes since last reconcilation",
-				ObservedRevision: currentRevision,
+			// Construct a partial commit with the existing information.
+			// Split the revision and take the last part as the hash.
+			// Example revision: main/43d7eb9c49cdd49b2494efd481aea1166fc22b67
+			var hash git.Hash
+			ss := strings.Split(currentRevision, "/")
+			if len(ss) > 1 {
+				hash = git.Hash(ss[len(ss)-1])
+			} else {
+				hash = git.Hash(ss[0])
 			}
+			c := &git.Commit{
+				Hash:      hash,
+				Reference: plumbing.NewBranchReferenceName(c.Branch).String(),
+			}
+			return c, nil
 		}
 	}
 
@@ -153,10 +165,21 @@ func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.
 		}
 
 		if currentRevision != "" && currentRevision == c.LastRevision {
-			return nil, git.NoChangesError{
-				Message:          "no changes since last reconcilation",
-				ObservedRevision: currentRevision,
+			// Construct a partial commit with the existing information.
+			// Split the revision and take the last part as the hash.
+			// Example revision: 6.1.4/bf09377bfd5d3bcac1e895fa8ce52dc76695c060
+			var hash git.Hash
+			ss := strings.Split(currentRevision, "/")
+			if len(ss) > 1 {
+				hash = git.Hash(ss[len(ss)-1])
+			} else {
+				hash = git.Hash(ss[0])
+			}
+			c := &git.Commit{
+				Hash:      hash,
+				Reference: ref.String(),
 			}
+			return c, nil
 		}
 	}
 	repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{

pkg/git/gogit/checkout_test.go

@@ -67,32 +67,36 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
 	}
 
 	tests := []struct {
-		name           string
-		branch         string
-		filesCreated   map[string]string
-		expectedCommit string
-		expectedErr    string
-		lastRevision   string
+		name                   string
+		branch                 string
+		filesCreated           map[string]string
+		lastRevision           string
+		expectedCommit         string
+		expectedConcreteCommit bool
+		expectedErr            string
 	}{
 		{
-			name:           "Default branch",
-			branch:         "master",
-			filesCreated:   map[string]string{"branch": "init"},
-			expectedCommit: firstCommit.String(),
+			name:                   "Default branch",
+			branch:                 "master",
+			filesCreated:           map[string]string{"branch": "init"},
+			expectedCommit:         firstCommit.String(),
+			expectedConcreteCommit: true,
 		},
 		{
-			name:         "skip clone if LastRevision hasn't changed",
-			branch:       "master",
-			filesCreated: map[string]string{"branch": "init"},
-			expectedErr:  fmt.Sprintf("no changes since last reconcilation: observed revision 'master/%s'", firstCommit.String()),
-			lastRevision: fmt.Sprintf("master/%s", firstCommit.String()),
+			name:                   "skip clone if LastRevision hasn't changed",
+			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"},
-			expectedCommit: secondCommit.String(),
-			lastRevision:   fmt.Sprintf("master/%s", firstCommit.String()),
+			name:                   "Other branch - revision has changed",
+			branch:                 "test",
+			filesCreated:           map[string]string{"branch": "second"},
+			lastRevision:           fmt.Sprintf("master/%s", firstCommit.String()),
+			expectedCommit:         secondCommit.String(),
+			expectedConcreteCommit: true,
 		},
 		{
 			name:        "Non existing branch",
@@ -120,58 +124,64 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
 			}
 			g.Expect(err).ToNot(HaveOccurred())
 			g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit))
+			g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectedConcreteCommit))
 
-			for k, v := range tt.filesCreated {
-				g.Expect(filepath.Join(tmpDir, k)).To(BeARegularFile())
-				g.Expect(os.ReadFile(filepath.Join(tmpDir, k))).To(BeEquivalentTo(v))
+			if tt.expectedConcreteCommit {
+				for k, v := range tt.filesCreated {
+					g.Expect(filepath.Join(tmpDir, k)).To(BeARegularFile())
+					g.Expect(os.ReadFile(filepath.Join(tmpDir, k))).To(BeEquivalentTo(v))
+				}
 			}
 		})
 	}
 }
 
 func TestCheckoutTag_Checkout(t *testing.T) {
+	type testTag struct {
+		name      string
+		annotated bool
+	}
+
 	tests := []struct {
-		name        string
-		tag         string
-		annotated   bool
-		checkoutTag string
-		expectTag   string
-		expectErr   string
-		lastRev     string
-		setLastRev  bool
+		name                 string
+		tagsInRepo           []testTag
+		checkoutTag          string
+		lastRevTag           string
+		expectConcreteCommit bool
+		expectErr            string
 	}{
 		{
-			name:        "Tag",
-			tag:         "tag-1",
-			checkoutTag: "tag-1",
-			expectTag:   "tag-1",
+			name:                 "Tag",
+			tagsInRepo:           []testTag{{"tag-1", false}},
+			checkoutTag:          "tag-1",
+			expectConcreteCommit: true,
 		},
 		{
-			name:        "Skip Tag if last revision hasn't changed",
-			tag:         "tag-2",
-			checkoutTag: "tag-2",
-			setLastRev:  true,
-			expectErr:   "no changes since last reconcilation",
+			name:                 "Annotated",
+			tagsInRepo:           []testTag{{"annotated", true}},
+			checkoutTag:          "annotated",
+			expectConcreteCommit: true,
 		},
 		{
-			name:        "Last revision changed",
-			tag:         "tag-3",
-			checkoutTag: "tag-3",
-			expectTag:   "tag-3",
-			lastRev:     "tag-3/<fake-hash>",
+			name: "Non existing tag",
+			// Without this go-git returns error "remote repository is empty".
+			tagsInRepo:  []testTag{{"tag-1", false}},
+			checkoutTag: "invalid",
+			expectErr:   "couldn't find remote ref \"refs/tags/invalid\"",
 		},
 		{
-			name:        "Annotated",
-			tag:         "annotated",
-			annotated:   true,
-			checkoutTag: "annotated",
-			expectTag:   "annotated",
+			name:                 "Skip clone - last revision unchanged",
+			tagsInRepo:           []testTag{{"tag-1", false}},
+			checkoutTag:          "tag-1",
+			lastRevTag:           "tag-1",
+			expectConcreteCommit: false,
 		},
 		{
-			name:        "Non existing tag",
-			tag:         "tag-1",
-			checkoutTag: "invalid",
-			expectErr:   "couldn't find remote ref \"refs/tags/invalid\"",
+			name:                 "Last revision changed",
+			tagsInRepo:           []testTag{{"tag-1", false}, {"tag-2", false}},
+			checkoutTag:          "tag-2",
+			lastRevTag:           "tag-1",
+			expectConcreteCommit: true,
 		},
 	}
 	for _, tt := range tests {
@@ -183,43 +193,55 @@ func TestCheckoutTag_Checkout(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			var h plumbing.Hash
-			var tagHash *plumbing.Reference
-			if tt.tag != "" {
-				h, err = commitFile(repo, "tag", tt.tag, time.Now())
-				if err != nil {
-					t.Fatal(err)
-				}
-				tagHash, err = tag(repo, h, !tt.annotated, tt.tag, time.Now())
-				if err != nil {
-					t.Fatal(err)
+			// Collect tags and their associated commit hash for later
+			// reference.
+			tagCommits := map[string]string{}
+
+			// Populate the repo with commits and tags.
+			if tt.tagsInRepo != nil {
+				for _, tr := range tt.tagsInRepo {
+					h, err := commitFile(repo, "tag", tr.name, time.Now())
+					if err != nil {
+						t.Fatal(err)
+					}
+					_, err = tag(repo, h, tr.annotated, tr.name, time.Now())
+					if err != nil {
+						t.Fatal(err)
+					}
+					tagCommits[tr.name] = h.String()
 				}
 			}
 
-			tag := CheckoutTag{
+			checkoutTag := CheckoutTag{
 				Tag: tt.checkoutTag,
 			}
-			if tt.setLastRev {
-				tag.LastRevision = fmt.Sprintf("%s/%s", tt.tag, tagHash.Hash().String())
+			// If last revision is provided, configure it.
+			if tt.lastRevTag != "" {
+				lc := tagCommits[tt.lastRevTag]
+				checkoutTag.LastRevision = fmt.Sprintf("%s/%s", tt.lastRevTag, lc)
 			}
 
-			if tt.lastRev != "" {
-				tag.LastRevision = tt.lastRev
-			}
 			tmpDir := t.TempDir()
 
-			cc, err := tag.Checkout(context.TODO(), tmpDir, path, nil)
+			cc, err := checkoutTag.Checkout(context.TODO(), tmpDir, path, nil)
 			if tt.expectErr != "" {
 				g.Expect(err).ToNot(BeNil())
 				g.Expect(err.Error()).To(ContainSubstring(tt.expectErr))
 				g.Expect(cc).To(BeNil())
 				return
 			}
 
+			// Check successful checkout results.
+			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.expectTag + "/" + h.String()))
-			g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile())
-			g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.tag))
+			g.Expect(cc.String()).To(Equal(tt.checkoutTag + "/" + targetTagHash))
+
+			// Check file content only when there's an actual checkout.
+			if tt.lastRevTag != tt.checkoutTag {
+				g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile())
+				g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.checkoutTag))
+			}
 		})
 	}
 }