Replace `repo.namedBlob` by `git.TreeEntry`. #22898

kousu · 2023-02-13T22:30:46Z

namedBlob turned out to be a poor imitation of a TreeEntry. Using the latter directly shortens this code.

This partially undoes #23152, which I found a merge conflict with, and also expands the test it added to cover the subtle README-in-a-subfolder case.

delvh

I just asked myself if this cache type was implemented for performance reasons, but according to the method implementations, that is negligible…
Was that different previously?
Otherwise, I don't know how this could happen.

kousu · 2023-02-13T22:47:09Z

for performance reasons

And even if it was, it was only optimizing for README files. How heavy is that really?

Otherwise, I don't know how this could happen.

Thanks for the vote of confidence!

zeripath · 2023-02-13T22:48:05Z

IIRC we needed the namedBlob at some point because we were passing round git.Blobs but we're able to get the TreeEntry just as easily now so it might be needed anymore

routers/web/repo/view.go

`namedBlob` turned out to be a poor imitation of a `TreeEntry`. Using the latter directly shorten this code.

kousu · 2023-03-10T19:00:11Z

The current CI failures are

Init storage failed: Put "http://minio:9000/gitea/": dial tcp: lookup minio on 127.0.0.11:53: no such host

and

=== Test_DeleteOrphanedIssueLabels (/drone/src/models/migrations/v1_14/v177_test.go:37)

LoadFixtures failed after retries: testfixtures: error inserting record: pq: column "exclusive" of relation "label" does not exist, on file: label.yml, index: 0, sql: INSERT INTO "label" ("num_closed_issues", "id", "repo_id", "org_id", "name", "color", "exclusive", "num_issues") VALUES ($1, $2, $3, $4, $5, $6, $7, $8), params: [0 1 1 0 label1 #abcdef false 2]

--- FAIL: Test_DeleteOrphanedIssueLabels (5.07s)

    v177_test.go:37: initializing fixtures from: /drone/src/models/migrations/fixtures/Test_DeleteOrphanedIssueLabels

    v177_test.go:37: error whilst loading fixtures from /drone/src/models/migrations/fixtures/Test_DeleteOrphanedIssueLabels: testfixtures: error inserting record: pq: column "exclusive" of relation "label" does not exist, on file: label.yml, index: 0, sql: INSERT INTO "label" ("num_closed_issues", "id", "repo_id", "org_id", "name", "color", "exclusive", "num_issues") VALUES ($1, $2, $3, $4, $5, $6, $7, $8), params: [0 1 1 0 label1 #abcdef false 2]

FAIL

FAIL	code.gitea.io/gitea/models/migrations/v1_14	6.460s

FAIL

These don't seem to be related to this patch. I'll restart and if it happens again I guess I'll try to reproduce locally.

EDIT: passing now; all I did was merge main: https://drone.gitea.io/go-gitea/gitea/69318

kousu · 2023-03-10T20:54:43Z

models/fixtures/repository.yml

-  size: 7028
+  size: 7320


This had to change due to the test additions I did. I used the same test repo (user2/repo1.git) that #23152 did, but I don't know why that PR didn't force changing this number.

kousu · 2023-03-10T21:18:17Z

routers/web/repo/view.go

 			Metas:        ctx.Repo.Repository.ComposeDocumentMetas(),
 			GitRepo:      ctx.Repo.GitRepo,
 		}, rd)
 		if err != nil {
-			log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.name, ctx.Repo.Repository, err)
+			log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.Name(), ctx.Repo.Repository, err)


Perhaps

Suggested change

log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.Name(), ctx.Repo.Repository, err)

log.Error("Render failed for %s in %-v: %v Falling back to rendering source", path.Join(subfolder, readmeFile.Name()), ctx.Repo.Repository, err)

?

routers/web/repo/view.go

lunny · 2023-03-15T11:53:53Z

Please update branch

kousu · 2023-03-15T17:09:18Z

Thank you @lunny, @delvh and @zeripath :)

jolheiser · 2023-03-15T21:07:25Z

@kousu
Please update again, I've added a label so we can get it merged hopefully without bugging you too many more times. 🙂

kousu · 2023-03-15T21:09:27Z

Of course! Here it comes around again.

I really appreciate getting this in. I've had my eye on this patch for a couple of months now.

Did I do something wrong btw? Did I miss clicking "Allow Edits from Maintainers" or something?

jolheiser · 2023-03-15T21:10:54Z

Did I do something wrong btw? Did I miss clicking "Allow Edits from Maintainers" or something?

Nope, not your fault, I believe it's a GitHub thing that doesn't let us update it because it's from an org vs a user account.

