Skip to content

Commit

Permalink
fix: updates refs from remotes when updating caches
Browse files Browse the repository at this point in the history 
Explcitly updates local refs from the remote.  This requires
explicitly defining the remote that the cached clone uses, which
as a knock-on effect, requires invalidating any cached clones that
do not have the correct remote set.

chore: tidy up function definitions to use grouped parameter notation
where appropriate

Fixes: Issue #190
  • Loading branch information
@nisimond
nisimond committed May 13, 2024
1 parent 25c07b4 commit 7744d42
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 45 deletions.
6 changes: 3 additions & 3 deletions internal/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ package internal

// GitManager manager responsible for Git operations.
type GitManager interface {
Clone(gitURL string, cloneDir string) error
Worktree(cloneDir string, version string, dstDir string) error
Update(cloneDir string) error
Clone(gitURL, origin, cloneDir string) error
Worktree(cloneDir, version, dstDir string) error
Update(origin, cloneDir string) error
Remote(cloneDir string) (string, error)
}
15 changes: 8 additions & 7 deletions internal/git/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,22 @@ func New(
}

// Clone the repo. This is a bare repo, with only metadata to start with.
func (g *Git) Clone(
gitURL string,
cloneDir string,
) error {
func (g *Git) Clone(gitURL, origin, cloneDir string) error {
_, err := g.execManager.RunCmd(
"git",
[]string{"clone", "--bare", "--filter=blob:none", gitURL, cloneDir},
[]string{"clone", "--bare", "--filter=blob:none", "--origin", origin, gitURL, cloneDir},
)
return err
}

// Update the repo. Fetch the current HEAD and any new tags that may have
// appeared, and update the cache.
func (g *Git) Update(cloneDir string) error {
_, err := g.execManager.RunCmdInDir("git", []string{"fetch", "--tags", "--force"}, cloneDir)
func (g *Git) Update(origin, cloneDir string) error {
_, err := g.execManager.RunCmdInDir(
"git",
[]string{"fetch", "--tags", "--force", origin, "+refs/heads/*:refs/heads/*"},
cloneDir,
)
return err
}

Expand Down
16 changes: 9 additions & 7 deletions internal/git/git_public_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type GitManagerPublicTestSuite struct {

gitURL string
gitVersion string
origin string
cloneDir string
dstDir string

Expand All @@ -68,6 +69,7 @@ func (suite *GitManagerPublicTestSuite) SetupTest() {

suite.gitURL = "https://example.com/user/repo.git"
suite.gitVersion = "abc123"
suite.origin = "gilt"
suite.cloneDir = "/cloneDir"
suite.dstDir = "/dstDir"

Expand All @@ -80,18 +82,18 @@ func (suite *GitManagerPublicTestSuite) TearDownTest() {

func (suite *GitManagerPublicTestSuite) TestCloneOk() {
suite.mockExec.EXPECT().
RunCmd("git", []string{"clone", "--bare", "--filter=blob:none", suite.gitURL, suite.cloneDir}).
RunCmd("git", []string{"clone", "--bare", "--filter=blob:none", "--origin", suite.origin, suite.gitURL, suite.cloneDir}).
Return("", nil)

err := suite.gm.Clone(suite.gitURL, suite.cloneDir)
err := suite.gm.Clone(suite.gitURL, suite.origin, suite.cloneDir)
assert.NoError(suite.T(), err)
}

func (suite *GitManagerPublicTestSuite) TestCloneReturnsError() {
errors := errors.New("tests error")
suite.mockExec.EXPECT().RunCmd(gomock.Any(), gomock.Any()).Return("", errors)

err := suite.gm.Clone(suite.gitURL, suite.cloneDir)
err := suite.gm.Clone(suite.gitURL, suite.origin, suite.cloneDir)
assert.Error(suite.T(), err)
}

Expand Down Expand Up @@ -132,18 +134,18 @@ func (suite *GitManagerPublicTestSuite) TestWorktreeErrorWhenAbsErrors() {

func (suite *GitManagerPublicTestSuite) TestUpdateOk() {
suite.mockExec.EXPECT().
RunCmdInDir("git", []string{"fetch", "--tags", "--force"}, suite.cloneDir).
RunCmdInDir("git", []string{"fetch", "--tags", "--force", suite.origin, "+refs/heads/*:refs/heads/*"}, suite.cloneDir).
Return("", nil)
err := suite.gm.Update(suite.cloneDir)
err := suite.gm.Update(suite.origin, suite.cloneDir)
assert.NoError(suite.T(), err)
}

func (suite *GitManagerPublicTestSuite) TestUpdateError() {
errors := errors.New("tests error")
suite.mockExec.EXPECT().
RunCmdInDir("git", []string{"fetch", "--tags", "--force"}, suite.cloneDir).
RunCmdInDir("git", []string{"fetch", "--tags", "--force", suite.origin, "+refs/heads/*:refs/heads/*"}, suite.cloneDir).
Return("", errors)
err := suite.gm.Update(suite.cloneDir)
err := suite.gm.Update(suite.origin, suite.cloneDir)
assert.Error(suite.T(), err)
}

Expand Down
6 changes: 4 additions & 2 deletions internal/mocks/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ package mocks

// GitManager manager responsible for Git operations.
type GitManager interface {
Clone(gitURL string, cloneDir string) error
Worktree(cloneDir string, version string, dstDir string) error
Clone(gitURL, origin, cloneDir string) error
Worktree(cloneDir, version, dstDir string) error
Update(origin, cloneDir string) error
Remote(cloneDir string) (string, error)
}
16 changes: 8 additions & 8 deletions internal/mocks/git/git_mock.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 16 additions & 12 deletions internal/repository/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package repository

import (
"errors"
"fmt"
"log/slog"
"strings"
Expand All @@ -31,6 +32,8 @@ import (
"github.com/retr0h/gilt/v2/pkg/config"
)

const ORIGIN = "gilt"

// We'll use this to normalize Git URLs as "safe" filenames
var replacer = strings.NewReplacer("/", "-", ":", "-")

Expand All @@ -55,25 +58,26 @@ func (r *Repository) Clone(
cloneDir string,
) (string, error) {
targetDir := r.appFs.Join(cloneDir, replacer.Replace(c.Git))
r.logger.Info(
"cloning",
slog.String("repository", c.Git),
slog.String("dstDir", targetDir),
)

if d, err := r.appFs.Open(targetDir); err != nil {
_ = d.Close()
if err := r.gitManager.Clone(c.Git, targetDir); err != nil {
remote, err := r.gitManager.Remote(targetDir)
if err == nil && !strings.Contains(remote, ORIGIN) {
r.logger.Info(
"'gilt' remote does not exist in clone, invalidating cache",
slog.String("dstDir", targetDir),
)
_ = r.appFs.RemoveAll(targetDir)
err = errors.New("cache does not exist")
}
if err != nil {
r.logger.Info("cloning", slog.String("repository", c.Git), slog.String("dstDir", targetDir))
if err := r.gitManager.Clone(c.Git, ORIGIN, targetDir); err != nil {
return targetDir, err
}
} else {
_ = d.Close()
r.logger.Info("clone already exists", slog.String("dstDir", targetDir))
if err := r.gitManager.Update(targetDir); err != nil {
if err := r.gitManager.Update(ORIGIN, targetDir); err != nil {
return targetDir, err
}
}

return targetDir, nil
}

Expand Down
33 changes: 27 additions & 6 deletions internal/repository/repository_public_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@ func (suite *RepositoryPublicTestSuite) TestCloneOk() {
}
targetDir := suite.appFs.Join(suite.cloneDir, suite.cacheDir)

errors := errors.New("tests error")
gomock.InOrder(
suite.mockGit.EXPECT().Clone(suite.gitURL, targetDir).Return(nil),
suite.mockGit.EXPECT().Remote(targetDir).Return("", errors),
suite.mockGit.EXPECT().Clone(suite.gitURL, repository.ORIGIN, targetDir).Return(nil),
)

_, err := repo.Clone(c, suite.cloneDir)
Expand All @@ -112,7 +114,8 @@ func (suite *RepositoryPublicTestSuite) TestCloneReturnsErrorWhenCloneErrors() {

errors := errors.New("tests error")
gomock.InOrder(
suite.mockGit.EXPECT().Clone(gomock.Any(), gomock.Any()).Return(errors),
suite.mockGit.EXPECT().Remote(gomock.Any()).Return("", errors),
suite.mockGit.EXPECT().Clone(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors),
)

_, err := repo.Clone(c, suite.cloneDir)
Expand All @@ -128,8 +131,26 @@ func (suite *RepositoryPublicTestSuite) TestCloneDoesNotCloneWhenCloneDirExists(
}
targetDir := suite.appFs.Join(suite.cloneDir, suite.cacheDir)

_ = suite.appFs.MkdirAll(targetDir, 0o755)
suite.mockGit.EXPECT().Update(targetDir).Return(nil)
suite.mockGit.EXPECT().Remote(targetDir).Return(repository.ORIGIN, nil)
suite.mockGit.EXPECT().Update(repository.ORIGIN, targetDir).Return(nil)

_, err := repo.Clone(c, suite.cloneDir)
assert.NoError(suite.T(), err)
}

func (suite *RepositoryPublicTestSuite) TestCloneInvalidatesCaches() {
repo := suite.NewRepositoryManager()

c := config.Repository{
Git: suite.gitURL,
Version: suite.gitSHA,
}
targetDir := suite.appFs.Join(suite.cloneDir, suite.cacheDir)

gomock.InOrder(
suite.mockGit.EXPECT().Remote(targetDir).Return("invalid", nil),
suite.mockGit.EXPECT().Clone(suite.gitURL, repository.ORIGIN, targetDir).Return(nil),
)

_, err := repo.Clone(c, suite.cloneDir)
assert.NoError(suite.T(), err)
Expand All @@ -144,9 +165,9 @@ func (suite *RepositoryPublicTestSuite) TestCloneUpdateCloneDirThrowsError() {
}
targetDir := suite.appFs.Join(suite.cloneDir, suite.cacheDir)

_ = suite.appFs.MkdirAll(targetDir, 0o755)
errors := errors.New("tests error")
suite.mockGit.EXPECT().Update(targetDir).Return(errors)
suite.mockGit.EXPECT().Remote(targetDir).Return(repository.ORIGIN, nil)
suite.mockGit.EXPECT().Update(repository.ORIGIN, targetDir).Return(errors)

_, err := repo.Clone(c, suite.cloneDir)
assert.Error(suite.T(), err)
Expand Down

0 comments on commit 7744d42

Please sign in to comment.