Skip to content

Rename/move binary and LFS files in web UI #34350

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

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
c6598c2
Enable path editing of non text files
bytedream May 2, 2025
f530856
Move lfs cannot edit context data
bytedream May 2, 2025
7684e17
Do not read file if it is too large
bytedream May 2, 2025
dcaadb8
Remove duplicated context data
bytedream May 3, 2025
cf44789
Fix new file not being editable
bytedream May 3, 2025
96bbf81
Merge branch 'main' into non-text-edit
bytedream May 4, 2025
5dc082a
Make LFS file paths editable
bytedream May 5, 2025
349d5b1
Do not show editor for locked lfs file by other user
bytedream May 5, 2025
07845a2
Unify locale keys
bytedream May 5, 2025
6f3b69b
Fix empty binary content after move
bytedream May 5, 2025
e960fe3
Fix file content only read on new file
bytedream May 5, 2025
02fe2db
Revert unnecessary locale key rename
bytedream May 5, 2025
2698321
Syntax and comment changes
bytedream May 5, 2025
691073d
Typo fix
bytedream May 5, 2025
2848fdf
Merge branch 'main' into non-text-edit
bytedream May 6, 2025
1efe0e7
Close data blobs
bytedream May 6, 2025
4d17f51
Remove unnecessary entry blob open
bytedream May 6, 2025
3d64a84
Update lfs pointer checks
bytedream May 6, 2025
9811c28
Merge branch 'main' into non-text-edit
bytedream May 8, 2025
3ad1e7e
Enable commit button only if something has changed
bytedream May 8, 2025
fb3e801
Update comments and names
bytedream May 8, 2025
614a4b3
Update
bytedream May 8, 2025
2c9c9e1
Update comment
bytedream May 8, 2025
07836d8
Refactor
bytedream May 8, 2025
0d27705
Make linter happy
bytedream May 8, 2025
3348c04
Show binary and lfs edit button in pull request changes
bytedream May 8, 2025
7133834
Additionally check if path changed to trigger pure rename operation
bytedream May 8, 2025
8739ebe
Update internal function names
bytedream May 8, 2025
2c2a7c5
Add rename test
bytedream May 8, 2025
627bce9
Format
bytedream May 8, 2025
4e8a8e4
Add copyright
bytedream May 8, 2025
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
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,9 @@ editor.upload_file = Upload File
editor.edit_file = Edit File
editor.preview_changes = Preview Changes
editor.cannot_edit_lfs_files = LFS files cannot be edited in the web interface.
editor.cannot_edit_too_large_file = The file is too large to be edited.
editor.cannot_edit_non_text_files = Binary files cannot be edited in the web interface.
editor.file_not_editable_hint = But you can still rename or move it.
editor.edit_this_file = Edit File
editor.this_file_locked = File is locked
editor.must_be_on_a_branch = You must be on a branch to make or propose changes to this file.
Expand Down
51 changes: 36 additions & 15 deletions routers/web/repo/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,6 @@ func editFile(ctx *context.Context, isNewFile bool) {
}

blob := entry.Blob()
if blob.Size() >= setting.UI.MaxDisplayFileSize {
ctx.NotFound(err)
return
}

