Skip to content

Tidy up handling of Git references. #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions internal/cachedirectory/cachedirectory.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"path"
"path/filepath"

"github.com/go-git/go-git/v5"
"github.com/pkg/errors"
)

Expand All @@ -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
}
Expand Down
20 changes: 6 additions & 14 deletions internal/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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 != "" {
Expand All @@ -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.")
Expand All @@ -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
}
}
Expand All @@ -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,
Expand Down
26 changes: 13 additions & 13 deletions internal/pull/pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
})
}

Expand Down Expand Up @@ -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",
})
}

Expand Down
31 changes: 10 additions & 21 deletions internal/push/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -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",
Expand All @@ -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
}

Expand Down
16 changes: 8 additions & 8 deletions internal/push/push_test/action-cache-initial/git/packed-refs
Original file line number Diff line number Diff line change
@@ -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

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.