* giteaofficial/main: [skip ci] Updated translations via Crowdin Replace `repo.namedBlob` by `git.TreeEntry`. (go-gitea#22898) Fix theme-auto loading (go-gitea#23504) Update path to docs theme file (go-gitea#23502) Use arm image for arm runner (go-gitea#23503)

delvh approved these changes Feb 13, 2023

View reviewed changes

GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Feb 13, 2023

lunny reviewed Feb 14, 2023

View reviewed changes

routers/web/repo/view.go Outdated Show resolved Hide resolved

lunny reviewed Feb 17, 2023

View reviewed changes

routers/web/repo/view.go Outdated Show resolved Hide resolved

kousu marked this pull request as draft February 17, 2023 15:55

This comment was marked as resolved.

Sign in to view

This comment was marked as off-topic.

Sign in to view

kousu marked this pull request as ready for review March 9, 2023 01:33

Replace repo.namedBlob by git.TreeEntry.

947604d

`namedBlob` turned out to be a poor imitation of a `TreeEntry`. Using the latter directly shorten this code.

kousu force-pushed the treeentry-findReadmeFile branch from 6c2779b to 947604d Compare March 9, 2023 23:33

kousu added 2 commits March 10, 2023 10:55

Forgotten line

e9e3518

fixup test

d6dba37

Merge branch 'main' into treeentry-findReadmeFile

d108c2f

kousu commented Mar 10, 2023

View reviewed changes

Merge branch 'main' into treeentry-findReadmeFile

580c379

lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 12, 2023

lunny added this to the 1.20.0 milestone Mar 12, 2023

lunny reviewed Mar 12, 2023

View reviewed changes

routers/web/repo/view.go Show resolved Hide resolved

kousu added 2 commits March 13, 2023 02:17

Merge branch 'main' into treeentry-findReadmeFile

6e162dc

Merge branch 'main' into treeentry-findReadmeFile

014a753

lunny approved these changes Mar 15, 2023

View reviewed changes

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 Mar 15, 2023

lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2023

Merge branch 'main' into treeentry-findReadmeFile

4e4aadb

jolheiser added the reviewed/prioritize-merge PR is in the merge queue. Merge as soon as possible, i.e. as edits by maintainers are not enabled label Mar 15, 2023

Merge branch 'main' into treeentry-findReadmeFile

f5703d4

jolheiser merged commit 6aef9e0 into go-gitea:main Mar 15, 2023

jolheiser removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. reviewed/prioritize-merge PR is in the merge queue. Merge as soon as possible, i.e. as edits by maintainers are not enabled labels Mar 15, 2023

kousu deleted the treeentry-findReadmeFile branch March 16, 2023 01:15

go-gitea locked and limited conversation to collaborators May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace `repo.namedBlob` by `git.TreeEntry`. #22898

Replace `repo.namedBlob` by `git.TreeEntry`. #22898

kousu commented Feb 13, 2023 •

edited

delvh left a comment

kousu commented Feb 13, 2023

zeripath commented Feb 13, 2023

This comment was marked as resolved.

This comment was marked as off-topic.

kousu commented Mar 10, 2023 •

edited

kousu Mar 10, 2023

kousu Mar 10, 2023 •

edited

lunny commented Mar 15, 2023

kousu commented Mar 15, 2023

jolheiser commented Mar 15, 2023

kousu commented Mar 15, 2023

jolheiser commented Mar 15, 2023

	log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.Name(), ctx.Repo.Repository, err)
	log.Error("Render failed for %s in %-v: %v Falling back to rendering source", path.Join(subfolder, readmeFile.Name()), ctx.Repo.Repository, err)

Replace repo.namedBlob by git.TreeEntry. #22898

Replace repo.namedBlob by git.TreeEntry. #22898

Conversation

kousu commented Feb 13, 2023 • edited

delvh left a comment

Choose a reason for hiding this comment

kousu commented Feb 13, 2023

zeripath commented Feb 13, 2023

This comment was marked as resolved.

This comment was marked as off-topic.

kousu commented Mar 10, 2023 • edited

kousu Mar 10, 2023

Choose a reason for hiding this comment

kousu Mar 10, 2023 • edited

Choose a reason for hiding this comment

lunny commented Mar 15, 2023

kousu commented Mar 15, 2023

jolheiser commented Mar 15, 2023

kousu commented Mar 15, 2023

jolheiser commented Mar 15, 2023

Replace `repo.namedBlob` by `git.TreeEntry`. #22898

Replace `repo.namedBlob` by `git.TreeEntry`. #22898

kousu commented Feb 13, 2023 •

edited

kousu commented Mar 10, 2023 •

edited

kousu Mar 10, 2023 •

edited