buf, dataRc, fInfo, err := getFileReader(ctx, ctx.Repo.Repository.ID, blob)
if err != nil {
Expand All @@ -162,26 +158,47 @@ func editFile(ctx *context.Context, isNewFile bool) {

defer dataRc.Close()

if fInfo.isLFSFile {
lfsLock, err := git_model.GetTreePathLock(ctx, ctx.Repo.Repository.ID, ctx.Repo.TreePath)
if err != nil {
ctx.ServerError("GetTreePathLock", err)
return
}
if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID {
ctx.NotFound(nil)
return
}
}

ctx.Data["FileSize"] = blob.Size()

// Only some file types are editable online as text.
if !fInfo.st.IsRepresentableAsText() || fInfo.isLFSFile {
ctx.NotFound(nil)
return
}
ctx.Data["IsFileEditable"] = fInfo.st.IsRepresentableAsText() && !fInfo.isLFSFile && blob.Size() < setting.UI.MaxDisplayFileSize

d, _ := io.ReadAll(dataRc)
if fInfo.isLFSFile {
ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_lfs_files")
} else if !fInfo.isTextFile {
ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_non_text_files")
}

buf = append(buf, d...)
if content, err := charset.ToUTF8(buf, charset.ConvertOpts{KeepBOM: true}); err != nil {
log.Error("ToUTF8: %v", err)
ctx.Data["FileContent"] = string(buf)
if blob.Size() >= setting.UI.MaxDisplayFileSize {
ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_too_large_file")
} else {
ctx.Data["FileContent"] = content
d, _ := io.ReadAll(dataRc)

buf = append(buf, d...)
if content, err := charset.ToUTF8(buf, charset.ConvertOpts{KeepBOM: true}); err != nil {
log.Error("ToUTF8: %v", err)
ctx.Data["FileContent"] = string(buf)
} else {
ctx.Data["FileContent"] = content
}
}
} else {
// Append filename from query, or empty string to allow username the new file.
treeNames = append(treeNames, fileName)

ctx.Data["IsFileEditable"] = true
}

ctx.Data["TreeNames"] = treeNames
Expand Down Expand Up @@ -280,6 +297,10 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
operation := "update"
if isNewFile {
operation = "create"
} else if !form.Content.Has() && ctx.Repo.TreePath != form.TreePath {
// The form content only has data if file is representable as text, is not too large and not in lfs. If it doesn't
// have data, the only possible operation is a rename
operation = "rename"
}

if _, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.ChangeRepoFilesOptions{
Expand All @@ -292,7 +313,7 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
Operation: operation,
FromTreePath: ctx.Repo.TreePath,
TreePath: form.TreePath,
ContentReader: strings.NewReader(strings.ReplaceAll(form.Content, "\r", "")),
ContentReader: strings.NewReader(strings.ReplaceAll(form.Content.Value(), "\r", "")),
},
},
Signoff: form.Signoff,
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func NewDiffPatchPost(ctx *context.Context) {
OldBranch: ctx.Repo.BranchName,
NewBranch: branchName,
Message: message,
Content: strings.ReplaceAll(form.Content, "\r", ""),
Content: strings.ReplaceAll(form.Content.Value(), "\r", ""),
Author: gitCommitter,
Committer: gitCommitter,
})
Expand Down
28 changes: 6 additions & 22 deletions routers/web/repo/view_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,6 @@ func prepareToRenderFile(ctx *context.Context, entry *git.TreeEntry) {
ctx.Data["LFSLockHint"] = ctx.Tr("repo.editor.this_file_locked")
}

// Assume file is not editable first.
if fInfo.isLFSFile {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.cannot_edit_lfs_files")
} else if !isRepresentableAsText {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.cannot_edit_non_text_files")
}

// read all needed attributes which will be used later
// there should be no performance different between reading 2 or 4 here
attrsMap, err := attribute.CheckAttributes(ctx, ctx.Repo.GitRepo, ctx.Repo.CommitID, attribute.CheckAttributeOpts{
Expand Down Expand Up @@ -243,21 +236,6 @@ func prepareToRenderFile(ctx *context.Context, entry *git.TreeEntry) {
ctx.Data["FileContent"] = fileContent
ctx.Data["LineEscapeStatus"] = statuses
}
if !fInfo.isLFSFile {
if ctx.Repo.CanEnableEditor(ctx, ctx.Doer) {
if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID {
ctx.Data["CanEditFile"] = false
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.this_file_locked")
} else {
ctx.Data["CanEditFile"] = true
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file")
}
} else if !ctx.Repo.RefFullName.IsBranch() {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch")
} else if !ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.BranchName) {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.fork_before_edit")
}
}

case fInfo.st.IsPDF():
ctx.Data["IsPDFFile"] = true
Expand Down Expand Up @@ -309,15 +287,21 @@ func prepareToRenderFile(ctx *context.Context, entry *git.TreeEntry) {

if ctx.Repo.CanEnableEditor(ctx, ctx.Doer) {
if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID {
ctx.Data["CanEditFile"] = false
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.this_file_locked")
ctx.Data["CanDeleteFile"] = false
ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.this_file_locked")
} else {
ctx.Data["CanEditFile"] = true
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file")
ctx.Data["CanDeleteFile"] = true
ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.delete_this_file")
}
} else if !ctx.Repo.RefFullName.IsBranch() {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch")
ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch")
} else if !ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.BranchName) {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.fork_before_edit")
ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_have_write_access")
}
}
3 changes: 2 additions & 1 deletion services/forms/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

