Merge pull request #39 from github/tidy-git

Tidy up handling of Git references.

Author: chrisgavin

internal/cachedirectory/cachedirectory.go

@@ -8,7 +8,6 @@ import (
 	"path"
 	"path/filepath"
 
-	"github.com/go-git/go-git/v5"
 	"github.com/pkg/errors"
 )
 
@@ -18,8 +17,6 @@ const errorCacheParentDoesNotExist = "Cannot create cache directory because its
 const errorPushNonCache = "The cache directory you have provided does not appear to be valid. Please check it exists and that you have run the `pull` command to populate it."
 const errorCacheLocked = "The cache directory is locked, likely due to a `pull` command being interrupted. Please run `pull` again to ensure all required data is downloaded."
 
-const CacheReferencePrefix = "refs/remotes/" + git.DefaultRemoteName + "/"
-
 type CacheDirectory struct {
 	path string
 }

internal/pull/pull.go

@@ -32,7 +32,7 @@ const sourceOwner = "github"
 const sourceRepository = "codeql-action"
 const sourceURL = "https://github.com/" + sourceOwner + "/" + sourceRepository + ".git"
 
-var relevantReferences = regexp.MustCompile("^refs/remotes/" + git.DefaultRemoteName + "/(heads|tags)/(main|v\\d+)$")
+var relevantReferences = regexp.MustCompile("^refs/(heads|tags)/(main|v\\d+)$")
 
 const defaultConfigurationPath = "src/defaults.json"
 
@@ -75,13 +75,10 @@ func (pullService *pullService) pullGit(fresh bool) error {
 		return errors.Wrap(err, "Error removing existing Git remote.")
 	}
 
