refactor: replace go-git with the git CLI for gitignore filtering (#1309)

Replace go-git in `exec --use-gitignore` filtering with `git ls-files`. The git CLI is already required for action clones since https://gitea.com/gitea/runner/pulls/1277, so this drops `go-git` and 14 other modules, and filtering now follows git's own ignore rules. Also stop deleting a remote action's `.gitignore` before copying it. Tracked files are never filtered, so the deletion only made copied actions differ from GitHub's action tarballs. Reviewed-on: https://gitea.com/gitea/runner/pulls/1309 Reviewed-by: bircni <[email protected]> Co-authored-by: silverwind <[email protected]>
2026-11-08 16:20:29 +00:00 · 2026-10-12 18:13:41 +00:00
1 parent ccb9afd7a5
commit 20c8b4abc4
@@ -16,10 +16,10 @@ import (
 	"path/filepath"
 	"strings"

-	git "github.com/go-git/go-git/v5"
-	"github.com/go-git/go-git/v5/plumbing/filemode"
-	"github.com/go-git/go-git/v5/plumbing/format/gitignore"
-	"github.com/go-git/go-git/v5/plumbing/format/index"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/common/git"
+
+	"golang.org/x/text/unicode/norm"
 )

 type Handler interface {
@@ -94,73 +94,62 @@ func (cc *CopyCollector) WriteFile(fpath string, fi fs.FileInfo, linkName string
 }

 type FileCollector struct {
-	Ignorer    gitignore.Matcher
-	SrcPath    string
-	SrcPrefix  string
-	SkipGitDir bool
-	Handler    Handler
+	SrcPath      string
+	SrcPrefix    string
+	SkipGitDir   bool
+	UseGitIgnore bool
+	Handler      Handler
 }

-func openGitIndex(path string) (*index.Index, error) {
-	repo, err := git.PlainOpen(path)
-	if err != nil {
-		return nil, err
+// foldPath compares git's spelling of a path with the filesystem's, which may differ in case or Unicode normalization.
+func foldPath(name string) string {
+	return strings.ToLower(norm.NFC.String(name))
+}
+
+func (fc *FileCollector) gitFiles(ctx context.Context) map[string]bool {
+	if !fc.UseGitIgnore {
+		return nil
 	}
-	return repo.Storer.Index()
+	files, err := git.ListFiles(ctx, fc.SrcPath)
+	if err != nil {
+		common.Logger(ctx).Debugf("Copying %s without applying .gitignore: %v", fc.SrcPath, err)
+		return nil
+	}
+	keep := map[string]bool{".": true}
+	for _, file := range files {
+		for file = foldPath(file); !keep[file]; file = path.Dir(file) {
+			keep[file] = true
+		}
+	}
+	return keep
 }

-func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []string) filepath.WalkFunc {
-	i, _ := openGitIndex(path.Join(fc.SrcPath, path.Join(submodulePath...)))
+func (fc *FileCollector) CollectFiles(ctx context.Context) filepath.WalkFunc {
+	keep := fc.gitFiles(ctx)
 	return func(file string, fi os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
-		if ctx != nil && ctx.Err() != nil {
+		if ctx.Err() != nil {
 			return errors.New("copy cancelled")
 		}
-
-		sansPrefix := strings.TrimPrefix(file, fc.SrcPrefix)
-		split := strings.Split(sansPrefix, string(filepath.Separator))
-		// The root folders should be skipped, submodules only have the last path component set to "." by filepath.Walk
-		if fi.IsDir() && len(split) > 0 && split[len(split)-1] == "." {
-			return nil
-		}
-		if fc.SkipGitDir && split[len(split)-1] == ".git" {
+		if fc.SkipGitDir && fi.Name() == ".git" {
 			if fi.IsDir() {
 				return filepath.SkipDir
 			}
 			return nil
 		}
-		var entry *index.Entry
-		if i != nil {
-			entry, err = i.Entry(strings.Join(split[len(submodulePath):], "/"))
-		} else {
-			err = index.ErrEntryNotFound
-		}
-		if err != nil && fc.Ignorer != nil && fc.Ignorer.Match(split, fi.IsDir()) {
-			if fi.IsDir() {
-				if i != nil {
-					ms, err := i.Glob(strings.Join(append(split[len(submodulePath):], "**"), "/"))
-					if err != nil || len(ms) == 0 {
-						return filepath.SkipDir
-					}
-				} else {
+		if keep != nil {
+			rel, _ := filepath.Rel(fc.SrcPath, file)
+			if rel = filepath.ToSlash(rel); !keep[foldPath(rel)] && !strings.Contains("/"+rel+"/", "/.git/") {
+				if fi.IsDir() {
 					return filepath.SkipDir
 				}
-			} else {
 				return nil
 			}
 		}
-		if err == nil && entry.Mode == filemode.Submodule {
-			err = filepath.Walk(file, fc.CollectFiles(ctx, split))
-			if err != nil {
-				return err
-			}
-			return filepath.SkipDir
-		}
-		path := filepath.ToSlash(sansPrefix)
+		path := filepath.ToSlash(strings.TrimPrefix(file, fc.SrcPrefix))

-		// return on non-regular files (thanks to [kumo](https://medium.com/@komuw/just-like-you-did-fbdd7df829d3) for this suggested update)
 		if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
 			linkName, err := os.Readlink(file)
 			if err != nil {
@@ -171,17 +160,12 @@ func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []strin
 			return nil
 		}

-		// open file
 		f, err := os.Open(file)
 		if err != nil {
 			return err
 		}
 		defer f.Close()
-
-		if ctx != nil {
-			stop := context.AfterFunc(ctx, func() { _ = f.Close() })
-			defer stop()
-		}
+		defer context.AfterFunc(ctx, func() { _ = f.Close() })()

 		return fc.Handler.WriteFile(path, fi, "", f)
 	}
@@ -9,106 +9,78 @@ import (
 	"bytes"
 	"context"
 	"io"
+	"maps"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"testing"

-	git "github.com/go-git/go-git/v5"
-	"github.com/go-git/go-git/v5/plumbing/format/gitignore"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )

-func TestIgnoredTrackedfile(t *testing.T) {
+func gitCmd(t *testing.T, args ...string) {
+	t.Helper()
+	out, err := exec.Command("git", args...).CombinedOutput()
+	require.NoError(t, err, string(out))
+}
+
+func TestCollectFilesKeepsWhatGitTracksOrDoesNotIgnore(t *testing.T) {
 	repoDir := filepath.Join(t.TempDir(), "mygitrepo")
-	repo, err := git.PlainInit(repoDir, false)
-	require.NoError(t, err)
-	require.NoError(t, os.WriteFile(filepath.Join(repoDir, ".gitignore"), []byte(".*\n"), 0o644))
-	require.NoError(t, os.WriteFile(filepath.Join(repoDir, ".env"), []byte("test=val1\n"), 0o644))
-	worktree, err := repo.Worktree()
-	require.NoError(t, err)
-	_, err = worktree.Add(".gitignore")
-	require.NoError(t, err)
+	for _, dir := range []string{repoDir, filepath.Join(repoDir, "sub"), filepath.Join(repoDir, "nested")} {
+		gitCmd(t, "init", "-q", dir)
+	}
+	for name, body := range map[string]string{
+		".gitignore":     ".*\nign/\n",
+		".env":           "",
+		"Renamed":        "",
+		"ign/tracked":    "",
+		"ign/untracked":  "",
+		"sub/.gitignore": "dropped\n",
+		"sub/dropped":    "",
+		"sub/kept":       "",
+		"nested/kept":    "",
+	} {
+		require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(repoDir, name)), 0o755))
+		require.NoError(t, os.WriteFile(filepath.Join(repoDir, name), []byte(body), 0o644))
+	}
+	require.NoError(t, os.Mkdir(filepath.Join(repoDir, "uninitialized"), 0o755))
+	gitCmd(t, "-C", repoDir, "add", "-f", ".gitignore", "Renamed", "ign/tracked")
+	require.NoError(t, os.Rename(filepath.Join(repoDir, "Renamed"), filepath.Join(repoDir, "renamed")))
+	for _, gitlink := range []string{"sub", "uninitialized"} {
+		gitCmd(t, "-C", repoDir, "update-index", "--add", "--cacheinfo", "160000,4b825dc642cb6eb9a060e54bf8d69288fbee4904,"+gitlink)
+	}
+	expected := map[string]string{".gitignore": "", "ign/tracked": "", "nested/kept": "", "renamed": "", "sub/.gitignore": "", "sub/kept": ""}
+	if runtime.GOOS != "windows" {
+		require.NoError(t, os.Symlink(".env", filepath.Join(repoDir, "test.env")))
+		expected["test.env"] = ".env"
+	}

 	var archive bytes.Buffer
 	tw := tar.NewWriter(&archive)
-	patterns, err := gitignore.ReadPatterns(worktree.Filesystem, nil)
-	require.NoError(t, err)
-	ignorer := gitignore.NewMatcher(patterns)
-	fc := &FileCollector{
-		Ignorer:   ignorer,
-		SrcPath:   repoDir,
-		SrcPrefix: repoDir + string(filepath.Separator),
-		Handler: &TarCollector{
-			TarWriter: tw,
-		},
-	}
-	err = filepath.Walk(repoDir, fc.CollectFiles(context.Background(), nil))
-	assert.NoError(t, err, "successfully collect files")
+	fc := &FileCollector{SrcPath: repoDir, SrcPrefix: repoDir + string(filepath.Separator), UseGitIgnore: true, Handler: &TarCollector{TarWriter: tw}}
+	require.NoError(t, filepath.Walk(repoDir, fc.CollectFiles(t.Context())))
 	require.NoError(t, tw.Close())
+
+	files := map[string]string{}
 	tr := tar.NewReader(&archive)
-	h, err := tr.Next()
-	assert.NoError(t, err, "tar must not be empty") //nolint:testifylint // pre-existing issue from nektos/act
-	assert.Equal(t, ".gitignore", h.Name)
-	_, err = tr.Next()
-	assert.ErrorIs(t, err, io.EOF, "tar must only contain one element")
+	for header, err := tr.Next(); err != io.EOF; header, err = tr.Next() {
+		require.NoError(t, err)
+		files[header.Name] = header.Linkname
+	}
+	assert.Contains(t, files, ".git/HEAD")
+	maps.DeleteFunc(files, func(name, _ string) bool { return strings.Contains("/"+name, "/.git/") })
+	assert.Equal(t, expected, files)
 }

-func TestSymlinks(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("creating symlinks requires elevated privileges on Windows")
-	}
-	repoDir := filepath.Join(t.TempDir(), "mygitrepo")
-	repo, err := git.PlainInit(repoDir, false)
-	require.NoError(t, err)
-	require.NoError(t, os.WriteFile(filepath.Join(repoDir, ".env"), []byte("test=val1\n"), 0o644))
-	require.NoError(t, os.Symlink(".env", filepath.Join(repoDir, "test.env")))
-	worktree, err := repo.Worktree()
-	require.NoError(t, err)
-	_, err = worktree.Add("test.env")
-	require.NoError(t, err)
-
-	var archive bytes.Buffer
-	tw := tar.NewWriter(&archive)
-	fc := &FileCollector{
-		SrcPath:   repoDir,
-		SrcPrefix: repoDir + string(filepath.Separator),
-		Handler: &TarCollector{
-			TarWriter: tw,
-		},
-	}
-	err = filepath.Walk(repoDir, fc.CollectFiles(context.Background(), nil))
-	assert.NoError(t, err, "successfully collect files")
-	require.NoError(t, tw.Close())
-	tr := tar.NewReader(&archive)
-	h, err := tr.Next()
-	files := map[string]tar.Header{}
-	for err == nil {
-		files[h.Name] = *h
-		h, err = tr.Next()
-	}
-
-	assert.Equal(t, ".env", files[".env"].Name)
-	assert.Equal(t, "test.env", files["test.env"].Name)
-	assert.Equal(t, ".env", files["test.env"].Linkname)
-	assert.ErrorIs(t, err, io.EOF, "tar must be read cleanly to EOF")
-}
-
-// Regression for https://gitea.com/gitea/runner/issues/876 and /941:
-// re-copying an action directory must overwrite a pre-existing read-only
-// file (e.g. a git pack .idx at mode 0444) instead of failing with EACCES
-// on macOS or "Access is denied" on Windows.
 func TestCopyCollectorWriteFileOverwritesReadOnlyFile(t *testing.T) {
 	dst := t.TempDir()
 	target := filepath.Join(dst, "sub", "pack.idx")
 	require.NoError(t, os.MkdirAll(filepath.Dir(target), 0o755))
 	require.NoError(t, os.WriteFile(target, []byte("old"), 0o444))
-
-	src := filepath.Join(t.TempDir(), "pack.idx")
-	require.NoError(t, os.WriteFile(src, []byte("new"), 0o444))
-	fi, err := os.Stat(src)
+	fi, err := os.Stat(target)
 	require.NoError(t, err)

 	cc := &CopyCollector{DstDir: dst}
@@ -119,8 +91,6 @@ func TestCopyCollectorWriteFileOverwritesReadOnlyFile(t *testing.T) {
 	assert.Equal(t, "new", string(got))
 }

-// Without the destination removal, os.Symlink fails with EEXIST when the
-// path already holds a regular file from an earlier copy of the action.
 func TestCopyCollectorWriteFileOverwritesFileWithSymlink(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("creating symlinks requires elevated privileges on Windows")
@@ -129,11 +99,8 @@ func TestCopyCollectorWriteFileOverwritesFileWithSymlink(t *testing.T) {
 	target := filepath.Join(dst, "link")
 	require.NoError(t, os.WriteFile(target, []byte("stale"), 0o644))

-	fi, err := os.Lstat(target)
-	require.NoError(t, err)
-
 	cc := &CopyCollector{DstDir: dst}
-	require.NoError(t, cc.WriteFile("link", fi, "target", nil))
+	require.NoError(t, cc.WriteFile("link", nil, "target", nil))

 	resolved, err := os.Readlink(target)
 	require.NoError(t, err)
@@ -143,7 +110,7 @@ func TestCopyCollectorWriteFileOverwritesFileWithSymlink(t *testing.T) {
 func TestFileCollectorCancellationAndWalkError(t *testing.T) {
 	ctx, cancel := context.WithCancel(t.Context())
 	cancel()
-	walk := (&FileCollector{}).CollectFiles(ctx, nil)
+	walk := (&FileCollector{}).CollectFiles(ctx)

 	err := walk("file", nil, nil)
 	require.EqualError(t, err, "copy cancelled")