Move tag checkout code to dedicated function

The code for checking out tags was duplicated. Already fairly complex, it turns out to need even more code to work reliably. For this reason, it will be beneficial to move it to a function in the gitutils package to avoid code duplication and facilitate testing.

Author: per1234

internal/libraries/git_integration_test.go

@@ -31,7 +31,6 @@ import (
 
 	"arduino.cc/repository/internal/libraries/db"
 	"arduino.cc/repository/internal/libraries/gitutils"
-	"github.com/go-git/go-git/v5"
 	"github.com/stretchr/testify/require"
 )
 
@@ -63,15 +62,7 @@ func TestUpdateLibraryJson(t *testing.T) {
 		tag, err := tags.Next()
 		require.NoError(t, err)
 
-		repoTree, err := r.Repository.Worktree()
-		require.NoError(t, err)
-		// Annotated tags have their own hash, different from the commit hash, so the tag must be resolved before checkout
-		resolvedTag, err := gitutils.ResolveTag(tag, r.Repository)
-		require.NoError(t, err)
-		err = repoTree.Checkout(&git.CheckoutOptions{Hash: *resolvedTag, Force: true})
-		require.NoError(t, err)
-		err = repoTree.Clean(&git.CleanOptions{Dir: true})
-		require.NoError(t, err)
+		err = gitutils.CheckoutTag(r.Repository, tag)
 
 		library, err := GenerateLibraryFromRepo(r)
 		require.NoError(t, err)

internal/libraries/gitutils/gitutils.go

@@ -31,8 +31,8 @@ import (
 	"github.com/go-git/go-git/v5/plumbing/object"
 )
 
-// ResolveTag returns the commit hash associated with a tag.
-func ResolveTag(tag *plumbing.Reference, repository *git.Repository) (*plumbing.Hash, error) {
+// resolveTag returns the commit hash associated with a tag.
+func resolveTag(tag *plumbing.Reference, repository *git.Repository) (*plumbing.Hash, error) {
 	// Annotated tags have their own hash, different from the commit hash, so the tag must be resolved to get the has for
 	// the associated commit.
 	// Tags may point to any Git object. Although not common, this can include tree and blob objects in addition to commits.
@@ -117,7 +117,7 @@ func SortedCommitTags(repository *git.Repository) ([]*plumbing.Reference, error)
 
 		// Annotated tags have their own hash, different from the commit hash, so tags must be resolved before
 		// cross-referencing against the commit hashes.
-		resolvedTag, err := ResolveTag(tag, repository)
+		resolvedTag, err := resolveTag(tag, repository)
 		if err != nil {
 			// Non-commit object tags are not included in the sorted list.
 			continue
@@ -156,3 +156,27 @@ func SortedCommitTags(repository *git.Repository) ([]*plumbing.Reference, error)
 
 	return sortedTags, nil
 }
+
+// CheckoutTag checks out the repository to the given tag.
+func CheckoutTag(repository *git.Repository, tag *plumbing.Reference) error {
+	repoTree, err := repository.Worktree()
+	if err != nil {
+		return err
+	}
+
+	// Annotated tags have their own hash, different from the commit hash, so the tag must be resolved before checkout
+	resolvedTag, err := resolveTag(tag, repository)
+	if err != nil {
+		return err
+	}
+
+	if err = repoTree.Checkout(&git.CheckoutOptions{Hash: *resolvedTag, Force: true}); err != nil {
+		return err
+	}
+
+	if err = repoTree.Clean(&git.CleanOptions{Dir: true}); err != nil {
+		return err
+	}
+
+	return nil
+}

internal/libraries/gitutils/gitutils_test.go

@@ -84,7 +84,7 @@ func TestResolveTag(t *testing.T) {
 		} {
 			testName := fmt.Sprintf("%s, %s", testTable.objectTypeName, annotationConfig.descriptor)
 			tag := makeTag(t, repository, testName, testTable.objectHash, annotationConfig.annotated)
-			resolvedTag, err := ResolveTag(tag, repository)
+			resolvedTag, err := resolveTag(tag, repository)
 			testTable.errorAssertion(t, err, fmt.Sprintf("%s tag resolution error", testName))
 			if err == nil {
 				assert.Equal(t, testTable.objectHash, *resolvedTag, fmt.Sprintf("%s tag resolution", testName))
@@ -139,9 +139,69 @@ func TestSortedCommitTags(t *testing.T) {
 	assert.Equal(t, tags, sorted)
 }
 
+func TestCheckoutTag(t *testing.T) {
+	// Create a folder for the test repository.
+	repositoryPath, err := paths.TempDir().MkTempDir("gitutils-TestCheckoutTag-repo")
+	require.NoError(t, err)
+
+	// Create test repository.
+	repository, err := git.PlainInit(repositoryPath.String(), false)
+	require.NoError(t, err)
+
+	// Generate meaningless commit history, creating some tags along the way.
+	var tags []*plumbing.Reference
+	tags = append(tags, makeTag(t, repository, "1.0.0", makeCommit(t, repository, repositoryPath), true))
+	makeCommit(t, repository, repositoryPath)
+	makeCommit(t, repository, repositoryPath)
+	tags = append(tags, makeTag(t, repository, "1.0.1", makeCommit(t, repository, repositoryPath), true))
+	makeCommit(t, repository, repositoryPath)
+	makeTag(t, repository, "tree-tag", getTreeHash(t, repository), true)
+	tags = append(tags, makeTag(t, repository, "1.0.2", makeCommit(t, repository, repositoryPath), true))
+	makeTag(t, repository, "blob-tag", getBlobHash(t, repository), true)
+	trackedFilePath, _ := commitFile(t, repository, repositoryPath)
+
+	for _, tag := range tags {
+		// Put the repository into a dirty state.
+		// Add an untracked file.
+		_, err = paths.WriteToTempFile([]byte{}, repositoryPath, "gitutils-TestCheckoutTag-tempfile")
+		require.NoError(t, err)
+		// Modify a tracked file.
+		err = trackedFilePath.WriteFile([]byte{42})
+		require.NoError(t, err)
+		// Create empty folder.
+		emptyFolderPath, err := repositoryPath.MkTempDir("gitutils-TestCheckoutTag-emptyFolder")
+		require.NoError(t, err)
+
+		err = CheckoutTag(repository, tag)
+		assert.NoError(t, err, fmt.Sprintf("Checking out tag %s", tag))
+
+		expectedHash, err := resolveTag(tag, repository)
+		require.NoError(t, err)
+		headRef, err := repository.Head()
+		require.NoError(t, err)
+		assert.Equal(t, *expectedHash, headRef.Hash(), "HEAD is at tag")
+
+		// Check if cleanup was successful.
+		tree, err := repository.Worktree()
+		require.NoError(t, err)
+		status, err := tree.Status()
+		require.NoError(t, err)
+		assert.True(t, status.IsClean(), "Repository is clean")
+		emptyFolderExists, err := emptyFolderPath.ExistCheck()
+		require.NoError(t, err)
+		assert.False(t, emptyFolderExists, "Empty folder was removed")
+	}
+}
+
 // makeCommit creates a test commit in the given repository and returns its plumbing.Hash object.
 func makeCommit(t *testing.T, repository *git.Repository, repositoryPath *paths.Path) plumbing.Hash {
-	_, err := paths.WriteToTempFile([]byte{}, repositoryPath, "gitutils-makeCommit-tempfile")
+	_, hash := commitFile(t, repository, repositoryPath)
+	return hash
+}
+
+// commitFile commits a file in the given repository and returns its path and the commit's plumbing.Hash object.
+func commitFile(t *testing.T, repository *git.Repository, repositoryPath *paths.Path) (*paths.Path, plumbing.Hash) {
+	filePath, err := paths.WriteToTempFile([]byte{}, repositoryPath, "gitutils-makeCommit-tempfile")
 	require.Nil(t, err)
 
 	worktree, err := repository.Worktree()
@@ -163,7 +223,7 @@ func makeCommit(t *testing.T, repository *git.Repository, repositoryPath *paths.
 	)
 	require.Nil(t, err)
 
-	return commit
+	return filePath, commit
 }
 
 // getTreeHash returns the plumbing.Hash object for an arbitrary Git tree object.

sync_libraries.go

@@ -37,7 +37,6 @@ import (
 	"arduino.cc/repository/internal/libraries/gitutils"
 	"arduino.cc/repository/internal/libraries/hash"
 	cc "github.com/arduino/golang-concurrent-workers"
-	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
 )
 
@@ -249,27 +248,10 @@ func syncLibraryTaggedRelease(logger *log.Logger, repo *libraries.Repository, ta
 
 	// Checkout desired tag
 	logger.Printf("Checking out tag: %s", tag.Name().Short())
-
-	repoTree, err := repo.Repository.Worktree()
-	if err != nil {
-		return err
-	}
-
-	// Annotated tags have their own hash, different from the commit hash, so the tag must be resolved before checkout
-	resolvedTag, err := gitutils.ResolveTag(tag, repo.Repository)
-	if err != nil {
-		// All unresolvable tags were already removed by gitutils.SortedCommitTags(), so there will never be an error under normal conditions.
-		panic(err)
-	}
-
-	if err = repoTree.Checkout(&git.CheckoutOptions{Hash: *resolvedTag, Force: true}); err != nil {
+	if err := gitutils.CheckoutTag(repo.Repository, tag); err != nil {
 		return fmt.Errorf("Error checking out repo: %s", err)
 	}
 
-	if err = repoTree.Clean(&git.CleanOptions{Dir: true}); err != nil {
-		return fmt.Errorf("Error cleaning repo: %s", err)
-	}
-
 	// Create library metadata from library.properties
 	library, err := libraries.GenerateLibraryFromRepo(repo)
 	if err != nil {