Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix DownloadFunc when migrating releases #27887

Merged
merged 4 commits into from
Nov 3, 2023
Merged

Fix DownloadFunc when migrating releases #27887

merged 4 commits into from
Nov 3, 2023

Conversation

Zettat123
Copy link
Contributor

We should not use asset.ID in DownloadFunc because DownloadFunc is a closure.

gitea/services/migrations/gitea_downloader.go

Lines 284 to 295 in 1bf5527

for _, asset := range rel.Attachments {
size := int(asset.Size)
dlCount := int(asset.DownloadCount)
r.Assets = append(r.Assets, &base.ReleaseAsset{
ID: asset.ID,
Name: asset.Name,
Size: &size,
DownloadCount: &dlCount,
Created: asset.Created,
DownloadURL: &asset.DownloadURL,
DownloadFunc: func() (io.ReadCloser, error) {
asset, _, err := g.client.GetReleaseAttachment(g.repoOwner, g.repoName, rel.ID, asset.ID)

A similar bug when migrating from GitHub has been fixed in #14703. This PR fixes the bug when migrating from Gitea and GitLab.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 3, 2023
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 3, 2023
wxiaoguang
wxiaoguang approved these changes Nov 3, 2023
View reviewed changes
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.

It's better to make the DownloadFunc a standard func giteaDownloadFunc(...), then there won't be such bug.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 3, 2023
@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 Nov 3, 2023
@lunny lunny added backport/v1.20 This PR should be backported to Gitea 1.20 backport/v1.21 This PR should be backported to Gitea 1.21 labels Nov 3, 2023
@lunny lunny added this to the 1.22.0 milestone Nov 3, 2023
@lunny lunny enabled auto-merge (squash) November 3, 2023 05:42
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 3, 2023
auto-merge was automatically disabled November 3, 2023 05:48

Head branch was pushed to by a user without write access

@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 3, 2023
@lunny lunny merged commit ae396ac into go-gitea:main Nov 3, 2023
24 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 3, 2023
@GiteaBot GiteaBot mentioned this pull request Nov 3, 2023
Merged
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Nov 3, 2023
@Zettat123 @GiteaBot
Fix DownloadFunc when migrating releases (go-gitea#27887)
d936616 
We should not use `asset.ID` in DownloadFunc because DownloadFunc is a
closure.

https://github.com/go-gitea/gitea/blob/1bf5527eac6b947010c8faf408f6747de2a2384f/services/migrations/gitea_downloader.go#L284-L295

A similar bug when migrating from GitHub has been fixed in go-gitea#14703. This
PR fixes the bug when migrating from Gitea and GitLab.
@GiteaBot GiteaBot mentioned this pull request Nov 3, 2023
Merged
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Nov 3, 2023
@Zettat123 @GiteaBot
Fix DownloadFunc when migrating releases (go-gitea#27887)
8346d17 
We should not use `asset.ID` in DownloadFunc because DownloadFunc is a
closure.

https://github.com/go-gitea/gitea/blob/1bf5527eac6b947010c8faf408f6747de2a2384f/services/migrations/gitea_downloader.go#L284-L295

A similar bug when migrating from GitHub has been fixed in go-gitea#14703. This
PR fixes the bug when migrating from Gitea and GitLab.
silverwind pushed a commit that referenced this pull request Nov 3, 2023
@GiteaBot @Zettat123
Fix DownloadFunc when migrating releases (#27887) (#27889)
4a48370 
Backport #27887 by @Zettat123

We should not use `asset.ID` in DownloadFunc because DownloadFunc is a
closure.

https://github.com/go-gitea/gitea/blob/1bf5527eac6b947010c8faf408f6747de2a2384f/services/migrations/gitea_downloader.go#L284-L295

A similar bug when migrating from GitHub has been fixed in #14703. This
PR fixes the bug when migrating from Gitea and GitLab.

Co-authored-by: Zettat123 <zettat123@gmail.com>
silverwind pushed a commit that referenced this pull request Nov 3, 2023
@GiteaBot @Zettat123
Fix DownloadFunc when migrating releases (#27887) (#27890)
8d0a4d7 
Backport #27887 by @Zettat123

We should not use `asset.ID` in DownloadFunc because DownloadFunc is a
closure.

https://github.com/go-gitea/gitea/blob/1bf5527eac6b947010c8faf408f6747de2a2384f/services/migrations/gitea_downloader.go#L284-L295

A similar bug when migrating from GitHub has been fixed in #14703. This
PR fixes the bug when migrating from Gitea and GitLab.

Co-authored-by: Zettat123 <zettat123@gmail.com>
@GiteaBot
Copy link
Collaborator

GiteaBot commented Nov 3, 2023

I was unable to create a backport for 1.20. @Zettat123, please send one manually. 🍵

go run ./contrib/backport 27887
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added the backport/manual No power to the bots! Create your backport yourself! label Nov 3, 2023
@GiteaBot
Copy link
Collaborator

GiteaBot commented Nov 3, 2023

I was unable to create a backport for 1.21. @Zettat123, please send one manually. 🍵

go run ./contrib/backport 27887
...  // fix git conflicts if any
go run ./contrib/backport --continue

@lunny lunny added backport/done All backports for this PR have been created and removed backport/manual No power to the bots! Create your backport yourself! labels Nov 3, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 6, 2023
@zjjhot
Merge remote-tracking branch 'upstream/main'
5780e1a 
* upstream/main:
  Fix edit topic UI (go-gitea#27925)
  Unify two factor check (go-gitea#27915)
  Revert go-gitea#27870 (go-gitea#27917)
  Fix JS NPE when viewing specific range of PR commits (go-gitea#27912)
  Install poetry dependencies with --no-root (go-gitea#27919)
  Show correct commit sha when viewing single commit diff (go-gitea#27916)
  Fix 500 when deleting a dismissed review (go-gitea#27903)
  Remove action runners on user deletion (go-gitea#27902)
  Remove SSH workaround (go-gitea#27893)
  Remove "tabindex" from some form buttons (go-gitea#27892)
  Refactor the function RemoveOrgUser (go-gitea#27582)
  Fix DownloadFunc when migrating releases (go-gitea#27887)
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this pull request Jan 17, 2024
@Zettat123 @fuxiaohei
Fix DownloadFunc when migrating releases (go-gitea#27887)
3b56b7c 
We should not use `asset.ID` in DownloadFunc because DownloadFunc is a
closure.

https://github.com/go-gitea/gitea/blob/1bf5527eac6b947010c8faf408f6747de2a2384f/services/migrations/gitea_downloader.go#L284-L295

A similar bug when migrating from GitHub has been fixed in go-gitea#14703. This
PR fixes the bug when migrating from Gitea and GitLab.
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/v1.20 This PR should be backported to Gitea 1.20 backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. type/bug
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.
4 participants
@Zettat123 @GiteaBot @lunny @wxiaoguang