Skip to content

Fix LFS file not stored in LFS when uploaded/edited via API or web UI #34367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion modules/git/attribute/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ func checkAttrCommand(gitRepo *git.Repository, treeish string, filenames, attrib
)
cancel = deleteTemporaryFile
}
} // else: no treeish, assume it is a not a bare repo, read from working directory
} 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")
}

cmd.AddDynamicArguments(attributes...)
if len(filenames) > 0 {
Expand Down
12 changes: 11 additions & 1 deletion modules/git/attribute/checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,18 @@ 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,
})
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
}

Expand Down
7 changes: 7 additions & 0 deletions services/repository/files/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -118,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
Expand Down