issues_model "code.gitea.io/gitea/models/issues"
project_model "code.gitea.io/gitea/models/project"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/web/middleware"
"code.gitea.io/gitea/services/context"
Expand Down Expand Up @@ -689,7 +690,7 @@ func (f *NewWikiForm) Validate(req *http.Request, errs binding.Errors) binding.E
// EditRepoFileForm form for changing repository file
type EditRepoFileForm struct {
TreePath string `binding:"Required;MaxSize(500)"`
Content string
Content optional.Option[string]
CommitSummary string `binding:"MaxSize(100)"`
CommitMessage string
CommitChoice string `binding:"Required;MaxSize(50)"`
Expand Down
153 changes: 128 additions & 25 deletions services/repository/files/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
contentStore := lfs.NewContentStore()
for _, file := range opts.Files {
switch file.Operation {
case "create", "update":
case "create", "update", "rename":
if err := CreateOrUpdateFile(ctx, t, file, contentStore, repo.ID, hasOldBranch); err != nil {
return nil, err
}
Expand Down Expand Up @@ -488,31 +488,32 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file
}
}

treeObjectContentReader := file.ContentReader
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{
Attributes: []string{attribute.Filter},
Filenames: []string{file.Options.treePath},
})
var oldEntry *git.TreeEntry
// Assume that the file.ContentReader of a pure rename operation is invalid. Use the file content how it's present in
// git instead
if file.Operation == "rename" {
lastCommitID, err := t.GetLastCommit(ctx)
if err != nil {
return err
}
commit, err := t.GetCommit(lastCommitID)
if err != nil {
return err
}

if attributesMap[file.Options.treePath] != nil && attributesMap[file.Options.treePath].Get(attribute.Filter).ToString().Value() == "lfs" {
// OK so we are supposed to LFS this data!
pointer, err := lfs.GeneratePointer(treeObjectContentReader)
if err != nil {
return err
}
lfsMetaObject = &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repoID}
treeObjectContentReader = strings.NewReader(pointer.StringContent())
if oldEntry, err = commit.GetTreeEntryByPath(file.Options.fromTreePath); err != nil {
return err
}
}

// Add the object to the database
objectHash, err := t.HashObject(ctx, treeObjectContentReader)
var objectHash string
var lfsPointer *lfs.Pointer
switch file.Operation {
case "create", "update":
objectHash, lfsPointer, err = createOrUpdateFileHash(ctx, t, file, hasOldBranch)
case "rename":
objectHash, lfsPointer, err = renameFileHash(ctx, t, oldEntry, file)
}
if err != nil {
return err
}
Expand All @@ -528,9 +529,9 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file
}
}

