Merge pull request '[PERFORMANCE] git check-attr on bare repo if supported' (#2763) from oliverpool/forgejo:check_attr_bare into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2763
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

modules/git/git.go

@@ -36,6 +36,7 @@ var (
 	SupportProcReceive     bool // >= 2.29
 	SupportHashSha256      bool // >= 2.42, SHA-256 repositories no longer an ‘experimental curiosity’
 	InvertedGitFlushEnv    bool // 2.43.1
+	SupportCheckAttrOnBare bool // >= 2.40
 
 	gitVersion *version.Version
 )
@@ -187,6 +188,7 @@ func InitFull(ctx context.Context) (err error) {
 	}
 	SupportProcReceive = CheckGitVersionAtLeast("2.29") == nil
 	SupportHashSha256 = CheckGitVersionAtLeast("2.42") == nil && !isGogit
+	SupportCheckAttrOnBare = CheckGitVersionAtLeast("2.40") == nil
 	if SupportHashSha256 {
 		SupportedObjectFormats = append(SupportedObjectFormats, Sha256ObjectFormat)
 	} else {

modules/git/repo_attribute.go

@@ -7,65 +7,147 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io"
 	"os"
+	"strings"
 
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/optional"
 )
 
-// CheckAttributeOpts represents the possible options to CheckAttribute
-type CheckAttributeOpts struct {
-	CachedOnly    bool
-	AllAttributes bool
-	Attributes    []string
-	Filenames     []string
-	IndexFile     string
-	WorkTree      string
+var LinguistAttributes = []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language", "linguist-documentation", "linguist-detectable"}
+
+// GitAttribute exposes an attribute from the .gitattribute file
+type GitAttribute string //nolint:revive
+
+// IsSpecified returns true if the gitattribute is set and not empty
+func (ca GitAttribute) IsSpecified() bool {
+	return ca != "" && ca != "unspecified"
 }
 
-// CheckAttribute return the Blame object of file
-func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[string]string, error) {
-	env := []string{}
-
-	if len(opts.IndexFile) > 0 {
-		env = append(env, "GIT_INDEX_FILE="+opts.IndexFile)
+// String returns the value of the attribute or "" if unspecified
+func (ca GitAttribute) String() string {
+	if !ca.IsSpecified() {
+		return ""
 	}
-	if len(opts.WorkTree) > 0 {
-		env = append(env, "GIT_WORK_TREE="+opts.WorkTree)
+	return string(ca)
 }
 
-	if len(env) > 0 {
-		env = append(os.Environ(), env...)
+// Prefix returns the value of the attribute before any question mark '?'
+//
+// sometimes used within gitlab-language: https://docs.gitlab.com/ee/user/project/highlighting.html#override-syntax-highlighting-for-a-file-type
+func (ca GitAttribute) Prefix() string {
+	s := ca.String()
+	if i := strings.IndexByte(s, '?'); i >= 0 {
+		return s[:i]
+	}
+	return s
 }
 
-	stdOut := new(bytes.Buffer)
-	stdErr := new(bytes.Buffer)
-
-	cmd := NewCommand(repo.Ctx, "check-attr", "-z")
-
-	if opts.AllAttributes {
-		cmd.AddArguments("-a")
-	} else {
-		for _, attribute := range opts.Attributes {
-			if attribute != "" {
-				cmd.AddDynamicArguments(attribute)
+// Bool returns true if "set"/"true", false if "unset"/"false", none otherwise
+func (ca GitAttribute) Bool() optional.Option[bool] {
+	switch ca {
+	case "set", "true":
+		return optional.Some(true)
+	case "unset", "false":
+		return optional.Some(false)
 	}
+	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
+}
+
+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
+
+	// git < 2.40 cannot run check-attr on bare repo, but needs INDEX + WORK_TREE
+	hasIndex := treeish == ""
+	if !hasIndex && !SupportCheckAttrOnBare {
+		indexFilename, worktree, cancel, err := repo.ReadTreeToTemporaryIndex(treeish)
+		if err != nil {
+			return nil, nil, nil, err
+		}
+		deleteTemporaryFile = cancel
+
+		env = append(env, "GIT_INDEX_FILE="+indexFilename, "GIT_WORK_TREE="+worktree)
+
+		hasIndex = true
+
+		// 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()
 		}
 	}
 
-	if opts.CachedOnly {
+	cmd := NewCommand(ctx, "check-attr", "-z")
+
+	if hasIndex {
 		cmd.AddArguments("--cached")
 	}
 
-	cmd.AddDashesAndList(opts.Filenames...)
+	if len(treeish) > 0 {
+		cmd.AddArguments("--source")
+		cmd.AddDynamicArguments(treeish)
+	}
+	cmd.AddDynamicArguments(attributes...)
 
-	if err := cmd.Run(&RunOpts{
+	// Version 2.43.1 has a bug where the behavior of `GIT_FLUSH` is flipped.
+	// Ref: https://lore.kernel.org/git/CABn0oJvg3M_kBW-u=j3QhKnO=6QOzk-YFTgonYw_UvFS1NTX4g@mail.gmail.com
+	if InvertedGitFlushEnv {
+		env = append(env, "GIT_FLUSH=0")
+	} else {
+		env = append(env, "GIT_FLUSH=1")
+	}
+
+	return cmd, &RunOpts{
 		Env: env,
 		Dir: repo.Path,
-		Stdout: stdOut,
-		Stderr: stdErr,
-	}); err != nil {
+	}, cancel, nil
+}
+
+// GitAttributes returns gitattribute.
+//
+// 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...)
+	if err != nil {
+		return nil, err
+	}
+	defer cancel()
+
+	stdOut := new(bytes.Buffer)
+	runOpts.Stdout = stdOut
+
+	stdErr := new(bytes.Buffer)
+	runOpts.Stderr = stdErr
+
+	cmd.AddDashesAndList(filename)
+
+	if err := cmd.Run(runOpts); err != nil {
 		return nil, fmt.Errorf("failed to run check-attr: %w\n%s\n%s", err, stdOut.String(), stdErr.String())
 	}
 
@@ -76,155 +158,14 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[
 		return nil, fmt.Errorf("wrong number of fields in return from check-attr")
 	}
 
-	name2attribute2info := make(map[string]map[string]string)
-
-	for i := 0; i < (len(fields) / 3); i++ {
-		filename := string(fields[3*i])
-		attribute := string(fields[3*i+1])
-		info := string(fields[3*i+2])
-		attribute2info := name2attribute2info[filename]
-		if attribute2info == nil {
-			attribute2info = make(map[string]string)
+	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)
 	}
-		attribute2info[attribute] = info
-		name2attribute2info[filename] = attribute2info
-	}
-
-	return name2attribute2info, nil
-}
-
-// CheckAttributeReader provides a reader for check-attribute content that can be long running
-type CheckAttributeReader struct {
-	// params
-	Attributes []string
-	Repo       *Repository
-	IndexFile  string
-	WorkTree   string
-
-	stdinReader io.ReadCloser
-	stdinWriter *os.File
-	stdOut      attributeWriter
-	cmd         *Command
-	env         []string
-	ctx         context.Context
-	cancel      context.CancelFunc
-}
-
-// Init initializes the CheckAttributeReader
-func (c *CheckAttributeReader) Init(ctx context.Context) error {
-	if len(c.Attributes) == 0 {
-		lw := new(nulSeparatedAttributeWriter)
-		lw.attributes = make(chan attributeTriple)
-		lw.closed = make(chan struct{})
-
-		c.stdOut = lw
-		c.stdOut.Close()
-		return fmt.Errorf("no provided Attributes to check")
-	}
-
-	c.ctx, c.cancel = context.WithCancel(ctx)
-	c.cmd = NewCommand(c.ctx, "check-attr", "--stdin", "-z")
-
-	if len(c.IndexFile) > 0 {
-		c.cmd.AddArguments("--cached")
-		c.env = append(c.env, "GIT_INDEX_FILE="+c.IndexFile)
-	}
-
-	if len(c.WorkTree) > 0 {
-		c.env = append(c.env, "GIT_WORK_TREE="+c.WorkTree)
-	}
-
-	// Version 2.43.1 has a bug where the behavior of `GIT_FLUSH` is flipped.
-	// Ref: https://lore.kernel.org/git/CABn0oJvg3M_kBW-u=j3QhKnO=6QOzk-YFTgonYw_UvFS1NTX4g@mail.gmail.com
-	if InvertedGitFlushEnv {
-		c.env = append(c.env, "GIT_FLUSH=0")
-	} else {
-		c.env = append(c.env, "GIT_FLUSH=1")
-	}
-
-	c.cmd.AddDynamicArguments(c.Attributes...)
-
-	var err error
-
-	c.stdinReader, c.stdinWriter, err = os.Pipe()
-	if err != nil {
-		c.cancel()
-		return err
-	}
-
-	lw := new(nulSeparatedAttributeWriter)
-	lw.attributes = make(chan attributeTriple, 5)
-	lw.closed = make(chan struct{})
-	c.stdOut = lw
-	return nil
-}
-
-// Run run cmd
-func (c *CheckAttributeReader) Run() error {
-	defer func() {
-		_ = c.stdinReader.Close()
-		_ = c.stdOut.Close()
-	}()
-	stdErr := new(bytes.Buffer)
-	err := c.cmd.Run(&RunOpts{
-		Env:    c.env,
-		Dir:    c.Repo.Path,
-		Stdin:  c.stdinReader,
-		Stdout: c.stdOut,
-		Stderr: stdErr,
-	})
-	if err != nil && //                      If there is an error we need to return but:
-		c.ctx.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
-		return fmt.Errorf("failed to run attr-check. Error: %w\nStderr: %s", err, stdErr.String())
-	}
-	return nil
-}
-
-// CheckPath check attr for given path
-func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err error) {
-	defer func() {
-		if err != nil && err != c.ctx.Err() {
-			log.Error("Unexpected error when checking path %s in %s. Error: %v", path, c.Repo.Path, err)
-		}
-	}()
-
-	select {
-	case <-c.ctx.Done():
-		return nil, c.ctx.Err()
-	default:
-	}
-
-	if _, err = c.stdinWriter.Write([]byte(path + "\x00")); err != nil {
-		defer c.Close()
-		return nil, err
-	}
-
-	rs = make(map[string]string)
-	for range c.Attributes {
-		select {
-		case attr, ok := <-c.stdOut.ReadAttribute():
-			if !ok {
-				return nil, c.ctx.Err()
-			}
-			rs[attr.Attribute] = attr.Value
-		case <-c.ctx.Done():
-			return nil, c.ctx.Err()
-		}
-	}
-	return rs, nil
-}
-
-// Close close pip after use
-func (c *CheckAttributeReader) Close() error {
-	c.cancel()
-	err := c.stdinWriter.Close()
-	return err
-}
-
-type attributeWriter interface {
-	io.WriteCloser
-	ReadAttribute() <-chan attributeTriple
+	return values, nil
 }
 
 type attributeTriple struct {
@@ -275,10 +216,6 @@ func (wr *nulSeparatedAttributeWriter) Write(p []byte) (n int, err error) {
 	return len(p), nil
 }
 
-func (wr *nulSeparatedAttributeWriter) ReadAttribute() <-chan attributeTriple {
-	return wr.attributes
-}
-
 func (wr *nulSeparatedAttributeWriter) Close() error {
 	select {
 	case <-wr.closed:
@@ -290,49 +227,87 @@ func (wr *nulSeparatedAttributeWriter) Close() error {
 	return nil
 }
 
-// Create a check attribute reader for the current repository and provided commit ID
-func (repo *Repository) CheckAttributeReader(commitID string) (*CheckAttributeReader, context.CancelFunc) {
-	indexFilename, worktree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID)
+// GitAttributeChecker creates an AttributeChecker for the given repository and provided commit ID.
+//
+// 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...)
 	if err != nil {
-		return nil, func() {}
+		return AttributeChecker{}, err
 	}
 
-	checker := &CheckAttributeReader{
-		Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language", "linguist-documentation", "linguist-detectable"},
-		Repo:       repo,
-		IndexFile:  indexFilename,
-		WorkTree:   worktree,
+	ac := AttributeChecker{
+		attributeNumber: len(attributes),
+		ctx:             cmd.parentContext,
+		cancel:          cancel, // will be cancelled on Close
 	}
-	ctx, cancel := context.WithCancel(repo.Ctx)
-	if err := checker.Init(ctx); err != nil {
-		log.Error("Unable to open checker for %s. Error: %v", commitID, err)
-	} else {
+
+	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")
 	go func() {
-			err := checker.Run()
-			if err != nil && err != ctx.Err() {
-				log.Error("Unable to open checker for %s. Error: %v", commitID, err)
+		defer stdinReader.Close()
+		defer lw.Close()
+
+		stdErr := new(bytes.Buffer)
+		runOpts.Stdin = stdinReader
+		runOpts.Stdout = lw
+		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: %w\nStderr: %s", err, stdErr.String())
 		}
-			cancel()
 	}()
-	}
-	deferable := func() {
-		_ = checker.Close()
-		cancel()
-		deleteTemporaryFile()
+
+	return ac, nil
 }
 
-	return checker, deferable
+type AttributeChecker struct {
+	ctx             context.Context
+	cancel          context.CancelFunc
+	stdinWriter     *os.File
+	attributeNumber int
+	attributesCh    <-chan attributeTriple
 }
 
-// true if "set"/"true", false if "unset"/"false", none otherwise
-func attributeToBool(attr map[string]string, name string) optional.Option[bool] {
-	if value, has := attr[name]; has && value != "unspecified" {
-		switch value {
-		case "set", "true":
-			return optional.Some(true)
-		case "unset", "false":
-			return optional.Some(false)
+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
+	}
+
+	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 optional.None[bool]()
+	return rs, nil
+}
+
+func (ac AttributeChecker) Close() error {
+	ac.cancel()
+	return ac.stdinWriter.Close()
 }

modules/git/repo_attribute_test.go

@@ -4,10 +4,14 @@
 package git
 
 import (
+	"path/filepath"
 	"testing"
 	"time"
 
+	"code.gitea.io/gitea/modules/test"
+
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) {
@@ -22,7 +26,7 @@ func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) {
 	assert.Len(t, testStr, n)
 	assert.NoError(t, err)
 	select {
-	case attr := <-wr.ReadAttribute():
+	case attr := <-wr.attributes:
 		assert.Equal(t, ".gitignore\"\n", attr.Filename)
 		assert.Equal(t, "linguist-vendored", attr.Attribute)
 		assert.Equal(t, "unspecified", attr.Value)
@@ -36,7 +40,7 @@ func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) {
 	assert.NoError(t, err)
 
 	select {
-	case attr := <-wr.ReadAttribute():
+	case attr := <-wr.attributes:
 		assert.Equal(t, ".gitignore\"\n", attr.Filename)
 		assert.Equal(t, "linguist-vendored", attr.Attribute)
 		assert.Equal(t, "unspecified", attr.Value)
@@ -51,14 +55,14 @@ func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) {
 	assert.NoError(t, err)
 
 	select {
-	case <-wr.ReadAttribute():
+	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.ReadAttribute():
+	case <-wr.attributes:
 		assert.FailNow(t, "There should not be an attribute ready to read")
 	case <-time.After(100 * time.Millisecond):
 	}
@@ -66,28 +70,28 @@ func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) {
 	_, err = wr.Write([]byte("value\x00"))
 	assert.NoError(t, err)
 
-	attr := <-wr.ReadAttribute()
+	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.ReadAttribute()
+	attr = <-wr.attributes
 	assert.NoError(t, err)
 	assert.EqualValues(t, attributeTriple{
 		Filename:  "shouldbe.vendor",
 		Attribute: "linguist-vendored",
 		Value:     "set",
 	}, attr)
-	attr = <-wr.ReadAttribute()
+	attr = <-wr.attributes
 	assert.NoError(t, err)
 	assert.EqualValues(t, attributeTriple{
 		Filename:  "shouldbe.vendor",
 		Attribute: "linguist-generated",
 		Value:     "unspecified",
 	}, attr)
-	attr = <-wr.ReadAttribute()
+	attr = <-wr.attributes
 	assert.NoError(t, err)
 	assert.EqualValues(t, attributeTriple{
 		Filename:  "shouldbe.vendor",
@@ -95,3 +99,112 @@ func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) {
 		Value:     "unspecified",
 	}, attr)
 }
+
+func TestGitAttributeBareNonBare(t *testing.T) {
+	if !SupportCheckAttrOnBare {
+		t.Skip("git check-attr supported on bare repo starting with git 2.40")
+	}
+
+	repoPath := filepath.Join(testReposDir, "language_stats_repo")
+	gitRepo, err := openRepositoryWithDefaultContext(repoPath)
+	require.NoError(t, err)
+	defer gitRepo.Close()
+
+	for _, commitID := range []string{
+		"8fee858da5796dfb37704761701bb8e800ad9ef3",
+		"341fca5b5ea3de596dc483e54c2db28633cd2f97",
+	} {
+		t.Run("GitAttributeChecker/"+commitID, func(t *testing.T) {
+			bareChecker, err := gitRepo.GitAttributeChecker(commitID, LinguistAttributes...)
+			assert.NoError(t, err)
+			t.Cleanup(func() { bareChecker.Close() })
+
+			bareStats, err := bareChecker.CheckPath("i-am-a-python.p")
+			assert.NoError(t, err)
+
+			defer test.MockVariableValue(&SupportCheckAttrOnBare, false)()
+			cloneChecker, err := gitRepo.GitAttributeChecker(commitID, LinguistAttributes...)
+			assert.NoError(t, err)
+			t.Cleanup(func() { 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)
+		})
+	}
+}
+
+func TestGitAttributes(t *testing.T) {
+	repoPath := filepath.Join(testReposDir, "language_stats_repo")
+	gitRepo, err := openRepositoryWithDefaultContext(repoPath)
+	require.NoError(t, err)
+	defer gitRepo.Close()
+
+	attr, err := gitRepo.GitAttributes("8fee858da5796dfb37704761701bb8e800ad9ef3", "i-am-a-python.p", LinguistAttributes...)
+	assert.NoError(t, err)
+	assert.EqualValues(t, map[string]GitAttribute{
+		"gitlab-language":        "unspecified",
+		"linguist-detectable":    "unspecified",
+		"linguist-documentation": "unspecified",
+		"linguist-generated":     "unspecified",
+		"linguist-language":      "Python",
+		"linguist-vendored":      "unspecified",
+	}, attr)
+
+	attr, err = gitRepo.GitAttributes("341fca5b5ea3de596dc483e54c2db28633cd2f97", "i-am-a-python.p", LinguistAttributes...)
+	assert.NoError(t, err)
+	assert.EqualValues(t, map[string]GitAttribute{
+		"gitlab-language":        "unspecified",
+		"linguist-detectable":    "unspecified",
+		"linguist-documentation": "unspecified",
+		"linguist-generated":     "unspecified",
+		"linguist-language":      "Cobra",
+		"linguist-vendored":      "unspecified",
+	}, attr)
+}
+
+func TestGitAttributeFirst(t *testing.T) {
+	repoPath := filepath.Join(testReposDir, "language_stats_repo")
+	gitRepo, err := openRepositoryWithDefaultContext(repoPath)
+	require.NoError(t, err)
+	defer gitRepo.Close()
+
+	t.Run("first is specified", func(t *testing.T) {
+		language, err := gitRepo.GitAttributeFirst("8fee858da5796dfb37704761701bb8e800ad9ef3", "i-am-a-python.p", "linguist-language", "gitlab-language")
+		assert.NoError(t, err)
+		assert.Equal(t, "Python", language.String())
+	})
+
+	t.Run("second is specified", func(t *testing.T) {
+		language, err := gitRepo.GitAttributeFirst("8fee858da5796dfb37704761701bb8e800ad9ef3", "i-am-a-python.p", "gitlab-language", "linguist-language")
+		assert.NoError(t, err)
+		assert.Equal(t, "Python", language.String())
+	})
+
+	t.Run("none is specified", func(t *testing.T) {
+		language, err := gitRepo.GitAttributeFirst("8fee858da5796dfb37704761701bb8e800ad9ef3", "i-am-a-python.p", "linguist-detectable", "gitlab-language", "non-existing")
+		assert.NoError(t, err)
+		assert.Equal(t, "", language.String())
+	})
+}
+
+func TestGitAttributeStruct(t *testing.T) {
+	assert.Equal(t, "", GitAttribute("").String())
+	assert.Equal(t, "", GitAttribute("unspecified").String())
+
+	assert.Equal(t, "python", GitAttribute("python").String())
+
+	assert.Equal(t, "text?token=Error", GitAttribute("text?token=Error").String())
+	assert.Equal(t, "text", GitAttribute("text?token=Error").Prefix())
+}

modules/git/repo_language_stats_nogogit.go

@@ -8,8 +8,8 @@ package git
 
 import (
 	"bytes"
+	"cmp"
 	"io"
-	"strings"
 
 	"code.gitea.io/gitea/modules/analyze"
 	"code.gitea.io/gitea/modules/log"
@@ -61,8 +61,11 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
 		return nil, err
 	}
 
-	checker, deferable := repo.CheckAttributeReader(commitID)
-	defer deferable()
+	checker, err := repo.GitAttributeChecker(commitID, LinguistAttributes...)
+	if err != nil {
+		return nil, err
+	}
+	defer checker.Close()
 
 	contentBuf := bytes.Buffer{}
 	var content []byte
@@ -102,14 +105,16 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
 		isDocumentation := optional.None[bool]()
 		isDetectable := optional.None[bool]()
 
-		if checker != nil {
 		attrs, err := checker.CheckPath(f.Name())
 		if err == nil {
-				isVendored = attributeToBool(attrs, "linguist-vendored")
-				isGenerated = attributeToBool(attrs, "linguist-generated")
-				isDocumentation = attributeToBool(attrs, "linguist-documentation")
-				isDetectable = attributeToBool(attrs, "linguist-detectable")
-				if language, has := attrs["linguist-language"]; has && language != "unspecified" && language != "" {
+			isVendored = attrs["linguist-vendored"].Bool()
+			isGenerated = attrs["linguist-generated"].Bool()
+			isDocumentation = attrs["linguist-documentation"].Bool()
+			isDetectable = attrs["linguist-detectable"].Bool()
+			if language := cmp.Or(
+				attrs["linguist-language"].String(),
+				attrs["gitlab-language"].Prefix(),
+			); language != "" {
 				// group languages, such as Pug -> HTML; SCSS -> CSS
 				group := enry.GetLanguageGroup(language)
 				if len(group) != 0 {
@@ -119,24 +124,6 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
 				// this language will always be added to the size
 				sizes[language] += f.Size()
 				continue
-				} else if language, has := attrs["gitlab-language"]; has && language != "unspecified" && language != "" {
-					// strip off a ? if present
-					if idx := strings.IndexByte(language, '?'); idx >= 0 {
-						language = language[:idx]
-					}
-					if len(language) != 0 {
-						// group languages, such as Pug -> HTML; SCSS -> CSS
-						group := enry.GetLanguageGroup(language)
-						if len(group) != 0 {
-							language = group
-						}
-
-						// this language will always be added to the size
-						sizes[language] += f.Size()
-						continue
-					}
-				}
-
 			}
 		}
 

modules/git/tests/repos/language_stats_repo/config

@@ -1,5 +1,5 @@
 [core]
 	repositoryformatversion = 0
 	filemode = true
-	bare = false
+	bare = true
 	logallrefupdates = true

modules/git/tests/repos/language_stats_repo/logs/HEAD

@@ -1 +1,2 @@
 0000000000000000000000000000000000000000 8fee858da5796dfb37704761701bb8e800ad9ef3 Andrew Thornton <art27@cantab.net> 1632140318 +0100	commit (initial): Add some test files for GetLanguageStats
+8fee858da5796dfb37704761701bb8e800ad9ef3 341fca5b5ea3de596dc483e54c2db28633cd2f97 oliverpool <git@olivier.pfad.fr> 1711278775 +0100	push

modules/git/tests/repos/language_stats_repo/logs/refs/heads/master

@@ -1 +1,2 @@
 0000000000000000000000000000000000000000 8fee858da5796dfb37704761701bb8e800ad9ef3 Andrew Thornton <art27@cantab.net> 1632140318 +0100	commit (initial): Add some test files for GetLanguageStats
+8fee858da5796dfb37704761701bb8e800ad9ef3 341fca5b5ea3de596dc483e54c2db28633cd2f97 oliverpool <git@olivier.pfad.fr> 1711278775 +0100	push

modules/git/tests/repos/language_stats_repo/refs/heads/master

@@ -1 +1 @@
-8fee858da5796dfb37704761701bb8e800ad9ef3
+341fca5b5ea3de596dc483e54c2db28633cd2f97

routers/web/repo/setting/lfs.go

@@ -145,30 +145,13 @@ func LFSLocks(ctx *context.Context) {
 		return
 	}
 
-	name2attribute2info, err := gitRepo.CheckAttribute(git.CheckAttributeOpts{
-		Attributes: []string{"lockable"},
-		Filenames:  filenames,
-		CachedOnly: true,
-	})
+	ctx.Data["Lockables"], err = lockablesGitAttributes(gitRepo, lfsLocks)
 	if err != nil {
-		log.Error("Unable to check attributes in %s (%v)", tmpBasePath, err)
+		log.Error("Unable to get lockablesGitAttributes in %s (%v)", tmpBasePath, err)
 		ctx.ServerError("LFSLocks", err)
 		return
 	}
 
-	lockables := make([]bool, len(lfsLocks))
-	for i, lock := range lfsLocks {
-		attribute2info, has := name2attribute2info[lock.Path]
-		if !has {
-			continue
-		}
-		if attribute2info["lockable"] != "set" {
-			continue
-		}
-		lockables[i] = true
-	}
-	ctx.Data["Lockables"] = lockables
-
 	filelist, err := gitRepo.LsFiles(filenames...)
 	if err != nil {
 		log.Error("Unable to lsfiles in %s (%v)", tmpBasePath, err)
@@ -189,6 +172,24 @@ func LFSLocks(ctx *context.Context) {
 	ctx.HTML(http.StatusOK, tplSettingsLFSLocks)
 }
 
+func lockablesGitAttributes(gitRepo *git.Repository, lfsLocks []*git_model.LFSLock) ([]bool, error) {
+	checker, err := gitRepo.GitAttributeChecker("", "lockable")
+	if err != nil {
+		return nil, fmt.Errorf("could not GitAttributeChecker: %w", err)
+	}
+	defer checker.Close()
+
+	lockables := make([]bool, len(lfsLocks))
+	for i, lock := range lfsLocks {
+		attrs, err := checker.CheckPath(lock.Path)
+		if err != nil {
+			return nil, fmt.Errorf("could not CheckPath(%s): %w", lock.Path, err)
+		}
+		lockables[i] = attrs["lockable"].Bool().Value()
+	}
+	return lockables, nil
+}
+
 // LFSLockFile locks a file
 func LFSLockFile(ctx *context.Context) {
 	if !setting.LFS.StartServer {

routers/web/repo/view.go

@@ -643,17 +643,12 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) {
 	}
 
 	if ctx.Repo.GitRepo != nil {
-		checker, deferable := ctx.Repo.GitRepo.CheckAttributeReader(ctx.Repo.CommitID)
-		if checker != nil {
-			defer deferable()
-			attrs, err := checker.CheckPath(ctx.Repo.TreePath)
-			if err == nil {
-				vendored, has := attrs["linguist-vendored"]
-				ctx.Data["IsVendored"] = has && (vendored == "set" || vendored == "true")
-
-				generated, has := attrs["linguist-generated"]
-				ctx.Data["IsGenerated"] = has && (generated == "set" || generated == "true")
-			}
+		attrs, err := ctx.Repo.GitRepo.GitAttributes(ctx.Repo.CommitID, ctx.Repo.TreePath, "linguist-vendored", "linguist-generated")
+		if err != nil {
+			log.Error("GitAttributes(%s, %s) failed: %v", ctx.Repo.CommitID, ctx.Repo.TreePath, err)
+		} else {
+			ctx.Data["IsVendored"] = attrs["linguist-vendored"].Bool().Value()
+			ctx.Data["IsGenerated"] = attrs["linguist-generated"].Bool().Value()
 		}
 	}
 

services/gitdiff/gitdiff.go

@@ -7,6 +7,7 @@ package gitdiff
 import (
 	"bufio"
 	"bytes"
+	"cmp"
 	"context"
 	"fmt"
 	"html"
@@ -1172,38 +1173,32 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi
 	}
 	diff.Start = opts.SkipTo
 
-	checker, deferable := gitRepo.CheckAttributeReader(opts.AfterCommitID)
-	defer deferable()
+	checker, err := gitRepo.GitAttributeChecker(opts.AfterCommitID, git.LinguistAttributes...)
+	if err != nil {
+		return nil, fmt.Errorf("unable to GitAttributeChecker: %w", err)
+	}
+	defer checker.Close()
 
 	for _, diffFile := range diff.Files {
-
 		gotVendor := false
 		gotGenerated := false
-		if checker != nil {
+
 		attrs, err := checker.CheckPath(diffFile.Name)
-			if err == nil {
-				if vendored, has := attrs["linguist-vendored"]; has {
-					if vendored == "set" || vendored == "true" {
-						diffFile.IsVendored = true
-						gotVendor = true
+		if err != nil {
+			log.Error("checker.CheckPath(%s) failed: %v", diffFile.Name, err)
 		} else {
-						gotVendor = vendored == "false"
-					}
-				}
-				if generated, has := attrs["linguist-generated"]; has {
-					if generated == "set" || generated == "true" {
-						diffFile.IsGenerated = true
-						gotGenerated = true
-					} else {
-						gotGenerated = generated == "false"
-					}
-				}
-				if language, has := attrs["linguist-language"]; has && language != "unspecified" && language != "" {
-					diffFile.Language = language
-				} else if language, has := attrs["gitlab-language"]; has && language != "unspecified" && language != "" {
-					diffFile.Language = language
-				}
-			}
+			vendored := attrs["linguist-vendored"].Bool()
+			diffFile.IsVendored = vendored.Value()
+			gotVendor = vendored.Has()
+
+			generated := attrs["linguist-generated"].Bool()
+			diffFile.IsGenerated = generated.Value()
+			gotGenerated = generated.Has()
+
+			diffFile.Language = cmp.Or(
+				attrs["linguist-language"].String(),
+				attrs["gitlab-language"].Prefix(),
+			)
 		}
 
 		if !gotVendor {

services/repository/files/content.go

@@ -273,31 +273,6 @@ func GetBlobBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git
 
 // TryGetContentLanguage tries to get the (linguist) language of the file content
 func TryGetContentLanguage(gitRepo *git.Repository, commitID, treePath string) (string, error) {
-	indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(commitID)
-	if err != nil {
-		return "", err
-	}
-
-	defer deleteTemporaryFile()
-
-	filename2attribute2info, err := gitRepo.CheckAttribute(git.CheckAttributeOpts{
-		CachedOnly: true,
-		Attributes: []string{"linguist-language", "gitlab-language"},
-		Filenames:  []string{treePath},
-		IndexFile:  indexFilename,
-		WorkTree:   worktree,
-	})
-	if err != nil {
-		return "", err
-	}
-
-	language := filename2attribute2info[treePath]["linguist-language"]
-	if language == "" || language == "unspecified" {
-		language = filename2attribute2info[treePath]["gitlab-language"]
-	}
-	if language == "unspecified" {
-		language = ""
-	}
-
-	return language, nil
+	attribute, err := gitRepo.GitAttributeFirst(commitID, treePath, "linguist-language", "gitlab-language")
+	return attribute.Prefix(), err
 }

services/repository/files/update.go

@@ -400,16 +400,12 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file
 	var lfsMetaObject *git_model.LFSMetaObject
 	if setting.LFS.StartServer && hasOldBranch {
 		// Check there is no way this can return multiple infos
-		filename2attribute2info, err := t.gitRepo.CheckAttribute(git.CheckAttributeOpts{
-			Attributes: []string{"filter"},
-			Filenames:  []string{file.Options.treePath},
-			CachedOnly: true,
-		})
+		filterAttribute, err := t.gitRepo.GitAttributeFirst("", file.Options.treePath, "filter")
 		if err != nil {
 			return err
 		}
 
-		if filename2attribute2info[file.Options.treePath] != nil && filename2attribute2info[file.Options.treePath]["filter"] == "lfs" {
+		if filterAttribute == "lfs" {
 			// OK so we are supposed to LFS this data!
 			pointer, err := lfs.GeneratePointer(treeObjectContentReader)
 			if err != nil {

services/repository/files/upload.go

@@ -105,24 +105,10 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
 		}
 	}
 
-	var filename2attribute2info map[string]map[string]string
-	if setting.LFS.StartServer {
-		filename2attribute2info, err = t.gitRepo.CheckAttribute(git.CheckAttributeOpts{
-			Attributes: []string{"filter"},
-			Filenames:  names,
-			CachedOnly: true,
-		})
-		if err != nil {
-			return err
-		}
-	}
-
 	// Copy uploaded files into repository.
-	for i := range infos {
-		if err := copyUploadedLFSFileIntoRepository(&infos[i], filename2attribute2info, t, opts.TreePath); err != nil {
+	if err := copyUploadedLFSFilesIntoRepository(infos, t, opts.TreePath); err != nil {
 		return err
 	}
-	}
 
 	// Now write the tree
 	treeHash, err := t.WriteTree()
@@ -169,7 +155,44 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
 	return repo_model.DeleteUploads(ctx, uploads...)
 }
 
-func copyUploadedLFSFileIntoRepository(info *uploadInfo, filename2attribute2info map[string]map[string]string, t *TemporaryUploadRepository, treePath string) error {
+func copyUploadedLFSFilesIntoRepository(infos []uploadInfo, t *TemporaryUploadRepository, treePath string) error {
+	var storeInLFSFunc func(string) (bool, error)
+
+	if setting.LFS.StartServer {
+		checker, err := t.gitRepo.GitAttributeChecker("", "filter")
+		if err != nil {
+			return err
+		}
+		defer checker.Close()
+
+		storeInLFSFunc = func(name string) (bool, error) {
+			attrs, err := checker.CheckPath(name)
+			if err != nil {
+				return false, fmt.Errorf("could not CheckPath(%s): %w", name, err)
+			}
+			return attrs["filter"] == "lfs", nil
+		}
+	}
+
+	// Copy uploaded files into repository.
+	for i, info := range infos {
+		storeInLFS := false
+		if storeInLFSFunc != nil {
+			var err error
+			storeInLFS, err = storeInLFSFunc(info.upload.Name)
+			if err != nil {
+				return err
+			}
+		}
+
+		if err := copyUploadedLFSFileIntoRepository(&infos[i], storeInLFS, t, treePath); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func copyUploadedLFSFileIntoRepository(info *uploadInfo, storeInLFS bool, t *TemporaryUploadRepository, treePath string) error {
 	file, err := os.Open(info.upload.LocalPath())
 	if err != nil {
 		return err
@@ -177,7 +200,7 @@ func copyUploadedLFSFileIntoRepository(info *uploadInfo, filename2attribute2info
 	defer file.Close()
 
 	var objectHash string
-	if setting.LFS.StartServer && filename2attribute2info[info.upload.Name] != nil && filename2attribute2info[info.upload.Name]["filter"] == "lfs" {
+	if storeInLFS {
 		// Handle LFS
 		// FIXME: Inefficient! this should probably happen in models.Upload
 		pointer, err := lfs.GeneratePointer(file)