[v7.0/forgejo] [REFACTOR] git attribute: test proper cancellation and unify nul-byte reader (#2939)

**Backport:** https://codeberg.org/forgejo/forgejo/pulls/2906

Following #2763 (refactor of git check-attr)
and #2866 (wrong log.Error format in check-attr)

- refactors the `nul-byte` reader to be used in both the streaming and one-off cases.
- add test for some failure cases
- don't log the error returned by `cmd.Run`, but return it to the `CheckPath` caller (which can then decide what to do with it).

This should solve the following flaky `log.Error` (or at least move it to the caller, instead of being inside a random goroutine):

https://codeberg.org/forgejo/forgejo/actions/runs/9541/jobs/5#jobstep-7-839

> FATAL ERROR: log.Error has been called: 2024/03/28 14:30:33 ...it/repo_attribute.go:313:func2() [E] Unable to open checker for 3fa2f829675543ecfc16b2891aebe8bf0608a8f4. Error: failed to run attr-check. Error: exit status 128
        Stderr: fatal: this operation must be run in a work tree

Co-authored-by: oliverpool <git@olivier.pfad.fr>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2939
Reviewed-by: oliverpool <oliverpool@noreply.codeberg.org>
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>

modules/git/repo_attribute.go

@@ -4,18 +4,76 @@
 package git
 
 import (
+	"bufio"
 	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"os"
 	"strings"
+	"sync/atomic"
 
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/optional"
 )
 
 var LinguistAttributes = []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language", "linguist-documentation", "linguist-detectable"}
 
+// newCheckAttrStdoutReader parses the nul-byte separated output of git check-attr on each call of
+// the returned function. The first reading error will stop the reading and be returned on all
+// subsequent calls.
+func newCheckAttrStdoutReader(r io.Reader, count int) func() (map[string]GitAttribute, error) {
+	scanner := bufio.NewScanner(r)
+
+	// adapted from bufio.ScanLines to split on nul-byte \x00
+	scanner.Split(func(data []byte, atEOF bool) (advance int, token []byte, err error) {
+		if atEOF && len(data) == 0 {
+			return 0, nil, nil
+		}
+		if i := bytes.IndexByte(data, '\x00'); i >= 0 {
+			// We have a full nul-terminated line.
+			return i + 1, data[0:i], nil
+		}
+		// If we're at EOF, we have a final, non-terminated line. Return it.
+		if atEOF {
+			return len(data), data, nil
+		}
+		// Request more data.
+		return 0, nil, nil
+	})
+
+	var err error
+	nextText := func() string {
+		if err != nil {
+			return ""
+		}
+		if !scanner.Scan() {
+			err = scanner.Err()
+			if err == nil {
+				err = io.ErrUnexpectedEOF
+			}
+			return ""
+		}
+		return scanner.Text()
+	}
+	nextAttribute := func() (string, GitAttribute, error) {
+		nextText() // discard filename
+		key := nextText()
+		value := GitAttribute(nextText())
+		return key, value, err
+	}
+	return func() (map[string]GitAttribute, error) {
+		values := make(map[string]GitAttribute, count)
+		for range count {
+			k, v, err := nextAttribute()
+			if err != nil {
+				return values, err
+			}
+			values[k] = v
+		}
+		return values, scanner.Err()
+	}
+}
+
 // GitAttribute exposes an attribute from the .gitattribute file
 type GitAttribute string //nolint:revive
 
@@ -54,29 +112,15 @@ func (ca GitAttribute) Bool() optional.Option[bool] {
 	return optional.None[bool]()
 }
 
-// GitAttributeFirst returns the first specified attribute
-//
-// If treeish is empty, the gitattribute will be read from the current repo (which MUST be a working directory and NOT bare).
-func (repo *Repository) GitAttributeFirst(treeish, filename string, attributes ...string) (GitAttribute, error) {
-	values, err := repo.GitAttributes(treeish, filename, attributes...)
-	if err != nil {
-		return "", err
-	}
-	for _, a := range attributes {
-		if values[a].IsSpecified() {
-			return values[a], nil
-		}
-	}
-	return "", nil
-}
-
+// gitCheckAttrCommand prepares the "git check-attr" command for later use as one-shot or streaming
+// instanciation.
 func (repo *Repository) gitCheckAttrCommand(treeish string, attributes ...string) (*Command, *RunOpts, context.CancelFunc, error) {
 	if len(attributes) == 0 {
 		return nil, nil, nil, fmt.Errorf("no provided attributes to check-attr")
 	}
 
 	env := os.Environ()
-	var deleteTemporaryFile context.CancelFunc
+	var removeTempFiles context.CancelFunc = func() {}
 
 	// git < 2.40 cannot run check-attr on bare repo, but needs INDEX + WORK_TREE
 	hasIndex := treeish == ""
@@ -85,7 +129,7 @@ func (repo *Repository) gitCheckAttrCommand(treeish string, attributes ...string
 		if err != nil {
 			return nil, nil, nil, err
 		}
-		deleteTemporaryFile = cancel
+		removeTempFiles = cancel
 
 		env = append(env, "GIT_INDEX_FILE="+indexFilename, "GIT_WORK_TREE="+worktree)
 
@@ -94,16 +138,8 @@ func (repo *Repository) gitCheckAttrCommand(treeish string, attributes ...string
 		// clear treeish to read from provided index/work_tree
 		treeish = ""
 	}
-	ctx, cancel := context.WithCancel(repo.Ctx)
-	if deleteTemporaryFile != nil {
-		ctxCancel := cancel
-		cancel = func() {
-			ctxCancel()
-			deleteTemporaryFile()
-		}
-	}
 
-	cmd := NewCommand(ctx, "check-attr", "-z")
+	cmd := NewCommand(repo.Ctx, "check-attr", "-z")
 
 	if hasIndex {
 		cmd.AddArguments("--cached")
@@ -126,18 +162,34 @@ func (repo *Repository) gitCheckAttrCommand(treeish string, attributes ...string
 	return cmd, &RunOpts{
 		Env: env,
 		Dir: repo.Path,
-	}, cancel, nil
+	}, removeTempFiles, nil
 }
 
-// GitAttributes returns gitattribute.
+// GitAttributeFirst returns the first specified attribute of the given filename.
+//
+// If treeish is empty, the gitattribute will be read from the current repo (which MUST be a working directory and NOT bare).
+func (repo *Repository) GitAttributeFirst(treeish, filename string, attributes ...string) (GitAttribute, error) {
+	values, err := repo.GitAttributes(treeish, filename, attributes...)
+	if err != nil {
+		return "", err
+	}
+	for _, a := range attributes {
+		if values[a].IsSpecified() {
+			return values[a], nil
+		}
+	}
+	return "", nil
+}
+
+// GitAttributes returns the gitattribute of the given filename.
 //
 // If treeish is empty, the gitattribute will be read from the current repo (which MUST be a working directory and NOT bare).
 func (repo *Repository) GitAttributes(treeish, filename string, attributes ...string) (map[string]GitAttribute, error) {
-	cmd, runOpts, cancel, err := repo.gitCheckAttrCommand(treeish, attributes...)
+	cmd, runOpts, removeTempFiles, err := repo.gitCheckAttrCommand(treeish, attributes...)
 	if err != nil {
 		return nil, err
 	}
-	defer cancel()
+	defer removeTempFiles()
 
 	stdOut := new(bytes.Buffer)
 	runOpts.Stdout = stdOut
@@ -151,163 +203,84 @@ func (repo *Repository) GitAttributes(treeish, filename string, attributes ...st
 		return nil, fmt.Errorf("failed to run check-attr: %w\n%s\n%s", err, stdOut.String(), stdErr.String())
 	}
 
-	// FIXME: This is incorrect on versions < 1.8.5
-	fields := bytes.Split(stdOut.Bytes(), []byte{'\000'})
-
-	if len(fields)%3 != 1 {
-		return nil, fmt.Errorf("wrong number of fields in return from check-attr")
-	}
-
-	values := make(map[string]GitAttribute, len(attributes))
-	for ; len(fields) >= 3; fields = fields[3:] {
-		// filename := string(fields[0])
-		attribute := string(fields[1])
-		value := string(fields[2])
-		values[attribute] = GitAttribute(value)
-	}
-	return values, nil
+	return newCheckAttrStdoutReader(stdOut, len(attributes))()
 }
 
-type attributeTriple struct {
-	Filename  string
-	Attribute string
-	Value     string
-}
-
-type nulSeparatedAttributeWriter struct {
-	tmp        []byte
-	attributes chan attributeTriple
-	closed     chan struct{}
-	working    attributeTriple
-	pos        int
-}
-
-func (wr *nulSeparatedAttributeWriter) Write(p []byte) (n int, err error) {
-	l, read := len(p), 0
-
-	nulIdx := bytes.IndexByte(p, '\x00')
-	for nulIdx >= 0 {
-		wr.tmp = append(wr.tmp, p[:nulIdx]...)
-		switch wr.pos {
-		case 0:
-			wr.working = attributeTriple{
-				Filename: string(wr.tmp),
-			}
-		case 1:
-			wr.working.Attribute = string(wr.tmp)
-		case 2:
-			wr.working.Value = string(wr.tmp)
-		}
-		wr.tmp = wr.tmp[:0]
-		wr.pos++
-		if wr.pos > 2 {
-			wr.attributes <- wr.working
-			wr.pos = 0
-		}
-		read += nulIdx + 1
-		if l > read {
-			p = p[nulIdx+1:]
-			nulIdx = bytes.IndexByte(p, '\x00')
-		} else {
-			return l, nil
-		}
-	}
-	wr.tmp = append(wr.tmp, p...)
-	return len(p), nil
-}
-
-func (wr *nulSeparatedAttributeWriter) Close() error {
-	select {
-	case <-wr.closed:
-		return nil
-	default:
-	}
-	close(wr.attributes)
-	close(wr.closed)
-	return nil
-}
-
-// GitAttributeChecker creates an AttributeChecker for the given repository and provided commit ID.
+// GitAttributeChecker creates an AttributeChecker for the given repository and provided commit ID
+// to retrieve the attributes of multiple files. The AttributeChecker must be closed after use.
 //
 // If treeish is empty, the gitattribute will be read from the current repo (which MUST be a working directory and NOT bare).
 func (repo *Repository) GitAttributeChecker(treeish string, attributes ...string) (AttributeChecker, error) {
-	cmd, runOpts, cancel, err := repo.gitCheckAttrCommand(treeish, attributes...)
+	cmd, runOpts, removeTempFiles, err := repo.gitCheckAttrCommand(treeish, attributes...)
 	if err != nil {
 		return AttributeChecker{}, err
 	}
 
-	ac := AttributeChecker{
-		attributeNumber: len(attributes),
-		ctx:             cmd.parentContext,
-		cancel:          cancel, // will be cancelled on Close
-	}
-
-	stdinReader, stdinWriter, err := os.Pipe()
-	if err != nil {
-		ac.cancel()
-		return AttributeChecker{}, err
-	}
-	ac.stdinWriter = stdinWriter // will be closed on Close
-
-	lw := new(nulSeparatedAttributeWriter)
-	lw.attributes = make(chan attributeTriple, len(attributes))
-	lw.closed = make(chan struct{})
-	ac.attributesCh = lw.attributes
-
 	cmd.AddArguments("--stdin")
+
+	// os.Pipe is needed (and not io.Pipe), otherwise cmd.Wait will wait for the stdinReader
+	// to be closed before returning (which would require another goroutine)
+	// https://go.dev/issue/23019
+	stdinReader, stdinWriter, err := os.Pipe() // reader closed in goroutine / writer closed on ac.Close
+	if err != nil {
+		return AttributeChecker{}, err
+	}
+	stdoutReader, stdoutWriter := io.Pipe() // closed in goroutine
+
+	ac := AttributeChecker{
+		removeTempFiles: removeTempFiles, // called on ac.Close
+		stdinWriter:     stdinWriter,
+		readStdout:      newCheckAttrStdoutReader(stdoutReader, len(attributes)),
+		err:             &atomic.Value{},
+	}
+
 	go func() {
 		defer stdinReader.Close()
-		defer lw.Close()
+		defer stdoutWriter.Close() // in case of a panic (no-op if already closed by CloseWithError at the end)
 
 		stdErr := new(bytes.Buffer)
 		runOpts.Stdin = stdinReader
-		runOpts.Stdout = lw
+		runOpts.Stdout = stdoutWriter
 		runOpts.Stderr = stdErr
+
 		err := cmd.Run(runOpts)
 
-		if err != nil && //                       If there is an error we need to return but:
-			cmd.parentContext.Err() != err && //  1. Ignore the context error if the context is cancelled or exceeds the deadline (RunWithContext could return c.ctx.Err() which is Canceled or DeadlineExceeded)
-			err.Error() != "signal: killed" { // 2. We should not pass up errors due to the program being killed
-			log.Error("failed to run attr-check. Error: %v\nStderr: %s", err, stdErr.String())
+		// if the context was cancelled, Run error is irrelevant
+		if e := cmd.parentContext.Err(); e != nil {
+			err = e
 		}
+
+		if err != nil { // decorate the returned error
+			err = fmt.Errorf("git check-attr (stderr: %q): %w", strings.TrimSpace(stdErr.String()), err)
+			ac.err.Store(err)
+		}
+		stdoutWriter.CloseWithError(err)
 	}()
 
 	return ac, nil
 }
 
 type AttributeChecker struct {
-	ctx             context.Context
-	cancel          context.CancelFunc
-	stdinWriter     *os.File
-	attributeNumber int
-	attributesCh    <-chan attributeTriple
+	removeTempFiles context.CancelFunc
+	stdinWriter     io.WriteCloser
+	readStdout      func() (map[string]GitAttribute, error)
+	err             *atomic.Value
 }
 
 func (ac AttributeChecker) CheckPath(path string) (map[string]GitAttribute, error) {
-	if err := ac.ctx.Err(); err != nil {
-		return nil, err
-	}
-
 	if _, err := ac.stdinWriter.Write([]byte(path + "\x00")); err != nil {
-		return nil, err
+		// try to return the Run error if available, since it is likely more helpful
+		// than just "broken pipe"
+		if aerr, _ := ac.err.Load().(error); aerr != nil {
+			return nil, aerr
+		}
+		return nil, fmt.Errorf("git check-attr: %w", err)
 	}
 
-	rs := make(map[string]GitAttribute)
-	for i := 0; i < ac.attributeNumber; i++ {
-		select {
-		case attr, ok := <-ac.attributesCh:
-			if !ok {
-				return nil, ac.ctx.Err()
-			}
-			rs[attr.Attribute] = GitAttribute(attr.Value)
-		case <-ac.ctx.Done():
-			return nil, ac.ctx.Err()
-		}
-	}
-	return rs, nil
+	return ac.readStdout()
 }
 
 func (ac AttributeChecker) Close() error {
-	ac.cancel()
+	ac.removeTempFiles()
 	return ac.stdinWriter.Close()
 }

modules/git/repo_attribute_test.go

@@ -4,7 +4,14 @@
 package git
 
 import (
+	"context"
+	"fmt"
+	"io"
+	"io/fs"
+	"os"
 	"path/filepath"
+	"runtime"
+	"strings"
 	"testing"
 	"time"
 
@@ -14,90 +21,63 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) {
-	wr := &nulSeparatedAttributeWriter{
-		attributes: make(chan attributeTriple, 5),
-	}
+func TestNewCheckAttrStdoutReader(t *testing.T) {
+	t.Run("two_times", func(t *testing.T) {
+		read := newCheckAttrStdoutReader(strings.NewReader(
+			".gitignore\x00linguist-vendored\x00unspecified\x00"+
+				".gitignore\x00linguist-vendored\x00specified",
+		), 1)
 
-	testStr := ".gitignore\"\n\x00linguist-vendored\x00unspecified\x00"
+		// first read
+		attr, err := read()
+		assert.NoError(t, err)
+		assert.Equal(t, map[string]GitAttribute{
+			"linguist-vendored": GitAttribute("unspecified"),
+		}, attr)
 
-	n, err := wr.Write([]byte(testStr))
+		// second read
+		attr, err = read()
+		assert.NoError(t, err)
+		assert.Equal(t, map[string]GitAttribute{
+			"linguist-vendored": GitAttribute("specified"),
+		}, attr)
+	})
+	t.Run("incomplete", func(t *testing.T) {
+		read := newCheckAttrStdoutReader(strings.NewReader(
+			"filename\x00linguist-vendored",
+		), 1)
 
-	assert.Len(t, testStr, n)
-	assert.NoError(t, err)
-	select {
-	case attr := <-wr.attributes:
-		assert.Equal(t, ".gitignore\"\n", attr.Filename)
-		assert.Equal(t, "linguist-vendored", attr.Attribute)
-		assert.Equal(t, "unspecified", attr.Value)
-	case <-time.After(100 * time.Millisecond):
-		assert.FailNow(t, "took too long to read an attribute from the list")
-	}
-	// Write a second attribute again
-	n, err = wr.Write([]byte(testStr))
+		_, err := read()
+		assert.Equal(t, io.ErrUnexpectedEOF, err)
+	})
+	t.Run("three_times", func(t *testing.T) {
+		read := newCheckAttrStdoutReader(strings.NewReader(
+			"shouldbe.vendor\x00linguist-vendored\x00set\x00"+
+				"shouldbe.vendor\x00linguist-generated\x00unspecified\x00"+
+				"shouldbe.vendor\x00linguist-language\x00unspecified\x00",
+		), 1)
 
-	assert.Len(t, testStr, n)
-	assert.NoError(t, err)
+		// first read
+		attr, err := read()
+		assert.NoError(t, err)
+		assert.Equal(t, map[string]GitAttribute{
+			"linguist-vendored": GitAttribute("set"),
+		}, attr)
 
-	select {
-	case attr := <-wr.attributes:
-		assert.Equal(t, ".gitignore\"\n", attr.Filename)
-		assert.Equal(t, "linguist-vendored", attr.Attribute)
-		assert.Equal(t, "unspecified", attr.Value)
-	case <-time.After(100 * time.Millisecond):
-		assert.FailNow(t, "took too long to read an attribute from the list")
-	}
+		// second read
+		attr, err = read()
+		assert.NoError(t, err)
+		assert.Equal(t, map[string]GitAttribute{
+			"linguist-generated": GitAttribute("unspecified"),
+		}, attr)
 
-	// Write a partial attribute
-	_, err = wr.Write([]byte("incomplete-file"))
-	assert.NoError(t, err)
-	_, err = wr.Write([]byte("name\x00"))
-	assert.NoError(t, err)
-
-	select {
-	case <-wr.attributes:
-		assert.FailNow(t, "There should not be an attribute ready to read")
-	case <-time.After(100 * time.Millisecond):
-	}
-	_, err = wr.Write([]byte("attribute\x00"))
-	assert.NoError(t, err)
-	select {
-	case <-wr.attributes:
-		assert.FailNow(t, "There should not be an attribute ready to read")
-	case <-time.After(100 * time.Millisecond):
-	}
-
-	_, err = wr.Write([]byte("value\x00"))
-	assert.NoError(t, err)
-
-	attr := <-wr.attributes
-	assert.Equal(t, "incomplete-filename", attr.Filename)
-	assert.Equal(t, "attribute", attr.Attribute)
-	assert.Equal(t, "value", attr.Value)
-
-	_, err = wr.Write([]byte("shouldbe.vendor\x00linguist-vendored\x00set\x00shouldbe.vendor\x00linguist-generated\x00unspecified\x00shouldbe.vendor\x00linguist-language\x00unspecified\x00"))
-	assert.NoError(t, err)
-	attr = <-wr.attributes
-	assert.NoError(t, err)
-	assert.EqualValues(t, attributeTriple{
-		Filename:  "shouldbe.vendor",
-		Attribute: "linguist-vendored",
-		Value:     "set",
-	}, attr)
-	attr = <-wr.attributes
-	assert.NoError(t, err)
-	assert.EqualValues(t, attributeTriple{
-		Filename:  "shouldbe.vendor",
-		Attribute: "linguist-generated",
-		Value:     "unspecified",
-	}, attr)
-	attr = <-wr.attributes
-	assert.NoError(t, err)
-	assert.EqualValues(t, attributeTriple{
-		Filename:  "shouldbe.vendor",
-		Attribute: "linguist-language",
-		Value:     "unspecified",
-	}, attr)
+		// third read
+		attr, err = read()
+		assert.NoError(t, err)
+		assert.Equal(t, map[string]GitAttribute{
+			"linguist-language": GitAttribute("unspecified"),
+		}, attr)
+	})
 }
 
 func TestGitAttributeBareNonBare(t *testing.T) {
@@ -114,33 +94,35 @@ func TestGitAttributeBareNonBare(t *testing.T) {
 		"8fee858da5796dfb37704761701bb8e800ad9ef3",
 		"341fca5b5ea3de596dc483e54c2db28633cd2f97",
 	} {
-		t.Run("GitAttributeChecker/"+commitID, func(t *testing.T) {
+		bareStats, err := gitRepo.GitAttributes(commitID, "i-am-a-python.p", LinguistAttributes...)
+		assert.NoError(t, err)
+
+		defer test.MockVariableValue(&SupportCheckAttrOnBare, false)()
+		cloneStats, err := gitRepo.GitAttributes(commitID, "i-am-a-python.p", LinguistAttributes...)
+		assert.NoError(t, err)
+
+		assert.EqualValues(t, cloneStats, bareStats)
+		refStats := cloneStats
+
+		t.Run("GitAttributeChecker/"+commitID+"/SupportBare", func(t *testing.T) {
 			bareChecker, err := gitRepo.GitAttributeChecker(commitID, LinguistAttributes...)
 			assert.NoError(t, err)
-			t.Cleanup(func() { bareChecker.Close() })
+			defer bareChecker.Close()
 
 			bareStats, err := bareChecker.CheckPath("i-am-a-python.p")
 			assert.NoError(t, err)
-
+			assert.EqualValues(t, refStats, bareStats)
+		})
+		t.Run("GitAttributeChecker/"+commitID+"/NoBareSupport", func(t *testing.T) {
 			defer test.MockVariableValue(&SupportCheckAttrOnBare, false)()
 			cloneChecker, err := gitRepo.GitAttributeChecker(commitID, LinguistAttributes...)
 			assert.NoError(t, err)
-			t.Cleanup(func() { cloneChecker.Close() })
+			defer cloneChecker.Close()
+
 			cloneStats, err := cloneChecker.CheckPath("i-am-a-python.p")
 			assert.NoError(t, err)
 
-			assert.EqualValues(t, cloneStats, bareStats)
-		})
-
-		t.Run("GitAttributes/"+commitID, func(t *testing.T) {
-			bareStats, err := gitRepo.GitAttributes(commitID, "i-am-a-python.p", LinguistAttributes...)
-			assert.NoError(t, err)
-
-			defer test.MockVariableValue(&SupportCheckAttrOnBare, false)()
-			cloneStats, err := gitRepo.GitAttributes(commitID, "i-am-a-python.p", LinguistAttributes...)
-			assert.NoError(t, err)
-
-			assert.EqualValues(t, cloneStats, bareStats)
+			assert.EqualValues(t, refStats, cloneStats)
 		})
 	}
 }
@@ -208,3 +190,157 @@ func TestGitAttributeStruct(t *testing.T) {
 	assert.Equal(t, "text?token=Error", GitAttribute("text?token=Error").String())
 	assert.Equal(t, "text", GitAttribute("text?token=Error").Prefix())
 }
+
+func TestGitAttributeCheckerError(t *testing.T) {
+	prepareRepo := func(t *testing.T) *Repository {
+		t.Helper()
+		path := t.TempDir()
+
+		// we can't use unittest.CopyDir because of an import cycle (git.Init in unittest)
+		require.NoError(t, CopyFS(path, os.DirFS(filepath.Join(testReposDir, "language_stats_repo"))))
+
+		gitRepo, err := openRepositoryWithDefaultContext(path)
+		require.NoError(t, err)
+		return gitRepo
+	}
+
+	t.Run("RemoveAll/BeforeRun", func(t *testing.T) {
+		gitRepo := prepareRepo(t)
+		defer gitRepo.Close()
+
+		assert.NoError(t, os.RemoveAll(gitRepo.Path))
+
+		ac, err := gitRepo.GitAttributeChecker("", "linguist-language")
+		require.NoError(t, err)
+
+		_, err = ac.CheckPath("i-am-a-python.p")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), `git check-attr (stderr: ""):`)
+	})
+
+	t.Run("RemoveAll/DuringRun", func(t *testing.T) {
+		gitRepo := prepareRepo(t)
+		defer gitRepo.Close()
+
+		ac, err := gitRepo.GitAttributeChecker("", "linguist-language")
+		require.NoError(t, err)
+
+		// calling CheckPath before would allow git to cache part of it and succesfully return later
+		assert.NoError(t, os.RemoveAll(gitRepo.Path))
+
+		_, err = ac.CheckPath("i-am-a-python.p")
+		assert.Error(t, err)
+		// Depending on the order of execution, the returned error can be:
+		// - a launch error "fork/exec /usr/bin/git: no such file or directory" (when the removal happens before the Run)
+		// - a git error (stderr: "fatal: Unable to read current working directory: No such file or directory"): exit status 128 (when the removal happens after the Run)
+		// (pipe error "write |1: broken pipe" should be replaced by one of the Run errors above)
+		assert.Contains(t, err.Error(), `git check-attr`)
+	})
+
+	t.Run("Cancelled/BeforeRun", func(t *testing.T) {
+		gitRepo := prepareRepo(t)
+		defer gitRepo.Close()
+
+		var cancel context.CancelFunc
+		gitRepo.Ctx, cancel = context.WithCancel(gitRepo.Ctx)
+		cancel()
+
+		ac, err := gitRepo.GitAttributeChecker("8fee858da5796dfb37704761701bb8e800ad9ef3", "linguist-language")
+		require.NoError(t, err)
+
+		_, err = ac.CheckPath("i-am-a-python.p")
+		assert.ErrorIs(t, err, context.Canceled)
+	})
+
+	t.Run("Cancelled/DuringRun", func(t *testing.T) {
+		gitRepo := prepareRepo(t)
+		defer gitRepo.Close()
+
+		var cancel context.CancelFunc
+		gitRepo.Ctx, cancel = context.WithCancel(gitRepo.Ctx)
+
+		ac, err := gitRepo.GitAttributeChecker("8fee858da5796dfb37704761701bb8e800ad9ef3", "linguist-language")
+		require.NoError(t, err)
+
+		attr, err := ac.CheckPath("i-am-a-python.p")
+		assert.NoError(t, err)
+		assert.Equal(t, "Python", attr["linguist-language"].String())
+
+		errCh := make(chan error)
+		go func() {
+			cancel()
+
+			for err == nil {
+				_, err = ac.CheckPath("i-am-a-python.p")
+				runtime.Gosched() // the cancellation must have time to propagate
+			}
+			errCh <- err
+		}()
+
+		select {
+		case <-time.After(time.Second):
+			t.Error("CheckPath did not complete within 1s")
+		case err = <-errCh:
+			assert.ErrorIs(t, err, context.Canceled)
+		}
+	})
+
+	t.Run("Closed/BeforeRun", func(t *testing.T) {
+		gitRepo := prepareRepo(t)
+		defer gitRepo.Close()
+
+		ac, err := gitRepo.GitAttributeChecker("8fee858da5796dfb37704761701bb8e800ad9ef3", "linguist-language")
+		require.NoError(t, err)
+
+		assert.NoError(t, ac.Close())
+
+		_, err = ac.CheckPath("i-am-a-python.p")
+		assert.ErrorIs(t, err, fs.ErrClosed)
+	})
+
+	t.Run("Closed/DuringRun", func(t *testing.T) {
+		gitRepo := prepareRepo(t)
+		defer gitRepo.Close()
+
+		ac, err := gitRepo.GitAttributeChecker("8fee858da5796dfb37704761701bb8e800ad9ef3", "linguist-language")
+		require.NoError(t, err)
+
+		attr, err := ac.CheckPath("i-am-a-python.p")
+		assert.NoError(t, err)
+		assert.Equal(t, "Python", attr["linguist-language"].String())
+
+		assert.NoError(t, ac.Close())
+
+		_, err = ac.CheckPath("i-am-a-python.p")
+		assert.ErrorIs(t, err, fs.ErrClosed)
+	})
+}
+
+// CopyFS is adapted from https://github.com/golang/go/issues/62484
+// which should be available with go1.23
+func CopyFS(dir string, fsys fs.FS) error {
+	return fs.WalkDir(fsys, ".", func(path string, d fs.DirEntry, _ error) error {
+		targ := filepath.Join(dir, filepath.FromSlash(path))
+		if d.IsDir() {
+			return os.MkdirAll(targ, 0o777)
+		}
+		r, err := fsys.Open(path)
+		if err != nil {
+			return err
+		}
+		defer r.Close()
+		info, err := r.Stat()
+		if err != nil {
+			return err
+		}
+		w, err := os.OpenFile(targ, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o666|info.Mode()&0o777)
+		if err != nil {
+			return err
+		}
+		if _, err := io.Copy(w, r); err != nil {
+			w.Close()
+			return fmt.Errorf("copying %s: %v", path, err)
+		}
+		return w.Close()
+	})
+}