if lfsMetaObject != nil {
if lfsPointer != nil {
// We have an LFS object - create it
lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, lfsMetaObject.RepositoryID, lfsMetaObject.Pointer)
lfsMetaObject, err := git_model.NewLFSMetaObject(ctx, repoID, *lfsPointer)
if err != nil {
return err
}
Expand All @@ -539,11 +540,20 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file
return err
}
if !exist {
_, err := file.ContentReader.Seek(0, io.SeekStart)
if err != nil {
return err
var lfsContentReader io.Reader
if file.Operation != "rename" {
if _, err := file.ContentReader.Seek(0, io.SeekStart); err != nil {
return err
}
lfsContentReader = file.ContentReader
} else {
if lfsContentReader, err = oldEntry.Blob().DataAsync(); err != nil {
return err
}
defer lfsContentReader.(io.ReadCloser).Close()
}
if err := contentStore.Put(lfsMetaObject.Pointer, file.ContentReader); err != nil {

if err := contentStore.Put(lfsMetaObject.Pointer, lfsContentReader); err != nil {
if _, err2 := git_model.RemoveLFSMetaObjectByOid(ctx, repoID, lfsMetaObject.Oid); err2 != nil {
return fmt.Errorf("unable to remove failed inserted LFS object %s: %v (Prev Error: %w)", lfsMetaObject.Oid, err2, err)
}
Expand All @@ -555,6 +565,99 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file
return nil
}

func createOrUpdateFileHash(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile, hasOldBranch bool) (string, *lfs.Pointer, error) {
treeObjectContentReader := file.ContentReader
var lfsPointer *lfs.Pointer
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{
Attributes: []string{attribute.Filter},
Filenames: []string{file.Options.treePath},
})
if err != nil {
return "", nil, err
}

if attributesMap[file.Options.treePath] != nil && attributesMap[file.Options.treePath].Get(attribute.Filter).ToString().Value() == "lfs" {
// OK so we are supposed to LFS this data!
pointer, err := lfs.GeneratePointer(treeObjectContentReader)
if err != nil {
return "", nil, err
}
lfsPointer = &pointer
treeObjectContentReader = strings.NewReader(pointer.StringContent())
}
}

// Add the object to the database
objectHash, err := t.HashObject(ctx, treeObjectContentReader)
if err != nil {
return "", nil, err
}

return objectHash, lfsPointer, nil
}

func renameFileHash(ctx context.Context, t *TemporaryUploadRepository, oldEntry *git.TreeEntry, file *ChangeRepoFile) (string, *lfs.Pointer, error) {
if setting.LFS.StartServer {
attributesMap, err := attribute.CheckAttributes(ctx, t.gitRepo, "" /* use temp repo's working dir */, attribute.CheckAttributeOpts{
Attributes: []string{attribute.Filter},
Filenames: []string{file.Options.treePath, file.Options.fromTreePath},
})
if err != nil {
return "", nil, err
}

oldIsLfs := attributesMap[file.Options.fromTreePath] != nil && attributesMap[file.Options.fromTreePath].Get(attribute.Filter).ToString().Value() == "lfs"
newIsLfs := attributesMap[file.Options.treePath] != nil && attributesMap[file.Options.treePath].Get(attribute.Filter).ToString().Value() == "lfs"

// If the old and new paths are both in lfs or both not in lfs, the object hash of the old file can be used directly
// as the object doesn't change
if oldIsLfs == newIsLfs {
return oldEntry.ID.String(), nil, nil
}

oldEntryReader, err := oldEntry.Blob().DataAsync()
if err != nil {
return "", nil, err
}
defer oldEntryReader.Close()

var treeObjectContentReader io.Reader
var lfsPointer *lfs.Pointer
// If the old path is in lfs but the new isn't, read the content from lfs and add it as normal git object
// If the new path is in lfs but the old isn't, read the content from the git object and generate a lfs
// pointer of it
if oldIsLfs {
pointer, err := lfs.ReadPointer(oldEntryReader)
if err != nil {
return "", nil, err
}
treeObjectContentReader, err = lfs.ReadMetaObject(pointer)
if err != nil {
return "", nil, err
}
defer treeObjectContentReader.(io.ReadCloser).Close()
} else {
pointer, err := lfs.GeneratePointer(oldEntryReader)
if err != nil {
return "", nil, err
}
treeObjectContentReader = strings.NewReader(pointer.StringContent())
lfsPointer = &pointer
}

// Add the object to the database
objectID, err := t.HashObject(ctx, treeObjectContentReader)
if err != nil {
return "", nil, err
}
return objectID, lfsPointer, nil
}

return oldEntry.ID.String(), nil, nil
}

// VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch
func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName string, treePaths []string) error {
protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName)
Expand Down
Loading