Skip push update when tag reference to blob/tree #23214
Conversation
adlternative
force-pushed
the
tag-to-blob-push-update-error
branch
from
0c23ade to
80ebe2a
Compare
adlternative
changed the title
techknowlogick
added
type/bug
outdated/backport/v1.19
|
Could we have a test for that? |
GiteaBot
added
the
lgtm/need 2
modules/git/repo_commit_nogogit.go
Outdated
| case "blob": | ||
| fallthrough | ||
| case "tree": | ||
| log.Debug("Special typ: %s", typ) |
There was a problem hiding this comment.
I don't think this log debug line will be useful. I think it can be dropped.
There was a problem hiding this comment.
You're going to need to add this/handle this in the gogit variant code too
There was a problem hiding this comment.
You're going to need to add this/handle this in the gogit variant code too
Well, you are right. I do a refactor for the getCommit() of gogit variant code in the latest patch.
adlternative
force-pushed
the
tag-to-blob-push-update-error
branch
5 times, most recently
from
06157d5 to
ddc3f9f
Compare
Because git ref may reference to blob/tree, but push update logic will treate the object as a commit. Therefore, an error occurred when parsing the commit object, resulting in the loss of the webhook notification of this ref and other refs. So we ignore the wrong type error here to let other common refs can do webhook normally. Hope to fix go-gitea#23213. Signed-off-by: ZheNing Hu <[email protected]>
adlternative
force-pushed
the
tag-to-blob-push-update-error
branch
from
ddc3f9f to
ebc77a5
Compare
Is here any good place to test this "push update"(webhook, post-receive)? |
adlternative
requested review from
zeripath and
KN4CK3R
and removed request for
zeripath and
KN4CK3R
You can add an integration test for that. |
|
-> Ignore trivial errors when updating push data #33864 A tag could point to anything, even a non-existing commit or non-blob. So to be simple, I think we could have 2 choices:
|
#33864 is simple enough and I think we do not need to test it now (actually it's difficult to test it because we need to manually construct a bad git repo) |
wxiaoguang
added
issue/needs-feedback
|
Thank you for the PR, 🙏 I think the problem should have been addressed by #33864 , feel free to reopen if there would be new improvements. |
Because git ref may reference to blob/tree, but
push update logic will treate the object as a commit.
Therefore, an error occurred when parsing the commit object, resulting in the loss of the webhook notification of this ref and other refs.
So we ignore the wrong type error here to let other common refs can do webhook normally.
Hope to fix #23213.