-	_, err = localRepository.CreateRemote(&config.RemoteConfig{
+	remote := git.NewRemote(localRepository.Storer, &config.RemoteConfig{
 		Name: git.DefaultRemoteName,
 		URLs: []string{pullService.gitCloneURL},
 	})
-	if err != nil {
-		return errors.Wrap(err, "Error setting Git remote.")
-	}
 
 	var credentials *githttp.BasicAuth
 	if pullService.sourceToken != "" {
@@ -91,10 +88,6 @@ func (pullService *pullService) pullGit(fresh bool) error {
 		}
 	}
 
-	remote, err := localRepository.Remote(git.DefaultRemoteName)
-	if err != nil {
-		return errors.Wrap(err, "Error getting remote.")
-	}
 	remoteReferences, err := remote.List(&git.ListOptions{Auth: credentials})
 	if err != nil {
 		return errors.Wrap(err, "Error listing remote references.")
@@ -108,8 +101,7 @@ func (pullService *pullService) pullGit(fresh bool) error {
 			return nil
 		}
 		for _, remoteReference := range remoteReferences {
-			remoteReferenceName := strings.TrimPrefix(remoteReference.Name().String(), "refs/")
-			if cachedirectory.CacheReferencePrefix+remoteReferenceName == localReference.Name().String() {
+			if remoteReference.Name().String() == localReference.Name().String() {
 				return nil
 			}
 		}
@@ -120,11 +112,11 @@ func (pullService *pullService) pullGit(fresh bool) error {
 		return nil
 	})
 
-	err = localRepository.FetchContext(pullService.ctx, &git.FetchOptions{
+	err = remote.FetchContext(pullService.ctx, &git.FetchOptions{
 		RemoteName: git.DefaultRemoteName,
 		RefSpecs: []config.RefSpec{
-			config.RefSpec("+refs/heads/*:" + cachedirectory.CacheReferencePrefix + "heads/*"),
-			config.RefSpec("+refs/tags/*:" + cachedirectory.CacheReferencePrefix + "tags/*"),
+			config.RefSpec("+refs/heads/*:refs/heads/*"),
+			config.RefSpec("+refs/tags/*:refs/tags/*"),
 		},
 		Progress: os.Stderr,
 		Tags:     git.NoTags,

internal/pull/pull_test.go

@@ -87,14 +87,14 @@ func TestPullGitFresh(t *testing.T) {
 	err := pullService.pullGit(true)
 	require.NoError(t, err)
 	test.CheckExpectedReferencesInRepository(t, pullService.cacheDirectory.GitPath(), []string{
-		"b9f01aa2c50f49898d4c7845a66be8824499fe9d refs/remotes/origin/heads/main",
-		"26936381e619a01122ea33993e3cebc474496805 refs/remotes/origin/heads/v1",
-		"e529a54fad10a936308b2220e05f7f00757f8e7c refs/remotes/origin/heads/v3",
-		"26936381e619a01122ea33993e3cebc474496805 refs/remotes/origin/tags/v2",
+		"b9f01aa2c50f49898d4c7845a66be8824499fe9d refs/heads/main",
+		"26936381e619a01122ea33993e3cebc474496805 refs/heads/v1",
+		"e529a54fad10a936308b2220e05f7f00757f8e7c refs/heads/v3",
+		"26936381e619a01122ea33993e3cebc474496805 refs/tags/v2",
 		// It is expected that we still pull these even though they don't match the expected pattern. We just ignore them later on.
-		"bd82b85707bc13904e3526517677039d4da4a9bb refs/remotes/origin/heads/very-ignored-branch",
-		"bd82b85707bc13904e3526517677039d4da4a9bb refs/remotes/origin/tags/an-ignored-tag-too",
-		"26936381e619a01122ea33993e3cebc474496805 refs/remotes/origin/heads/a-ref-that-will-need-pruning",
+		"bd82b85707bc13904e3526517677039d4da4a9bb refs/heads/very-ignored-branch",
+		"bd82b85707bc13904e3526517677039d4da4a9bb refs/tags/an-ignored-tag-too",
+		"26936381e619a01122ea33993e3cebc474496805 refs/heads/a-ref-that-will-need-pruning",
 	})
 }
 
@@ -123,12 +123,12 @@ func TestPullGitNotFreshWithChanges(t *testing.T) {
 	err = pullService.pullGit(false)
 	require.NoError(t, err)
 	test.CheckExpectedReferencesInRepository(t, pullService.cacheDirectory.GitPath(), []string{
-		"b9f01aa2c50f49898d4c7845a66be8824499fe9d refs/remotes/origin/heads/main",
-		"26936381e619a01122ea33993e3cebc474496805 refs/remotes/origin/heads/v1",
-		"33d42021633d74bcd0bf9c95e3d3159131a5faa7 refs/remotes/origin/heads/v3", // v3 was force-pushed, and should have been force-pulled too.
-		"42d077b4730d1ba413f7bb7e0fa7c98653fb0c78 refs/remotes/origin/heads/v4", // v4 is a new branch.
-		"bd82b85707bc13904e3526517677039d4da4a9bb refs/remotes/origin/tags/an-ignored-tag-too",
-		"26936381e619a01122ea33993e3cebc474496805 refs/remotes/origin/heads/a-ref-that-will-need-pruning/because-it-now-has-this-extra-bit",
+		"b9f01aa2c50f49898d4c7845a66be8824499fe9d refs/heads/main",
+		"26936381e619a01122ea33993e3cebc474496805 refs/heads/v1",
+		"33d42021633d74bcd0bf9c95e3d3159131a5faa7 refs/heads/v3", // v3 was force-pushed, and should have been force-pulled too.
+		"42d077b4730d1ba413f7bb7e0fa7c98653fb0c78 refs/heads/v4", // v4 is a new branch.
+		"bd82b85707bc13904e3526517677039d4da4a9bb refs/tags/an-ignored-tag-too",
+		"26936381e619a01122ea33993e3cebc474496805 refs/heads/a-ref-that-will-need-pruning/because-it-now-has-this-extra-bit",
 	})
 }
 

internal/push/push.go

@@ -29,7 +29,6 @@ import (
 	"golang.org/x/oauth2"
 )
 
-const remoteName = "enterprise"
 const repositoryHomepage = "https://github.com/github/codeql-action-sync-tool/"
 
 const errorAlreadyExists = "The destination repository already exists, but it was not created with the CodeQL Action sync tool. If you are sure you want to push the CodeQL Action to it, re-run this command with the `--force` flag."
@@ -132,14 +131,10 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu
 		return errors.Wrap(err, "Error reading Git repository from cache.")
 	}
 
-	_ = gitRepository.DeleteRemote(remoteName)
-	_, err = gitRepository.CreateRemote(&config.RemoteConfig{
-		Name: remoteName,
+	remote := git.NewRemote(gitRepository.Storer, &config.RemoteConfig{
+		Name: git.DefaultRemoteName,
 		URLs: []string{remoteURL},
 	})
-	if err != nil {
-		return errors.Wrap(err, "Error adding repository remote.")
-	}
 
 	credentials := &githttp.BasicAuth{
 		Username: "x-access-token",
@@ -154,37 +149,31 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu
 		}
 		refSpecBatches = append(refSpecBatches, []config.RefSpec{})
 		for _, releasePathStat := range releasePathStats {
-			refSpecBatches[0] = append(refSpecBatches[0], config.RefSpec("+"+cachedirectory.CacheReferencePrefix+"tags/"+releasePathStat.Name()+":refs/tags/"+releasePathStat.Name()))
+			refSpecBatches[0] = append(refSpecBatches[0], config.RefSpec("+refs/tags/"+releasePathStat.Name()+":refs/tags/"+releasePathStat.Name()))
 		}
 	} else {
 		// We've got to push `main` on its own, so that it will be made the default branch if the repository has just been created. We then push everything else afterwards.
 		refSpecBatches = [][]config.RefSpec{
 			[]config.RefSpec{
-				config.RefSpec("+" + cachedirectory.CacheReferencePrefix + "heads/main:refs/heads/main"),
+				config.RefSpec("+refs/heads/main:refs/heads/main"),
 			},
 			[]config.RefSpec{
-				config.RefSpec("+" + cachedirectory.CacheReferencePrefix + "heads/*:refs/heads/*"),
-				config.RefSpec("+" + cachedirectory.CacheReferencePrefix + "tags/*:refs/tags/*"),
+				config.RefSpec("+refs/*:refs/*"),
 			},
 		}
 	}
 	for _, refSpecs := range refSpecBatches {
-		err = gitRepository.PushContext(pushService.ctx, &git.PushOptions{
-			RemoteName: remoteName,
-			RefSpecs:   refSpecs,
-			Auth:       credentials,
-			Progress:   os.Stderr,
-			Force:      true,
+		err = remote.PushContext(pushService.ctx, &git.PushOptions{
+			RefSpecs: refSpecs,
+			Auth:     credentials,
+			Progress: os.Stderr,
+			Force:    true,
 		})
 		if err != nil && errors.Cause(err) != git.NoErrAlreadyUpToDate {
 			return errors.Wrap(err, "Error pushing Action to GitHub Enterprise Server.")
 		}
 	}
 
-	err = gitRepository.DeleteRemote(remoteName)
-	if err != nil {
-		return errors.Wrap(err, "Error removing repository remote.")
-	}
 	return nil
 }
 

internal/push/push_test/action-cache-initial/git/packed-refs

@@ -1,9 +1,9 @@
 # pack-refs with: peeled fully-peeled sorted 
-b9f01aa2c50f49898d4c7845a66be8824499fe9d refs/remotes/origin/heads/main
-26936381e619a01122ea33993e3cebc474496805 refs/remotes/origin/heads/v1
-e529a54fad10a936308b2220e05f7f00757f8e7c refs/remotes/origin/heads/v3
-bd82b85707bc13904e3526517677039d4da4a9bb refs/remotes/origin/heads/very-ignored-branch
-bd82b85707bc13904e3526517677039d4da4a9bb refs/remotes/origin/tags/an-ignored-tag-too
-26936381e619a01122ea33993e3cebc474496805 refs/remotes/origin/tags/codeql-bundle-20200101
-26936381e619a01122ea33993e3cebc474496805 refs/remotes/origin/tags/codeql-bundle-20200630
-26936381e619a01122ea33993e3cebc474496805 refs/remotes/origin/tags/v2
+b9f01aa2c50f49898d4c7845a66be8824499fe9d refs/heads/main
+26936381e619a01122ea33993e3cebc474496805 refs/heads/v1
+e529a54fad10a936308b2220e05f7f00757f8e7c refs/heads/v3
+bd82b85707bc13904e3526517677039d4da4a9bb refs/heads/very-ignored-branch
+bd82b85707bc13904e3526517677039d4da4a9bb refs/tags/an-ignored-tag-too
+26936381e619a01122ea33993e3cebc474496805 refs/tags/codeql-bundle-20200101
+26936381e619a01122ea33993e3cebc474496805 refs/tags/codeql-bundle-20200630
+26936381e619a01122ea33993e3cebc474496805 refs/tags/v2