From bb4f798c8827658f116a1b8262c053c1e6687442 Mon Sep 17 00:00:00 2001 From: bytedream Date: Sun, 4 May 2025 22:13:23 +0200 Subject: [PATCH 1/9] Use HEAD as tree-ish source for bare repo attributes --- services/repository/files/update.go | 2 +- services/repository/files/upload.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index fbf59c40edb97..abf6a4b5064f7 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -492,7 +492,7 @@ 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 - attributesMap, err := attribute.CheckAttributes(ctx, t.gitRepo, "" /* use temp repo's working dir */, attribute.CheckAttributeOpts{ + attributesMap, err := attribute.CheckAttributes(ctx, t.gitRepo, "HEAD", attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, Filenames: []string{file.Options.treePath}, }) diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index f348cb68ab543..4623efb157f91 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -108,7 +108,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use var attributesMap map[string]*attribute.Attributes if setting.LFS.StartServer { - attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "" /* use temp repo's working dir */, attribute.CheckAttributeOpts{ + attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "HEAD", attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, Filenames: names, }) From c4e5c8bcfd595a3d290166e7029b4c415edcd25e Mon Sep 17 00:00:00 2001 From: bytedream Date: Mon, 5 May 2025 01:29:45 +0200 Subject: [PATCH 2/9] Fix upload failing on empty repo --- services/repository/files/upload.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index 4623efb157f91..eaafde97b3ad0 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -107,7 +107,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } var attributesMap map[string]*attribute.Attributes - if setting.LFS.StartServer { + if setting.LFS.StartServer && hasOldBranch { attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "HEAD", attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, Filenames: names, From 90e10ea872d40c254a5bde61a6878660aaff938a Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 00:42:46 +0200 Subject: [PATCH 3/9] Add option to use cached flag --- modules/git/attribute/batch.go | 2 +- modules/git/attribute/checker.go | 7 +++++-- services/repository/files/update.go | 4 +++- services/repository/files/upload.go | 4 +++- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/modules/git/attribute/batch.go b/modules/git/attribute/batch.go index 4e31fda5753cd..1a71ef02ac586 100644 --- a/modules/git/attribute/batch.go +++ b/modules/git/attribute/batch.go @@ -36,7 +36,7 @@ func NewBatchChecker(repo *git.Repository, treeish string, attributes []string) } }() - cmd, envs, cleanup, err := checkAttrCommand(repo, treeish, nil, attributes) + cmd, envs, cleanup, err := checkAttrCommand(repo, treeish, nil, attributes, false) if err != nil { return nil, err } diff --git a/modules/git/attribute/checker.go b/modules/git/attribute/checker.go index c17006a15407b..d0fd2d59bdeca 100644 --- a/modules/git/attribute/checker.go +++ b/modules/git/attribute/checker.go @@ -13,7 +13,7 @@ import ( "code.gitea.io/gitea/modules/git" ) -func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attributes []string) (*git.Command, []string, func(), error) { +func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attributes []string, cached bool) (*git.Command, []string, func(), error) { cancel := func() {} envs := []string{"GIT_FLUSH=1"} cmd := git.NewCommand("check-attr", "-z") @@ -39,6 +39,8 @@ func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attrib ) cancel = deleteTemporaryFile } + } else if cached { + cmd.AddArguments("--cached") } // else: no treeish, assume it is a not a bare repo, read from working directory cmd.AddDynamicArguments(attributes...) @@ -51,12 +53,13 @@ func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attrib type CheckAttributeOpts struct { Filenames []string Attributes []string + Cached bool } // CheckAttributes return the attributes of the given filenames and attributes in the given treeish. // If treeish is empty, then it will use current working directory, otherwise it will use the provided treeish on the bare repo func CheckAttributes(ctx context.Context, gitRepo *git.Repository, treeish string, opts CheckAttributeOpts) (map[string]*Attributes, error) { - cmd, envs, cancel, err := checkAttrCommand(gitRepo, treeish, opts.Filenames, opts.Attributes) + cmd, envs, cancel, err := checkAttrCommand(gitRepo, treeish, opts.Filenames, opts.Attributes, opts.Cached) if err != nil { return nil, err } diff --git a/services/repository/files/update.go b/services/repository/files/update.go index abf6a4b5064f7..60c89c5baa8ca 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -492,9 +492,11 @@ 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 - attributesMap, err := attribute.CheckAttributes(ctx, t.gitRepo, "HEAD", attribute.CheckAttributeOpts{ + attributesMap, err := attribute.CheckAttributes(ctx, t.gitRepo, "", attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, Filenames: []string{file.Options.treePath}, + // An index is set, so it's okay to list the attributes from it + Cached: true, }) if err != nil { return err diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index eaafde97b3ad0..8d2bc2c320dc8 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -108,9 +108,11 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use var attributesMap map[string]*attribute.Attributes if setting.LFS.StartServer && hasOldBranch { - attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "HEAD", attribute.CheckAttributeOpts{ + attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "", attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, Filenames: names, + // An index is set, so it's okay to list the attributes from it + Cached: true, }) if err != nil { return err From 03de41f70cc0edd6fbd2eb25497f4decc7b88318 Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 02:23:03 +0200 Subject: [PATCH 4/9] Add comments --- modules/git/attribute/checker.go | 2 ++ services/repository/files/update.go | 2 +- services/repository/files/upload.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/git/attribute/checker.go b/modules/git/attribute/checker.go index d0fd2d59bdeca..3df986d7cbf80 100644 --- a/modules/git/attribute/checker.go +++ b/modules/git/attribute/checker.go @@ -40,6 +40,8 @@ func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attrib cancel = deleteTemporaryFile } } else if cached { + // read from existing index instead of working tree, in cases where the repo is bare or the working tree contains + // unstaged changes that shouldn't affect the attribute check cmd.AddArguments("--cached") } // else: no treeish, assume it is a not a bare repo, read from working directory diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 60c89c5baa8ca..52b7e0d842433 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -492,7 +492,7 @@ 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 - attributesMap, err := attribute.CheckAttributes(ctx, t.gitRepo, "", attribute.CheckAttributeOpts{ + attributesMap, err := attribute.CheckAttributes(ctx, t.gitRepo, "" /* use temp repo's working dir */, attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, Filenames: []string{file.Options.treePath}, // An index is set, so it's okay to list the attributes from it diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index 8d2bc2c320dc8..bbd53f44c8bbe 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -108,7 +108,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use var attributesMap map[string]*attribute.Attributes if setting.LFS.StartServer && hasOldBranch { - attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "", attribute.CheckAttributeOpts{ + attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "" /* use temp repo's working dir */, attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, Filenames: names, // An index is set, so it's okay to list the attributes from it From 5f94be683b6b8117a1c78c95ebbaae6db307750e Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 03:00:12 +0200 Subject: [PATCH 5/9] Add test --- modules/git/attribute/checker_test.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/modules/git/attribute/checker_test.go b/modules/git/attribute/checker_test.go index 97db43460bb81..fe8ccd80c287d 100644 --- a/modules/git/attribute/checker_test.go +++ b/modules/git/attribute/checker_test.go @@ -57,8 +57,19 @@ func Test_Checker(t *testing.T) { assert.Equal(t, expectedAttrs(), attrs["i-am-a-python.p"]) }) + t.Run("Run git check-attr in bare repository using index", func(t *testing.T) { + attrs, err := CheckAttributes(t.Context(), gitRepo, "", CheckAttributeOpts{ + Filenames: []string{"i-am-a-python.p"}, + Attributes: LinguistAttributes, + Cached: true, + }) + assert.NoError(t, err) + assert.Len(t, attrs, 1) + assert.Equal(t, expectedAttrs(), attrs["i-am-a-python.p"]) + }) + if !git.DefaultFeatures().SupportCheckAttrOnBare { - t.Skip("git version 2.40 is required to support run check-attr on bare repo") + t.Skip("git version 2.40 is required to support run check-attr on bare repo without using index") return } From de1131f1a1cf22fcb64270c0c49c9c113443db1b Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 03:06:22 +0200 Subject: [PATCH 6/9] Update comment --- modules/git/attribute/checker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/git/attribute/checker.go b/modules/git/attribute/checker.go index 3df986d7cbf80..a6cc771b62a58 100644 --- a/modules/git/attribute/checker.go +++ b/modules/git/attribute/checker.go @@ -40,8 +40,8 @@ func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attrib cancel = deleteTemporaryFile } } else if cached { - // read from existing index instead of working tree, in cases where the repo is bare or the working tree contains - // unstaged changes that shouldn't affect the attribute check + // read from existing index instead of work tree, in cases where the repo is bare and has an index or the work tree + // contains unstaged changes that shouldn't affect the attribute check cmd.AddArguments("--cached") } // else: no treeish, assume it is a not a bare repo, read from working directory From 74e55839bc2989deea01d14b0571b205dfb18647 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 8 May 2025 09:22:06 +0800 Subject: [PATCH 7/9] fix --- modules/git/attribute/batch.go | 2 +- modules/git/attribute/checker.go | 14 +++++++------- modules/git/attribute/checker_test.go | 1 - services/repository/files/update.go | 2 -- services/repository/files/upload.go | 4 +--- 5 files changed, 9 insertions(+), 14 deletions(-) diff --git a/modules/git/attribute/batch.go b/modules/git/attribute/batch.go index 1a71ef02ac586..4e31fda5753cd 100644 --- a/modules/git/attribute/batch.go +++ b/modules/git/attribute/batch.go @@ -36,7 +36,7 @@ func NewBatchChecker(repo *git.Repository, treeish string, attributes []string) } }() - cmd, envs, cleanup, err := checkAttrCommand(repo, treeish, nil, attributes, false) + cmd, envs, cleanup, err := checkAttrCommand(repo, treeish, nil, attributes) if err != nil { return nil, err } diff --git a/modules/git/attribute/checker.go b/modules/git/attribute/checker.go index a6cc771b62a58..167b31416e1fa 100644 --- a/modules/git/attribute/checker.go +++ b/modules/git/attribute/checker.go @@ -13,7 +13,7 @@ import ( "code.gitea.io/gitea/modules/git" ) -func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attributes []string, cached bool) (*git.Command, []string, func(), error) { +func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attributes []string) (*git.Command, []string, func(), error) { cancel := func() {} envs := []string{"GIT_FLUSH=1"} cmd := git.NewCommand("check-attr", "-z") @@ -39,11 +39,12 @@ func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attrib ) cancel = deleteTemporaryFile } - } else if cached { - // read from existing index instead of work tree, in cases where the repo is bare and has an index or the work tree - // contains unstaged changes that shouldn't affect the attribute check + } else { + // Read from existing index, in cases where the repo is bare and has an index, + // or the work tree contains unstaged changes that shouldn't affect the attribute check. + // It is caller's responsibility to add changed ".gitattributes" into the index if they want to respect the new changes. cmd.AddArguments("--cached") - } // else: no treeish, assume it is a not a bare repo, read from working directory + } cmd.AddDynamicArguments(attributes...) if len(filenames) > 0 { @@ -55,13 +56,12 @@ func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attrib type CheckAttributeOpts struct { Filenames []string Attributes []string - Cached bool } // CheckAttributes return the attributes of the given filenames and attributes in the given treeish. // If treeish is empty, then it will use current working directory, otherwise it will use the provided treeish on the bare repo func CheckAttributes(ctx context.Context, gitRepo *git.Repository, treeish string, opts CheckAttributeOpts) (map[string]*Attributes, error) { - cmd, envs, cancel, err := checkAttrCommand(gitRepo, treeish, opts.Filenames, opts.Attributes, opts.Cached) + cmd, envs, cancel, err := checkAttrCommand(gitRepo, treeish, opts.Filenames, opts.Attributes) if err != nil { return nil, err } diff --git a/modules/git/attribute/checker_test.go b/modules/git/attribute/checker_test.go index fe8ccd80c287d..67fbda8918800 100644 --- a/modules/git/attribute/checker_test.go +++ b/modules/git/attribute/checker_test.go @@ -61,7 +61,6 @@ func Test_Checker(t *testing.T) { attrs, err := CheckAttributes(t.Context(), gitRepo, "", CheckAttributeOpts{ Filenames: []string{"i-am-a-python.p"}, Attributes: LinguistAttributes, - Cached: true, }) assert.NoError(t, err) assert.Len(t, attrs, 1) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 52b7e0d842433..fbf59c40edb97 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -495,8 +495,6 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file attributesMap, err := attribute.CheckAttributes(ctx, t.gitRepo, "" /* use temp repo's working dir */, attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, Filenames: []string{file.Options.treePath}, - // An index is set, so it's okay to list the attributes from it - Cached: true, }) if err != nil { return err diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index bbd53f44c8bbe..f348cb68ab543 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -107,12 +107,10 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } var attributesMap map[string]*attribute.Attributes - if setting.LFS.StartServer && hasOldBranch { + if setting.LFS.StartServer { attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "" /* use temp repo's working dir */, attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, Filenames: names, - // An index is set, so it's okay to list the attributes from it - Cached: true, }) if err != nil { return err From ce7cfe81dec1190ee355563b46c834530ed6abf5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 8 May 2025 09:44:32 +0800 Subject: [PATCH 8/9] add comment --- services/repository/files/upload.go | 1 + 1 file changed, 1 insertion(+) diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index f348cb68ab543..f3cfbcc8f0401 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -107,6 +107,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } var attributesMap map[string]*attribute.Attributes + // when uploading to an empty repo, the old branch doesn't exist, but some "global gitattributes" or "info/attributes" may exist if setting.LFS.StartServer { attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "" /* use temp repo's working dir */, attribute.CheckAttributeOpts{ Attributes: []string{attribute.Filter}, From b5ddcbc2efaccf8d78b8bdd0781819efc0fd472a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 8 May 2025 09:49:09 +0800 Subject: [PATCH 9/9] add more comments --- services/repository/files/upload.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index f3cfbcc8f0401..68a071cd2814a 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -119,6 +119,12 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } // Copy uploaded files into repository. + // TODO: there is a small problem: when uploading LFS files with ".gitattributes", the "check-attr" runs before this loop, + // so LFS files are not able to be added as LFS objects. Ideally we need to do in 3 steps in the future: + // 1. Add ".gitattributes" to git index + // 2. Run "check-attr" (the previous attribute.CheckAttributes call) + // 3. Add files to git index (this loop) + // This problem is trivial so maybe no need to spend too much time on it at the moment. for i := range infos { if err := copyUploadedLFSFileIntoRepository(ctx, &infos[i], attributesMap, t, opts.TreePath); err != nil { return err