Skip to content

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Sep 16, 2022

Fixes #21184
Regression of #19552

Instead of using GetBlobByPath I use the already existing instances.

We need more information from #19530 if that error is still present.

@KN4CK3R KN4CK3R added this to the 1.18.0 milestone Sep 16, 2022
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, and the comment is clear.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Sep 16, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 16, 2022
Comment on lines +153 to +162
headReader, headBlobCloser, err := csvReaderFromCommit(&markup.RenderContext{Ctx: ctx, RelativePath: diffFile.Name}, headBlob)
if headBlobCloser != nil {
defer headBlobCloser.Close()
}
if err == errTooLarge {
return CsvDiffResult{nil, err.Error()}
}
if err != nil {
log.Error("CreateCsvDiff error whilst creating headReader from file %s in commit %s in %s: %v", diffFile.Name, headCommit.ID.String(), ctx.Repo.Repository.Name, err)
return CsvDiffResult{nil, "unable to load file from head commit"}
if err == errTooLarge {
return CsvDiffResult{nil, err.Error()}
}
log.Error("error whilst creating csv.Reader from file %s in head commit %s in %s: %v", diffFile.Name, headBlob.ID.String(), ctx.Repo.Repository.Name, err)
return CsvDiffResult{nil, "unable to load file"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be beneficial to extract this block into one common function taking
ctx context.Context, relativePath string, blob *git.blob, commitType string,
and then calling it once with
ctx, diffFile.OldName, baseBlob, "base"
and once with
ctx, diffFile.Name, headBlob, "head"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a bug fix, I think current approach is also clear and readable.

@lunny lunny merged commit 43c10de into go-gitea:main Sep 17, 2022
wxiaoguang pushed a commit to wxiaoguang/gitea that referenced this pull request Sep 17, 2022
Fixes go-gitea#21184
Regression of go-gitea#19552

Instead of using `GetBlobByPath` I use the already existing instances.

We need more information from go-gitea#19530 if that error is still present.
@wxiaoguang wxiaoguang added backport/done All backports for this PR have been created backport/v1.17 labels Sep 17, 2022
@wxiaoguang
Copy link
Contributor

@KN4CK3R KN4CK3R deleted the fix-21184 branch September 17, 2022 09:24
wxiaoguang added a commit that referenced this pull request Sep 17, 2022
Backport #21189
Fixes #21184
Regression of #19552

Instead of using `GetBlobByPath`, use the already existing instances.
zjjhot added a commit to zjjhot/gitea that referenced this pull request Sep 19, 2022
* upstream/main:
  Fix typo (go-gitea#21201)
  Remove unnecessary length check for repo's Description & Website (go-gitea#21194)
  Treat git object mode 40755 as directory (go-gitea#21195)
  Fix reaction of issues (go-gitea#21185)
  Fix CSV diff for added/deleted files (go-gitea#21189)
  Show label description in comments section (go-gitea#21156)
  Limit length of repo description and repo url input fields (go-gitea#21119)
vanhoang1107 added a commit to vanhoang1107/gitea that referenced this pull request Oct 31, 2022
* src/release/v1.17: (26 commits)
  Fix reaction of issues (go-gitea#21185) (go-gitea#21196)
  Fix CSV diff for added/deleted files (go-gitea#21189) (go-gitea#21193)
  Fix pagination limit parameter problem (go-gitea#21111)
  Add MD5 back to template helper functions to avoid breaking (go-gitea#21102)
  Add changelog for v1.17.2 (go-gitea#21089)
  Fix sub folder in repository missing add file dropdown (go-gitea#21069) (go-gitea#21083)
  Fix hard-coded timeout and error panic in API archive download endpoint (go-gitea#20925) (go-gitea#21051)
  Fix delete user missed some comments (go-gitea#21067) (go-gitea#21068)
  Delete unreferenced packages when deleting a package version (go-gitea#20977) (go-gitea#21060)
  Redirect if user does not exist on admin pages (go-gitea#20981) (go-gitea#21059)
  Set uploadpack.allowFilter etc on gitea serv to enable partial clones with ssh (go-gitea#20902) (go-gitea#21058)
  Fix 500 on time in timeline API (go-gitea#21052) (go-gitea#21057)
  Fill the specified ref in webhook test payload (go-gitea#20961) (go-gitea#21055)
  Add another index for Action table on postgres (go-gitea#21033) (go-gitea#21054)
  fix broken insecureskipverify handling in rediss connection uris (go-gitea#20967) (go-gitea#21053)
  Add more checks in migration code (go-gitea#21011) (go-gitea#21050)
  Add Dev, Peer and Optional dependencies to npm PackageMetadataVersion (go-gitea#21017) (go-gitea#21044)
  Improve arc-green code theme (go-gitea#21039) (go-gitea#21042)
  Add down key check has tribute container (go-gitea#21016) (go-gitea#21038)
  Do not add links to Posters or Assignees with ID < 0 (go-gitea#20577) (go-gitea#21037)
  ...
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSV diff: unable to load file from base commit
6 participants