diff --git a/e2e/tests/up/git_lfs_test.go b/e2e/tests/up/git_lfs_test.go new file mode 100644 index 000000000..eabe42e18 --- /dev/null +++ b/e2e/tests/up/git_lfs_test.go @@ -0,0 +1,117 @@ +package up + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/devsy-org/devsy/pkg/command" + "github.com/devsy-org/devsy/pkg/git" +) + +const lfsPointer = "version https://git-lfs.github.com/spec/v1\n" + + "oid sha256:0000000000000000000000000000000000000000000000000000000000000\n" + + "size 4\n" + +func TestCloneLFSRepoSucceedsWithoutGitLFSBinary(t *testing.T) { + hideGitLFSFromPath(t) + simulateStaleGlobalLFSConfig(t) + + sourceDir := newLFSFixtureRepo(t) + + targetDir := filepath.Join(t.TempDir(), "clone") + if err := git.At(targetDir).Clone(context.Background(), sourceDir); err != nil { + t.Fatalf("clone: %v", err) + } + + dataPath := filepath.Join(targetDir, "data.bin") + got, err := os.ReadFile(dataPath) // #nosec G304 -- path is under a test-owned t.TempDir() + if err != nil { + t.Fatal(err) + } + if string(got) != lfsPointer { + t.Errorf("data.bin content = %q, want pointer stub %q", got, lfsPointer) + } +} + +// hideGitLFSFromPath restricts PATH to a directory containing only "git" and +// "cat", simulating a host without the git-lfs binary installed. "cat" stays +// available so the clone's filter.lfs.smudge=cat override actually runs, +// rather than relying on git's fallback for an unresolvable filter command. +func hideGitLFSFromPath(t *testing.T) { + t.Helper() + + if !command.Exists("git") { + t.Skip("git not installed") + } + + binDir := t.TempDir() + for _, name := range []string{"git", "cat"} { + path, err := exec.LookPath(name) + if err != nil { + t.Skipf("%s not found on PATH", name) + } + if err := os.Symlink(path, filepath.Join(binDir, name)); err != nil { + t.Fatal(err) + } + } + t.Setenv("PATH", binDir) +} + +// simulateStaleGlobalLFSConfig sets a fresh, isolated global gitconfig that +// registers the LFS filter driver, as if `git lfs install --global` had been +// run in the past even though the git-lfs binary is no longer present. +func simulateStaleGlobalLFSConfig(t *testing.T) { + t.Helper() + + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + runGit(t, home, "config", "--global", "user.email", "test@example.com") + runGit(t, home, "config", "--global", "user.name", "Test") + runGit(t, home, "config", "--global", "filter.lfs.process", "git-lfs filter-process") + runGit(t, home, "config", "--global", "filter.lfs.smudge", "git-lfs smudge -- %f") + runGit(t, home, "config", "--global", "filter.lfs.clean", "git-lfs clean -- %f") +} + +// newLFSFixtureRepo creates a repo declaring an LFS-tracked file, using local +// filter overrides so fixture setup doesn't need the (absent) git-lfs binary. +func newLFSFixtureRepo(t *testing.T) string { + t.Helper() + + dir := t.TempDir() + runGit(t, dir, "init", "--quiet") + runGit(t, dir, "config", "filter.lfs.clean", "cat") + runGit(t, dir, "config", "filter.lfs.smudge", "cat") + runGit(t, dir, "config", "filter.lfs.process", "") + + writeFile( + t, + filepath.Join(dir, ".gitattributes"), + "*.bin filter=lfs diff=lfs merge=lfs -text\n", + ) + writeFile(t, filepath.Join(dir, "data.bin"), lfsPointer) + + runGit(t, dir, "add", ".") + runGit(t, dir, "commit", "--quiet", "-m", "add lfs-tracked file") + return dir +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } +} + +func runGit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) // #nosec G204 -- fixed binary name, test-only + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } +} diff --git a/pkg/agent/workspace.go b/pkg/agent/workspace.go index b3e91ecb5..c535868ee 100644 --- a/pkg/agent/workspace.go +++ b/pkg/agent/workspace.go @@ -297,10 +297,14 @@ func removeGitCredentialHelper(ctx context.Context, helper, workspaceDir string) if helper == "" { return } - if err := gitcredentials.RemoveHelperFromPath( - ctx, - gitcredentials.GetLocalGitConfigPath(workspaceDir), - ); err != nil { + gitConfigPath := gitcredentials.GetLocalGitConfigPath(workspaceDir) + if _, err := os.Stat(gitConfigPath); err != nil { + if !errors.Is(err, os.ErrNotExist) { + log.Errorf("stat git config: %v", err) + } + return + } + if err := gitcredentials.RemoveHelperFromPath(ctx, gitConfigPath); err != nil { log.Errorf("remove git credential helper: %v", err) } } diff --git a/pkg/git/lfs.go b/pkg/git/lfs.go index 8c9a8f64b..0904e7912 100644 --- a/pkg/git/lfs.go +++ b/pkg/git/lfs.go @@ -13,15 +13,31 @@ import ( "github.com/devsy-org/devsy/pkg/log" ) -// lfsFilterMarker is the .gitattributes token that marks a path as LFS-tracked. var lfsFilterMarker = []byte("filter=lfs") -// smudgeSkippedForClone reports whether the clone should set GIT_LFS_SKIP_SMUDGE. -func smudgeSkippedForClone(mode LFSMode) bool { - if mode == LFSFull && !command.Exists(binGitLFS) { - log.Info("git-lfs not found, skipping LFS smudge; LFS files will be pointer stubs") +// GIT_LFS_SKIP_SMUDGE is read by the git-lfs binary itself, so it's no help +// when that binary is missing; disable the filter driver instead. +var lfsDisableFilterArgs = []string{ + flagConfig, "filter.lfs.process=", + flagConfig, "filter.lfs.smudge=cat", + flagConfig, "filter.lfs.clean=cat", +} + +func cloneArgsForLFS() []string { + if !command.Exists(binGitLFS) { + log.Info( + "git-lfs not found, disabling LFS filters for clone; LFS files will be pointer stubs", + ) + return lfsDisableFilterArgs + } + return nil +} + +func cloneEnvForLFS() []string { + if command.Exists(binGitLFS) { + return []string{"GIT_LFS_SKIP_SMUDGE=1"} } - return true + return nil } // SetupLFS configures Git LFS in the repository. diff --git a/pkg/git/lfs_test.go b/pkg/git/lfs_test.go index 14f4bfde8..c2b8116c9 100644 --- a/pkg/git/lfs_test.go +++ b/pkg/git/lfs_test.go @@ -32,14 +32,20 @@ func TestLFSModeResolution(t *testing.T) { } } -func TestSmudgeAlwaysSkippedForClone(t *testing.T) { - // The smudge filter is skipped at clone time for every mode; hydration is - // done explicitly afterward (or not at all). - for _, mode := range []LFSMode{LFSFull, LFSSetupOnly, LFSSkip} { - if !smudgeSkippedForClone(mode) { - t.Errorf("smudgeSkippedForClone(%v) = false, want true", mode) - } +func TestCloneEnvForLFS(t *testing.T) { + want := []string(nil) + if command.Exists(binGitLFS) { + want = []string{"GIT_LFS_SKIP_SMUDGE=1"} + } + assert.DeepEqual(t, want, cloneEnvForLFS()) +} + +func TestCloneArgsForLFS(t *testing.T) { + var want []string + if !command.Exists(binGitLFS) { + want = lfsDisableFilterArgs } + assert.DeepEqual(t, want, cloneArgsForLFS()) } // lfsSubcommands returns the `lfs ...` invocations a fakeRunner recorded. diff --git a/pkg/git/repo.go b/pkg/git/repo.go index d55ae65fa..656a90bb4 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -265,13 +265,14 @@ func (r *Repo) cloneWith(ctx context.Context, repository string, c cloneConfig) pw := newProgressWriter(w) env := append([]string{}, r.env...) - if smudgeSkippedForClone(c.lfsMode) { - env = append(env, "GIT_LFS_SKIP_SMUDGE=1") - } + env = append(env, cloneEnvForLFS()...) + + args := c.args(repository, r.path) + args = append(args, cloneArgsForLFS()...) _, err := r.runner.Run(ctx, RunOptions{ Env: env, - Args: c.args(repository, r.path), + Args: args, Stdout: pw, Stderr: pw, })