From c6598c27d3b8b90f041bc83110a517f4799b7823 Mon Sep 17 00:00:00 2001 From: bytedream Date: Fri, 2 May 2025 23:22:12 +0200 Subject: [PATCH 01/30] Enable path editing of non text files --- options/locale/locale_en-US.ini | 4 ++- routers/web/repo/editor.go | 11 ++----- routers/web/repo/view_file.go | 33 ++++++++++--------- templates/repo/editor/edit.tmpl | 56 +++++++++++++++++++++------------ 4 files changed, 57 insertions(+), 47 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a8fabc9ca1014..029ffc4c6f8ba 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1331,7 +1331,6 @@ 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_non_text_files = Binary files cannot be edited in the web interface. 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. @@ -1395,6 +1394,9 @@ editor.user_no_push_to_branch = User cannot push to branch editor.require_signed_commit = Branch requires a signed commit editor.cherry_pick = Cherry-pick %s onto: editor.revert = Revert %s onto: +editor.file_too_large_not_editable = The file is too large to be edited. +editor.binary_file_not_editable = Binary file content is not editable. +editor.file_not_editable_hint = But you can still rename or move it. commits.desc = Browse source code change history. commits.commits = Commits diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index c925b6115147a..81c2cd8e69764 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -146,11 +146,6 @@ func editFile(ctx *context.Context, isNewFile bool) { } blob := entry.Blob() - if blob.Size() >= setting.UI.MaxDisplayFileSize { - ctx.NotFound(err) - return - } - dataRc, err := blob.DataAsync() if err != nil { ctx.NotFound(err) @@ -159,6 +154,7 @@ func editFile(ctx *context.Context, isNewFile bool) { defer dataRc.Close() + ctx.Data["IsFileTooLarge"] = blob.Size() >= setting.UI.MaxDisplayFileSize ctx.Data["FileSize"] = blob.Size() buf := make([]byte, 1024) @@ -166,10 +162,7 @@ func editFile(ctx *context.Context, isNewFile bool) { buf = buf[:n] // Only some file types are editable online as text. - if !typesniffer.DetectContentType(buf).IsRepresentableAsText() { - ctx.NotFound(nil) - return - } + ctx.Data["IsFileText"] = typesniffer.DetectContentType(buf).IsRepresentableAsText() d, _ := io.ReadAll(dataRc) diff --git a/routers/web/repo/view_file.go b/routers/web/repo/view_file.go index 3df6051975bc4..009aa6b505e2c 100644 --- a/routers/web/repo/view_file.go +++ b/routers/web/repo/view_file.go @@ -143,8 +143,6 @@ func prepareToRenderFile(ctx *context.Context, entry *git.TreeEntry) { // 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 @@ -243,21 +241,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 @@ -307,6 +290,22 @@ func prepareToRenderFile(ctx *context.Context, entry *git.TreeEntry) { } } + 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") + } + } + if ctx.Repo.CanEnableEditor(ctx, ctx.Doer) { if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID { ctx.Data["CanDeleteFile"] = false diff --git a/templates/repo/editor/edit.tmpl b/templates/repo/editor/edit.tmpl index ae8a60c20c116..9a59fdac70b40 100644 --- a/templates/repo/editor/edit.tmpl +++ b/templates/repo/editor/edit.tmpl @@ -28,31 +28,47 @@ -
-
- - {{if .IsFileText}} + {{if .IsFileEditable}}
- {{else if .IsFileTooLarge}} -
-
-

{{ctx.Locale.Tr "repo.editor.file_too_large_not_editable"}}

-

{{ctx.Locale.Tr "repo.editor.file_not_editable_hint"}}

-
-
{{else}}
-

{{ctx.Locale.Tr "repo.editor.binary_file_not_editable"}}

+

{{.NotEditableReason}}

{{ctx.Locale.Tr "repo.editor.file_not_editable_hint"}}

From 349d5b159751da77181256b42ea5c3687c5d0625 Mon Sep 17 00:00:00 2001 From: bytedream Date: Mon, 5 May 2025 02:07:01 +0200 Subject: [PATCH 07/30] Do not show editor for locked lfs file by other user --- routers/web/repo/editor.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 8783b3850ceff..ee43037bd1a4c 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -162,6 +162,18 @@ 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. From 07845a262db5821b64e2b88dd4824f485a151757 Mon Sep 17 00:00:00 2001 From: bytedream Date: Mon, 5 May 2025 02:14:30 +0200 Subject: [PATCH 08/30] Unify locale keys --- options/locale/locale_en-US.ini | 6 +++--- routers/web/repo/editor.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index c1c6903a44c3b..b0a2148a91137 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1330,6 +1330,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_binary_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. @@ -1393,9 +1396,6 @@ editor.user_no_push_to_branch = User cannot push to branch editor.require_signed_commit = Branch requires a signed commit editor.cherry_pick = Cherry-pick %s onto: editor.revert = Revert %s onto: -editor.file_too_large_not_editable = The file is too large to be edited. -editor.binary_file_not_editable = Binary file content is not editable. -editor.file_not_editable_hint = But you can still rename or move it. commits.desc = Browse source code change history. commits.commits = Commits diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index ee43037bd1a4c..70bbcd996578d 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -182,11 +182,11 @@ func editFile(ctx *context.Context, isNewFile bool) { 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.binary_file_not_editable") + ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_binary_files") } if blob.Size() >= setting.UI.MaxDisplayFileSize { - ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.file_too_large_not_editable") + ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_too_large_file") } else { d, _ := io.ReadAll(dataRc) From 6f3b69b900fed5d2b012810ae30afd2a6a6953b3 Mon Sep 17 00:00:00 2001 From: bytedream Date: Mon, 5 May 2025 17:10:08 +0200 Subject: [PATCH 09/30] Fix empty binary content after move --- routers/web/repo/editor.go | 11 ++-- routers/web/repo/patch.go | 2 +- services/forms/repo_form.go | 3 +- services/repository/files/update.go | 80 +++++++++++++++++++++++++---- 4 files changed, 78 insertions(+), 18 deletions(-) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 70bbcd996578d..025d29f64c934 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -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 { @@ -303,6 +299,11 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b operation = "create" } + var contentReader io.ReadSeeker + if isNewFile && form.Content.Has() { + contentReader = strings.NewReader(strings.ReplaceAll(form.Content.Value(), "\r", "")) + } + if _, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.ChangeRepoFilesOptions{ LastCommitID: form.LastCommit, OldBranch: ctx.Repo.BranchName, @@ -313,7 +314,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: contentReader, }, }, Signoff: form.Signoff, diff --git a/routers/web/repo/patch.go b/routers/web/repo/patch.go index ca346b7e6c313..3ffd8f89c4e20 100644 --- a/routers/web/repo/patch.go +++ b/routers/web/repo/patch.go @@ -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, }) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index a2827e516a5d8..d11ad0a54c9ec 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -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" @@ -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)"` diff --git a/services/repository/files/update.go b/services/repository/files/update.go index fbf59c40edb97..6b1555fb9aea4 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -43,7 +43,7 @@ type ChangeRepoFile struct { Operation string TreePath string FromTreePath string - ContentReader io.ReadSeeker + ContentReader io.ReadSeeker // nil if the operation is a pure rename SHA string Options *RepoFileOptions } @@ -461,6 +461,11 @@ func handleCheckErrors(file *ChangeRepoFile, commit *git.Commit, opts *ChangeRep // CreateOrUpdateFile handles creating or updating a file for ChangeRepoFiles func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile, contentStore *lfs.ContentStore, repoID int64, hasOldBranch bool) error { + // ContentReader is only allowed to be nil if the file is moving + if file.ContentReader == nil && file.TreePath == file.FromTreePath { + return nil + } + // Get the two paths (might be the same if not moving) from the index if they exist filesInIndex, err := t.LsFiles(ctx, file.TreePath, file.FromTreePath) if err != nil { @@ -488,26 +493,69 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file } } - treeObjectContentReader := file.ContentReader + var treeObjectContentReader io.Reader = file.ContentReader + var oldEntry *git.TreeEntry + // If nil, use the existing content + if file.ContentReader == nil { + lastCommit, _ := t.GetLastCommit(ctx) + commit, err := t.GetCommit(lastCommit) + if err != nil { + return err + } + + oldEntry, err = commit.GetTreeEntryByPath(file.Options.fromTreePath) + if err != nil { + return err + } + + treeObjectContentReader, err = oldEntry.Blob().DataAsync() + if err != nil { + return err + } + } + 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}, + Filenames: []string{file.Options.treePath, file.Options.fromTreePath}, }) 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) + var pointer *lfs.Pointer + // Get existing lfs pointer if the old tree path is in lfs + if oldEntry != nil && attributesMap[file.Options.fromTreePath] != nil && attributesMap[file.Options.fromTreePath].Get(attribute.Filter).ToString().Value() == "lfs" { + pointerReader, err := oldEntry.Blob().DataAsync() + if err != nil { + return err + } + p, err := lfs.ReadPointer(pointerReader) if err != nil { return err } - lfsMetaObject = &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repoID} + pointer = &p + } + + if attributesMap[file.Options.treePath] != nil && attributesMap[file.Options.treePath].Get(attribute.Filter).ToString().Value() == "lfs" { + // Only generate a new lfs pointer if the old tree path isn't in lfs or the object content is changed + if pointer == nil { + p, err := lfs.GeneratePointer(treeObjectContentReader) + if err != nil { + return err + } + pointer = &p + } + + lfsMetaObject = &git_model.LFSMetaObject{Pointer: *pointer, RepositoryID: repoID} treeObjectContentReader = strings.NewReader(pointer.StringContent()) + } else if pointer != nil { // old tree path was in lfs, new is not + treeObjectContentReader, err = lfs.ReadMetaObject(*pointer) + if err != nil { + return err + } } } @@ -539,11 +587,21 @@ 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.ContentReader != nil { + _, err := file.ContentReader.Seek(0, io.SeekStart) + if err != nil { + return err + } + lfsContentReader = file.ContentReader + } else { + lfsContentReader, err = oldEntry.Blob().DataAsync() + if err != nil { + return err + } } - 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) } From e960fe34c58327fb0d302005578250f14dc29c4d Mon Sep 17 00:00:00 2001 From: bytedream Date: Mon, 5 May 2025 18:55:47 +0200 Subject: [PATCH 10/30] Fix file content only read on new file --- routers/web/repo/editor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 025d29f64c934..e1a48818e59ea 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -300,7 +300,8 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b } var contentReader io.ReadSeeker - if isNewFile && form.Content.Has() { + // form content only has data if file is representable as text, is not too large and not in lfs + if isNewFile || form.Content.Has() { contentReader = strings.NewReader(strings.ReplaceAll(form.Content.Value(), "\r", "")) } From 02fe2dbe37a911c0f31038672939429f5914a016 Mon Sep 17 00:00:00 2001 From: bytedream Date: Mon, 5 May 2025 20:43:20 +0200 Subject: [PATCH 11/30] Revert unnecessary locale key rename --- options/locale/locale_en-US.ini | 2 +- routers/web/repo/editor.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b0a2148a91137..6d2666c89af67 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1331,7 +1331,7 @@ 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_binary_files = Binary files cannot be edited in the web interface. +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 diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index e1a48818e59ea..4a2242065f13d 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -178,7 +178,7 @@ func editFile(ctx *context.Context, isNewFile bool) { 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_binary_files") + ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_non_text_files") } if blob.Size() >= setting.UI.MaxDisplayFileSize { From 2698321eb1484730de5c5cbea34038f4f24de73e Mon Sep 17 00:00:00 2001 From: bytedream Date: Mon, 5 May 2025 21:12:29 +0200 Subject: [PATCH 12/30] Syntax and comment changes --- services/repository/files/update.go | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 6b1555fb9aea4..9ef0310fbc511 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -461,11 +461,6 @@ func handleCheckErrors(file *ChangeRepoFile, commit *git.Commit, opts *ChangeRep // CreateOrUpdateFile handles creating or updating a file for ChangeRepoFiles func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile, contentStore *lfs.ContentStore, repoID int64, hasOldBranch bool) error { - // ContentReader is only allowed to be nil if the file is moving - if file.ContentReader == nil && file.TreePath == file.FromTreePath { - return nil - } - // Get the two paths (might be the same if not moving) from the index if they exist filesInIndex, err := t.LsFiles(ctx, file.TreePath, file.FromTreePath) if err != nil { @@ -495,21 +490,21 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file var treeObjectContentReader io.Reader = file.ContentReader var oldEntry *git.TreeEntry - // If nil, use the existing content + // If no new content should be commited, use the file from the last commit as content if file.ContentReader == nil { - lastCommit, _ := t.GetLastCommit(ctx) - commit, err := t.GetCommit(lastCommit) + lastCommit, err := t.GetLastCommit(ctx) if err != nil { return err } - - oldEntry, err = commit.GetTreeEntryByPath(file.Options.fromTreePath) + commit, err := t.GetCommit(lastCommit) if err != nil { return err } - treeObjectContentReader, err = oldEntry.Blob().DataAsync() - if err != nil { + if oldEntry, err = commit.GetTreeEntryByPath(file.Options.fromTreePath); err != nil { + return err + } + if treeObjectContentReader, err = oldEntry.Blob().DataAsync(); err != nil { return err } } @@ -526,7 +521,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file } var pointer *lfs.Pointer - // Get existing lfs pointer if the old tree path is in lfs + // Get existing lfs pointer if the old path is in lfs if oldEntry != nil && attributesMap[file.Options.fromTreePath] != nil && attributesMap[file.Options.fromTreePath].Get(attribute.Filter).ToString().Value() == "lfs" { pointerReader, err := oldEntry.Blob().DataAsync() if err != nil { @@ -540,7 +535,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file } if attributesMap[file.Options.treePath] != nil && attributesMap[file.Options.treePath].Get(attribute.Filter).ToString().Value() == "lfs" { - // Only generate a new lfs pointer if the old tree path isn't in lfs or the object content is changed + // Only generate a new lfs pointer if the old path isn't in lfs or the object content changes if pointer == nil { p, err := lfs.GeneratePointer(treeObjectContentReader) if err != nil { @@ -589,14 +584,12 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file if !exist { var lfsContentReader io.Reader if file.ContentReader != nil { - _, err := file.ContentReader.Seek(0, io.SeekStart) - if err != nil { + if _, err := file.ContentReader.Seek(0, io.SeekStart); err != nil { return err } lfsContentReader = file.ContentReader } else { - lfsContentReader, err = oldEntry.Blob().DataAsync() - if err != nil { + if lfsContentReader, err = oldEntry.Blob().DataAsync(); err != nil { return err } } From 691073d73636a8b8a75f251c46b9bd256d550743 Mon Sep 17 00:00:00 2001 From: bytedream Date: Mon, 5 May 2025 21:17:24 +0200 Subject: [PATCH 13/30] Typo fix --- services/repository/files/update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 9ef0310fbc511..da789530fe37b 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -490,7 +490,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file var treeObjectContentReader io.Reader = file.ContentReader var oldEntry *git.TreeEntry - // If no new content should be commited, use the file from the last commit as content + // If no new content should be committed, use the file from the last commit as content if file.ContentReader == nil { lastCommit, err := t.GetLastCommit(ctx) if err != nil { From 1efe0e7ea7c004c78b56a2bdaa911838f9eb8c3d Mon Sep 17 00:00:00 2001 From: bytedream Date: Tue, 6 May 2025 13:29:34 +0200 Subject: [PATCH 14/30] Close data blobs --- services/repository/files/update.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index da789530fe37b..8b751303538a6 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -490,7 +490,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file var treeObjectContentReader io.Reader = file.ContentReader var oldEntry *git.TreeEntry - // If no new content should be committed, use the file from the last commit as content + // If no new content is committed, use the file from the last commit as content if file.ContentReader == nil { lastCommit, err := t.GetLastCommit(ctx) if err != nil { @@ -507,6 +507,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file if treeObjectContentReader, err = oldEntry.Blob().DataAsync(); err != nil { return err } + defer treeObjectContentReader.(io.ReadCloser).Close() } var lfsMetaObject *git_model.LFSMetaObject @@ -535,7 +536,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file } if attributesMap[file.Options.treePath] != nil && attributesMap[file.Options.treePath].Get(attribute.Filter).ToString().Value() == "lfs" { - // Only generate a new lfs pointer if the old path isn't in lfs or the object content changes + // Only generate a new lfs pointer if the old path isn't in lfs if pointer == nil { p, err := lfs.GeneratePointer(treeObjectContentReader) if err != nil { @@ -592,6 +593,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file if lfsContentReader, err = oldEntry.Blob().DataAsync(); err != nil { return err } + defer lfsContentReader.(io.ReadCloser).Close() } if err := contentStore.Put(lfsMetaObject.Pointer, lfsContentReader); err != nil { From 4d17f51417386d62016f844575ec99a66d78d8e7 Mon Sep 17 00:00:00 2001 From: bytedream Date: Tue, 6 May 2025 14:02:03 +0200 Subject: [PATCH 15/30] Remove unnecessary entry blob open --- services/repository/files/update.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 8b751303538a6..6e88c1a8cf50e 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -524,11 +524,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file var pointer *lfs.Pointer // Get existing lfs pointer if the old path is in lfs if oldEntry != nil && attributesMap[file.Options.fromTreePath] != nil && attributesMap[file.Options.fromTreePath].Get(attribute.Filter).ToString().Value() == "lfs" { - pointerReader, err := oldEntry.Blob().DataAsync() - if err != nil { - return err - } - p, err := lfs.ReadPointer(pointerReader) + p, err := lfs.ReadPointer(treeObjectContentReader) if err != nil { return err } From 3d64a8492920691dd97f491e2d89c6aa29f3470c Mon Sep 17 00:00:00 2001 From: bytedream Date: Tue, 6 May 2025 14:08:30 +0200 Subject: [PATCH 16/30] Update lfs pointer checks --- services/repository/files/update.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 6e88c1a8cf50e..4bee1dc5e452a 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -521,30 +521,26 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file return err } - var pointer *lfs.Pointer + var pointer lfs.Pointer // Get existing lfs pointer if the old path is in lfs if oldEntry != nil && attributesMap[file.Options.fromTreePath] != nil && attributesMap[file.Options.fromTreePath].Get(attribute.Filter).ToString().Value() == "lfs" { - p, err := lfs.ReadPointer(treeObjectContentReader) - if err != nil { + if pointer, err = lfs.ReadPointer(treeObjectContentReader); err != nil { return err } - pointer = &p } if attributesMap[file.Options.treePath] != nil && attributesMap[file.Options.treePath].Get(attribute.Filter).ToString().Value() == "lfs" { // Only generate a new lfs pointer if the old path isn't in lfs - if pointer == nil { - p, err := lfs.GeneratePointer(treeObjectContentReader) - if err != nil { + if !pointer.IsValid() { + if pointer, err = lfs.GeneratePointer(treeObjectContentReader); err != nil { return err } - pointer = &p } - lfsMetaObject = &git_model.LFSMetaObject{Pointer: *pointer, RepositoryID: repoID} + lfsMetaObject = &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repoID} treeObjectContentReader = strings.NewReader(pointer.StringContent()) - } else if pointer != nil { // old tree path was in lfs, new is not - treeObjectContentReader, err = lfs.ReadMetaObject(*pointer) + } else if pointer.IsValid() { // old path was in lfs, new is not + treeObjectContentReader, err = lfs.ReadMetaObject(pointer) if err != nil { return err } From 3ad1e7e215a3fbcf3409eaa451eae05107af2b1c Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 12:27:10 +0200 Subject: [PATCH 17/30] Enable commit button only if something has changed --- templates/repo/editor/commit_form.tmpl | 2 +- web_src/js/features/repo-editor.ts | 47 +++++++++++++------------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/templates/repo/editor/commit_form.tmpl b/templates/repo/editor/commit_form.tmpl index 8f46c47b96bf2..7efed773492c6 100644 --- a/templates/repo/editor/commit_form.tmpl +++ b/templates/repo/editor/commit_form.tmpl @@ -77,7 +77,7 @@
{{end}}
- {{ctx.Locale.Tr "repo.editor.cancel"}} diff --git a/web_src/js/features/repo-editor.ts b/web_src/js/features/repo-editor.ts index 0f77508f70d8a..acf4127399513 100644 --- a/web_src/js/features/repo-editor.ts +++ b/web_src/js/features/repo-editor.ts @@ -141,38 +141,39 @@ export function initRepoEditor() { } }); + const elForm = document.querySelector('.repository.editor .edit.form'); + + // Using events from https://github.com/codedance/jquery.AreYouSure#advanced-usage + // to enable or disable the commit button + const commitButton = document.querySelector('#commit-button'); + const dirtyFileClass = 'dirty-file'; + + // Enabling the button at the start if the page has posted + if (document.querySelector('input[name="page_has_posted"]')?.value === 'true') { + commitButton.disabled = false; + } + + // Registering a custom listener for the file path and the file content + // FIXME: it is not quite right here (old bug), it causes double-init, the global areYouSure "dirty" class will also be added + applyAreYouSure(elForm, { + silent: true, + dirtyClass: dirtyFileClass, + fieldSelector: ':input:not(.commit-form-wrapper :input)', + change($form: any) { + const dirty = $form[0]?.classList.contains(dirtyFileClass); + commitButton.disabled = !dirty; + }, + }); + // on the upload page, there is no editor(textarea) const editArea = document.querySelector('.page-content.repository.editor textarea#edit_area'); if (!editArea) return; - const elForm = document.querySelector('.repository.editor .edit.form'); initEditPreviewTab(elForm); (async () => { const editor = await createCodeEditor(editArea, filenameInput); - // Using events from https://github.com/codedance/jquery.AreYouSure#advanced-usage - // to enable or disable the commit button - const commitButton = document.querySelector('#commit-button'); - const dirtyFileClass = 'dirty-file'; - - // Disabling the button at the start - if (document.querySelector('input[name="page_has_posted"]').value !== 'true') { - commitButton.disabled = true; - } - - // Registering a custom listener for the file path and the file content - // FIXME: it is not quite right here (old bug), it causes double-init, the global areYouSure "dirty" class will also be added - applyAreYouSure(elForm, { - silent: true, - dirtyClass: dirtyFileClass, - fieldSelector: ':input:not(.commit-form-wrapper :input)', - change($form: any) { - const dirty = $form[0]?.classList.contains(dirtyFileClass); - commitButton.disabled = !dirty; - }, - }); - // Update the editor from query params, if available, // only after the dirtyFileClass initialization const params = new URLSearchParams(window.location.search); From fb3e80106d2551a628573cd2fb91cfc61a5ab8a5 Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 12:35:52 +0200 Subject: [PATCH 18/30] Update comments and names --- services/repository/files/update.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 4bee1dc5e452a..661b9bb09dd3d 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -490,13 +490,14 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file var treeObjectContentReader io.Reader = file.ContentReader var oldEntry *git.TreeEntry - // If no new content is committed, use the file from the last commit as content + // If no new content is committed, which is only the case if file is renamed, use the old file from the last commit as + // content if file.ContentReader == nil { - lastCommit, err := t.GetLastCommit(ctx) + lastCommitID, err := t.GetLastCommit(ctx) if err != nil { return err } - commit, err := t.GetCommit(lastCommit) + commit, err := t.GetCommit(lastCommitID) if err != nil { return err } From 614a4b34d98294827de5ada34904885dd7fc290c Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 12:51:58 +0200 Subject: [PATCH 19/30] Update --- routers/web/repo/editor.go | 12 +++++------- services/repository/files/update.go | 18 +++++++++--------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index db3e10e871755..91b9c069d99af 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -297,12 +297,10 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b operation := "update" if isNewFile { operation = "create" - } - - var contentReader io.ReadSeeker - // form content only has data if file is representable as text, is not too large and not in lfs - if isNewFile || form.Content.Has() { - contentReader = strings.NewReader(strings.ReplaceAll(form.Content.Value(), "\r", "")) + } else if !form.Content.Has() { + // 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{ @@ -315,7 +313,7 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b Operation: operation, FromTreePath: ctx.Repo.TreePath, TreePath: form.TreePath, - ContentReader: contentReader, + ContentReader: strings.NewReader(strings.ReplaceAll(form.Content.Value(), "\r", "")), }, }, Signoff: form.Signoff, diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 661b9bb09dd3d..fdb28a35cb987 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -43,7 +43,7 @@ type ChangeRepoFile struct { Operation string TreePath string FromTreePath string - ContentReader io.ReadSeeker // nil if the operation is a pure rename + ContentReader io.ReadSeeker SHA string Options *RepoFileOptions } @@ -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 } @@ -490,9 +490,9 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file var treeObjectContentReader io.Reader = file.ContentReader var oldEntry *git.TreeEntry - // If no new content is committed, which is only the case if file is renamed, use the old file from the last commit as - // content - if file.ContentReader == nil { + // 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 @@ -523,15 +523,15 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file } var pointer lfs.Pointer - // Get existing lfs pointer if the old path is in lfs - if oldEntry != nil && attributesMap[file.Options.fromTreePath] != nil && attributesMap[file.Options.fromTreePath].Get(attribute.Filter).ToString().Value() == "lfs" { + // Get existing lfs pointer if the operation is a pure rename and the old path is in lfs. This prevents the + // re-generation/re-hash of a lfs pointer to the same data + if file.Operation == "rename" && attributesMap[file.Options.fromTreePath] != nil && attributesMap[file.Options.fromTreePath].Get(attribute.Filter).ToString().Value() == "lfs" { if pointer, err = lfs.ReadPointer(treeObjectContentReader); err != nil { return err } } if attributesMap[file.Options.treePath] != nil && attributesMap[file.Options.treePath].Get(attribute.Filter).ToString().Value() == "lfs" { - // Only generate a new lfs pointer if the old path isn't in lfs if !pointer.IsValid() { if pointer, err = lfs.GeneratePointer(treeObjectContentReader); err != nil { return err @@ -577,7 +577,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file } if !exist { var lfsContentReader io.Reader - if file.ContentReader != nil { + if file.Operation != "rename" { if _, err := file.ContentReader.Seek(0, io.SeekStart); err != nil { return err } From 2c9c9e1b14028818c5852eac8988a324e0e133b9 Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 12:59:09 +0200 Subject: [PATCH 20/30] Update comment --- services/repository/files/update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index fdb28a35cb987..b9655ef7489b2 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -524,7 +524,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file var pointer lfs.Pointer // Get existing lfs pointer if the operation is a pure rename and the old path is in lfs. This prevents the - // re-generation/re-hash of a lfs pointer to the same data + // regeneration/rehash of a lfs pointer to the same data if file.Operation == "rename" && attributesMap[file.Options.fromTreePath] != nil && attributesMap[file.Options.fromTreePath].Get(attribute.Filter).ToString().Value() == "lfs" { if pointer, err = lfs.ReadPointer(treeObjectContentReader); err != nil { return err From 07836d815b7dbf9b35512ab06eb6547f69d4e4a9 Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 16:50:48 +0200 Subject: [PATCH 21/30] Refactor --- services/repository/files/update.go | 150 +++++++++++++++++++--------- 1 file changed, 102 insertions(+), 48 deletions(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index b9655ef7489b2..311ba5a9219dc 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -488,7 +488,6 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file } } - var treeObjectContentReader io.Reader = file.ContentReader 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 @@ -505,53 +504,15 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file if oldEntry, err = commit.GetTreeEntryByPath(file.Options.fromTreePath); err != nil { return err } - if treeObjectContentReader, err = oldEntry.Blob().DataAsync(); err != nil { - return err - } - defer treeObjectContentReader.(io.ReadCloser).Close() } - 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, file.Options.fromTreePath}, - }) - if err != nil { - return err - } - - var pointer lfs.Pointer - // Get existing lfs pointer if the operation is a pure rename and the old path is in lfs. This prevents the - // regeneration/rehash of a lfs pointer to the same data - if file.Operation == "rename" && attributesMap[file.Options.fromTreePath] != nil && attributesMap[file.Options.fromTreePath].Get(attribute.Filter).ToString().Value() == "lfs" { - if pointer, err = lfs.ReadPointer(treeObjectContentReader); err != nil { - return err - } - } - - if attributesMap[file.Options.treePath] != nil && attributesMap[file.Options.treePath].Get(attribute.Filter).ToString().Value() == "lfs" { - if !pointer.IsValid() { - if pointer, err = lfs.GeneratePointer(treeObjectContentReader); err != nil { - return err - } - } - - lfsMetaObject = &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repoID} - treeObjectContentReader = strings.NewReader(pointer.StringContent()) - } else if pointer.IsValid() { // old path was in lfs, new is not - treeObjectContentReader, err = lfs.ReadMetaObject(pointer) - if err != nil { - return err - } - } - } - - // Add the object to the database - objectHash, err := t.HashObject(ctx, treeObjectContentReader) - if err != nil { - return err + var objectHash string + var lfsPointer *lfs.Pointer + switch file.Operation { + case "create", "update": + objectHash, lfsPointer, err = createOrUpdateFile(ctx, t, file, hasOldBranch) + case "rename": + objectHash, lfsPointer, err = renameFile(ctx, t, oldEntry, file, hasOldBranch) } // Add the object to the index @@ -565,9 +526,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 } @@ -601,6 +562,99 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file return nil } +func createOrUpdateFile(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 renameFile(ctx context.Context, t *TemporaryUploadRepository, oldEntry *git.TreeEntry, file *ChangeRepoFile, hasOldBranch bool) (string, *lfs.Pointer, error) { + if setting.LFS.StartServer && hasOldBranch { + 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 path 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 + } else { + 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) From 0d27705d467f707c3d2104e90b0e62b31f123f5c Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 17:30:51 +0200 Subject: [PATCH 22/30] Make linter happy --- services/repository/files/update.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 311ba5a9219dc..d96986d45f556 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -512,7 +512,10 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file case "create", "update": objectHash, lfsPointer, err = createOrUpdateFile(ctx, t, file, hasOldBranch) case "rename": - objectHash, lfsPointer, err = renameFile(ctx, t, oldEntry, file, hasOldBranch) + objectHash, lfsPointer, err = renameFile(ctx, t, oldEntry, file) + } + if err != nil { + return err } // Add the object to the index @@ -595,8 +598,8 @@ func createOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file return objectHash, lfsPointer, nil } -func renameFile(ctx context.Context, t *TemporaryUploadRepository, oldEntry *git.TreeEntry, file *ChangeRepoFile, hasOldBranch bool) (string, *lfs.Pointer, error) { - if setting.LFS.StartServer && hasOldBranch { +func renameFile(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}, @@ -608,7 +611,7 @@ func renameFile(ctx context.Context, t *TemporaryUploadRepository, oldEntry *git 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 path are both in lfs or both not in lfs, the object hash of the old file can be used directly + // 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 @@ -650,9 +653,9 @@ func renameFile(ctx context.Context, t *TemporaryUploadRepository, oldEntry *git return "", nil, err } return objectID, lfsPointer, nil - } else { - return oldEntry.ID.String(), nil, nil } + + return oldEntry.ID.String(), nil, nil } // VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch From 3348c04cc95367a13ab07394b9bd5578b0bc4344 Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 18:28:56 +0200 Subject: [PATCH 23/30] Show binary and lfs edit button in pull request changes --- templates/repo/diff/box.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index fa96d2f0e217c..22abf9a2193cc 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -148,7 +148,7 @@ {{ctx.Locale.Tr "repo.diff.view_file"}} {{else}} {{ctx.Locale.Tr "repo.diff.view_file"}} - {{if and $.Repository.CanEnableEditor $.CanEditFile (not $file.IsLFSFile) (not $file.IsBin)}} + {{if and $.Repository.CanEnableEditor $.CanEditFile}} {{ctx.Locale.Tr "repo.editor.edit_this_file"}} {{end}} {{end}} From 7133834ffe1a19d81e9764db37fa344768e6cc95 Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 23:06:08 +0200 Subject: [PATCH 24/30] Additionally check if path changed to trigger pure rename operation --- routers/web/repo/editor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 91b9c069d99af..af713bbafa6f7 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -297,7 +297,7 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b operation := "update" if isNewFile { operation = "create" - } else if !form.Content.Has() { + } 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" From 8739ebe1ddf9d04f0b89ec551ea77b540e326501 Mon Sep 17 00:00:00 2001 From: bytedream Date: Thu, 8 May 2025 23:06:19 +0200 Subject: [PATCH 25/30] Update internal function names --- services/repository/files/update.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/repository/files/update.go b/services/repository/files/update.go index d96986d45f556..b93538ea5ea01 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -510,9 +510,9 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file var lfsPointer *lfs.Pointer switch file.Operation { case "create", "update": - objectHash, lfsPointer, err = createOrUpdateFile(ctx, t, file, hasOldBranch) + objectHash, lfsPointer, err = createOrUpdateFileHash(ctx, t, file, hasOldBranch) case "rename": - objectHash, lfsPointer, err = renameFile(ctx, t, oldEntry, file) + objectHash, lfsPointer, err = renameFileHash(ctx, t, oldEntry, file) } if err != nil { return err @@ -565,7 +565,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file return nil } -func createOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile, hasOldBranch bool) (string, *lfs.Pointer, error) { +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 { @@ -598,7 +598,7 @@ func createOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file return objectHash, lfsPointer, nil } -func renameFile(ctx context.Context, t *TemporaryUploadRepository, oldEntry *git.TreeEntry, file *ChangeRepoFile) (string, *lfs.Pointer, error) { +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}, From 2c2a7c5c800cdf6d4c3f5f0dd80b90769386fa11 Mon Sep 17 00:00:00 2001 From: bytedream Date: Fri, 9 May 2025 01:21:30 +0200 Subject: [PATCH 26/30] Add rename test --- services/repository/files/update_test.go | 49 ++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 services/repository/files/update_test.go diff --git a/services/repository/files/update_test.go b/services/repository/files/update_test.go new file mode 100644 index 0000000000000..1239b79537078 --- /dev/null +++ b/services/repository/files/update_test.go @@ -0,0 +1,49 @@ +package files + +import ( + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/lfs" + "code.gitea.io/gitea/services/contexttest" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestUpdateRename(t *testing.T) { + unittest.PrepareTestEnv(t) + ctx, _ := contexttest.MockContext(t, "user2/repo1") + contexttest.LoadRepo(t, ctx, 1) + contexttest.LoadRepoCommit(t, ctx) + contexttest.LoadUser(t, ctx, 2) + contexttest.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + + repo := ctx.Repo.Repository + branch := repo.DefaultBranch + + temp, _ := NewTemporaryUploadRepository(repo) + _ = temp.Clone(ctx, branch, true) + _ = temp.SetDefaultIndex(ctx) + + filesBeforeRename, _ := temp.LsFiles(ctx, "README.txt", "README.md") + assert.Equal(t, []string{"README.md", ""}, filesBeforeRename) + + file := &ChangeRepoFile{ + Operation: "rename", + FromTreePath: "README.md", + TreePath: "README.txt", + ContentReader: nil, + SHA: "", + Options: &RepoFileOptions{ + fromTreePath: "README.md", + treePath: "README.txt", + executable: false, + }, + } + contentStore := lfs.NewContentStore() + + err := CreateOrUpdateFile(ctx, temp, file, contentStore, 1, true) + assert.NoError(t, err) + + filesAfterRename, _ := temp.LsFiles(ctx, "README.txt", "README.md") + assert.Equal(t, []string{"README.txt", ""}, filesAfterRename) +} From 627bce976c464f6ba2a5406fa9138d2af742377a Mon Sep 17 00:00:00 2001 From: bytedream Date: Fri, 9 May 2025 01:22:26 +0200 Subject: [PATCH 27/30] Format --- services/repository/files/update_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/repository/files/update_test.go b/services/repository/files/update_test.go index 1239b79537078..ce3b45033eb09 100644 --- a/services/repository/files/update_test.go +++ b/services/repository/files/update_test.go @@ -1,11 +1,13 @@ package files import ( + "testing" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/services/contexttest" + "github.com/stretchr/testify/assert" - "testing" ) func TestUpdateRename(t *testing.T) { From 4e8a8e4c0531a611b65b134d45da77e5e547a845 Mon Sep 17 00:00:00 2001 From: bytedream Date: Fri, 9 May 2025 01:40:50 +0200 Subject: [PATCH 28/30] Add copyright --- services/repository/files/update_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/repository/files/update_test.go b/services/repository/files/update_test.go index ce3b45033eb09..e1a8b04c7083d 100644 --- a/services/repository/files/update_test.go +++ b/services/repository/files/update_test.go @@ -1,3 +1,6 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + package files import ( From b8d5d358eee6c4090350c9d1646891a8ffeffaa4 Mon Sep 17 00:00:00 2001 From: bytedream Date: Fri, 9 May 2025 14:02:08 +0200 Subject: [PATCH 29/30] Update --- routers/web/repo/editor.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index af713bbafa6f7..b819a96819eb2 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -177,11 +177,9 @@ func editFile(ctx *context.Context, isNewFile bool) { if fInfo.isLFSFile { ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_lfs_files") - } else if !fInfo.isTextFile { + } else if !fInfo.st.IsRepresentableAsText() { ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_non_text_files") - } - - if blob.Size() >= setting.UI.MaxDisplayFileSize { + } else if blob.Size() >= setting.UI.MaxDisplayFileSize { ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_too_large_file") } else { d, _ := io.ReadAll(dataRc) From 42b7c8728882a443b8e21cc77eb525d7ffbf351c Mon Sep 17 00:00:00 2001 From: bytedream Date: Fri, 9 May 2025 14:03:17 +0200 Subject: [PATCH 30/30] Get file size from fInfo --- routers/web/repo/editor.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index b819a96819eb2..62de825e527a6 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -170,16 +170,16 @@ func editFile(ctx *context.Context, isNewFile bool) { } } - ctx.Data["FileSize"] = blob.Size() + ctx.Data["FileSize"] = fInfo.fileSize // Only some file types are editable online as text. - ctx.Data["IsFileEditable"] = fInfo.st.IsRepresentableAsText() && !fInfo.isLFSFile && blob.Size() < setting.UI.MaxDisplayFileSize + ctx.Data["IsFileEditable"] = fInfo.st.IsRepresentableAsText() && !fInfo.isLFSFile && fInfo.fileSize < setting.UI.MaxDisplayFileSize if fInfo.isLFSFile { ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_lfs_files") } else if !fInfo.st.IsRepresentableAsText() { ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_non_text_files") - } else if blob.Size() >= setting.UI.MaxDisplayFileSize { + } else if fInfo.fileSize >= setting.UI.MaxDisplayFileSize { ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_too_large_file") } else { d, _ := io.ReadAll(dataRc)