diff --git a/internal/pkg/datasources/datasource_git.go b/internal/pkg/datasources/datasource_git.go index 066f1a6..5f053f1 100644 --- a/internal/pkg/datasources/datasource_git.go +++ b/internal/pkg/datasources/datasource_git.go @@ -287,6 +287,44 @@ func (d *GitLoader) stashPendingChanges(logger *logrus.Entry, gitDir string) err return utils.ExecuteCommand(logger, cmd, d.secrets()) } +// hasInitialCommit reports whether gitDir has a commit checked out. An unborn +// repository has a .git directory but cannot run commands such as stash or +// reset, because HEAD does not resolve to a commit yet. +func (d *GitLoader) hasInitialCommit(gitDir string) bool { + cmd := exec.Command("git", "rev-parse", "--verify", "HEAD") + cmd.Dir = gitDir + cmd.Env = os.Environ() + + return cmd.Run() == nil +} + +// removeStaleHEADLock removes the lock that can be left behind when a previous +// data-loader run is interrupted while Git updates HEAD. It is cleaned before +// this loader starts its Git sync sequence. +func (d *GitLoader) removeStaleHEADLock(gitDir string) (bool, error) { + headLockPath := filepath.Join(gitDir, ".git", "HEAD.lock") + info, err := os.Lstat(headLockPath) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + + return false, fmt.Errorf("failed to inspect stale Git HEAD lock %s, err: %w", headLockPath, err) + } + if !info.Mode().IsRegular() { + return false, fmt.Errorf("refusing to remove non-regular Git HEAD lock %s", headLockPath) + } + + if err := os.Remove(headLockPath); err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, fmt.Errorf("failed to remove stale Git HEAD lock %s, err: %w", headLockPath, err) + } + + return true, nil +} + func (d *GitLoader) resetHardToTargetBranch(logger *logrus.Entry, gitDir string, branch string) error { logger = logger.WithFields(logrus.Fields{ "workingDirectory": gitDir, @@ -573,30 +611,42 @@ func (d *GitLoader) syncWithClone(logger *logrus.Entry, fromURI string, alteredF } func (d *GitLoader) syncWithPull(logger *logrus.Entry, _ string, alteredFromURI string, _ string, finalizedGitDir string) error { - err := d.configBeforeOperations(logger, finalizedGitDir) + removed, err := d.removeStaleHEADLock(finalizedGitDir) if err != nil { return err } - - err = d.updateIndex(logger, finalizedGitDir) - if err != nil { - // update index is ignorable - logger.Warnf("failed to update index for git repository, err: %s", err) + if removed { + logger.Warn("removed stale .git/HEAD.lock left by a previous interrupted Git operation") } - err = d.addAll(logger, finalizedGitDir) + err = d.configBeforeOperations(logger, finalizedGitDir) if err != nil { return err } - err = d.stashPendingChanges(logger, finalizedGitDir) - if err != nil { - return err - } + if d.hasInitialCommit(finalizedGitDir) { + err = d.updateIndex(logger, finalizedGitDir) + if err != nil { + // update index is ignorable + logger.Warnf("failed to update index for git repository, err: %s", err) + } - err = d.resetHardToTargetBranch(logger, finalizedGitDir, d.gitOptions.Branch) - if err != nil { - return err + err = d.addAll(logger, finalizedGitDir) + if err != nil { + return err + } + + err = d.stashPendingChanges(logger, finalizedGitDir) + if err != nil { + return err + } + + err = d.resetHardToTargetBranch(logger, finalizedGitDir, d.gitOptions.Branch) + if err != nil { + return err + } + } else { + logger.Debug("git repository has no initial commit; skipping stash and reset before initial pull") } pullRemoteName := fmt.Sprintf("dataset-pull-remote-%s", utils.RandomHashString(8)) diff --git a/internal/pkg/datasources/datasource_git_test.go b/internal/pkg/datasources/datasource_git_test.go index e92c62f..83e0f71 100644 --- a/internal/pkg/datasources/datasource_git_test.go +++ b/internal/pkg/datasources/datasource_git_test.go @@ -3,13 +3,121 @@ package datasources import ( "fmt" "os" + "os/exec" + "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestGitLoaderSyncFromUnbornRepository(t *testing.T) { + remoteDir, branch := createBareGitRemote(t) + + for _, testCase := range []struct { + name string + withStaleHEADLock bool + }{ + {name: "pulls initial commit"}, + {name: "removes stale HEAD lock and pulls initial commit", withStaleHEADLock: true}, + } { + t.Run(testCase.name, func(t *testing.T) { + rootDir := t.TempDir() + t.Setenv("GIT_CONFIG_GLOBAL", filepath.Join(rootDir, "gitconfig")) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + + repoDir := filepath.Join(rootDir, "repository") + require.NoError(t, os.Mkdir(repoDir, 0755)) + runGit(t, repoDir, "init") + + headLockPath := filepath.Join(repoDir, ".git", "HEAD.lock") + if testCase.withStaleHEADLock { + require.NoError(t, os.WriteFile(headLockPath, nil, 0600)) + } + + loader, err := NewGitLoader(map[string]string{"branch": branch}, Options{Root: rootDir}, Secrets{}) + require.NoError(t, err) + require.NoError(t, loader.Sync(remoteDir, "repository")) + + assert.NotEmpty(t, strings.TrimSpace(runGit(t, repoDir, "rev-parse", "--verify", "HEAD"))) + assert.Equal(t, "initial content\n", string(requireFileContents(t, filepath.Join(repoDir, "README.md")))) + if testCase.withStaleHEADLock { + assert.NoFileExists(t, headLockPath) + } + }) + } +} + +func createBareGitRemote(t *testing.T) (string, string) { + t.Helper() + + rootDir := t.TempDir() + sourceDir := filepath.Join(rootDir, "source") + remoteDir := filepath.Join(rootDir, "remote.git") + require.NoError(t, os.Mkdir(sourceDir, 0755)) + runGit(t, sourceDir, "init") + runGit(t, sourceDir, "config", "user.name", "test user") + runGit(t, sourceDir, "config", "user.email", "test@example.com") + require.NoError(t, os.WriteFile(filepath.Join(sourceDir, "README.md"), []byte("initial content\n"), 0600)) + runGit(t, sourceDir, "add", "README.md") + runGit(t, sourceDir, "commit", "-m", "initial commit") + branch := strings.TrimSpace(runGit(t, sourceDir, "branch", "--show-current")) + + runGit(t, rootDir, "init", "--bare", remoteDir) + runGit(t, sourceDir, "remote", "add", "origin", remoteDir) + runGit(t, sourceDir, "push", "origin", branch) + + return remoteDir, branch +} + +func runGit(t *testing.T, directory string, args ...string) string { + t.Helper() + + command := exec.Command("git", args...) + command.Dir = directory + output, err := command.CombinedOutput() + require.NoErrorf(t, err, "git %s failed:\n%s", strings.Join(args, " "), output) + + return string(output) +} + +func requireFileContents(t *testing.T, filePath string) []byte { + t.Helper() + + contents, err := os.ReadFile(filePath) + require.NoError(t, err) + return contents +} func TestGitLoader(t *testing.T) { + t.Run("removes stale HEAD lock before pull", func(t *testing.T) { + git, err := NewGitLoader(map[string]string{}, Options{}, Secrets{}) + require.NoError(t, err) + + gitDir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(gitDir, ".git"), 0755)) + headLockPath := filepath.Join(gitDir, ".git", "HEAD.lock") + require.NoError(t, os.WriteFile(headLockPath, nil, 0600)) + + removed, err := git.removeStaleHEADLock(gitDir) + require.NoError(t, err) + assert.True(t, removed) + assert.NoFileExists(t, headLockPath) + }) + + t.Run("does not remove non-regular HEAD lock", func(t *testing.T) { + git, err := NewGitLoader(map[string]string{}, Options{}, Secrets{}) + require.NoError(t, err) + + gitDir := t.TempDir() + headLockPath := filepath.Join(gitDir, ".git", "HEAD.lock") + require.NoError(t, os.MkdirAll(headLockPath, 0755)) + + removed, err := git.removeStaleHEADLock(gitDir) + assert.False(t, removed) + require.ErrorContains(t, err, "refusing to remove non-regular Git HEAD lock") + assert.DirExists(t, headLockPath) + }) t.Run("clone", func(t *testing.T) { git, err := NewGitLoader(map[string]string{ "branch": "master", @@ -130,6 +238,11 @@ func TestGitLoader(t *testing.T) { stderr: "", exit: 0, }, + { + stdout: "head", + stderr: "", + exit: 0, + }, { stdout: "update", stderr: "", @@ -176,12 +289,13 @@ func TestGitLoader(t *testing.T) { assert.NoError(t, err) }) bbs := fakeGit.GetAllInputs() - assert.Contains(t, string(bbs[5]), "remote add") - assert.Contains(t, string(bbs[6]), "pull") - bbs[5] = []byte{} + assert.Contains(t, string(bbs[6]), "remote add") + assert.Contains(t, string(bbs[7]), "pull") bbs[6] = []byte{} + bbs[7] = []byte{} assert.Equal(t, [][]byte{ []byte("config --global safe.directory *\n"), + []byte("rev-parse --verify HEAD\n"), []byte("update-index --refresh\n"), []byte("add -u\n"), []byte("stash\n"), @@ -202,6 +316,11 @@ func TestGitLoader(t *testing.T) { stderr: "", exit: 0, }, + { + stdout: "head", + stderr: "", + exit: 0, + }, { stdout: "update", stderr: "", @@ -253,12 +372,13 @@ func TestGitLoader(t *testing.T) { assert.NoError(t, err) }) bbs := fakeGit.GetAllInputs() - assert.Contains(t, string(bbs[5]), "remote add") - bbs[5] = []byte{} - assert.Contains(t, string(bbs[7]), "branch1") - bbs[7] = []byte{} + assert.Contains(t, string(bbs[6]), "remote add") + bbs[6] = []byte{} + assert.Contains(t, string(bbs[8]), "branch1") + bbs[8] = []byte{} assert.Equal(t, [][]byte{ []byte("config --global safe.directory *\n"), + []byte("rev-parse --verify HEAD\n"), []byte("update-index --refresh\n"), []byte("add -u\n"), []byte("stash\n"), diff --git a/internal/pkg/datasources/fake.go b/internal/pkg/datasources/fake.go index 53e4ee9..d07af93 100644 --- a/internal/pkg/datasources/fake.go +++ b/internal/pkg/datasources/fake.go @@ -37,8 +37,7 @@ func (f *fakeCommand) Inject() error { require.NoError(f.t, os.WriteFile(path.Join(f.path, fmt.Sprintf(".%s_exit_%d", f.cmd, i)), []byte(strconv.Itoa(o.exit)), 0600)) } t, err := template.New("fakeCommand").Parse( - ` -#!/usr/bin/env bash + `#!/usr/bin/env bash index=0 if [ -f "{{.path}}/.{{.cmd}}_index" ]; then index=$(cat "{{.path}}/.{{.cmd}}_index")