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.

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

Add releases/latest{file} route #25429

Closed
wants to merge 2 commits into from

Conversation

dboreham
Copy link

Possible partial fix (the "download url for latest release" part) for #3712
Draft PR posted for comment, tests currently missing.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 21, 2023
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 21, 2023
@JakobDev
Copy link
Contributor

{repo}/releases/latest is already a existing download link. So you should use {repo}/releases/latest/{filename} instead of {repo}/releases/download-latest/{filename}. A big problem I see here, is that many Projects tend to include the Version on the filename.

@dboreham
Copy link
Author

I don't think {repo}/releases/latest works here because it only redirects to the specific release page.
It doesn't allow for downloading a named artifact from the latest release, like:

$ curl http://gitea.local:3000/org/repo/releases/download-latest/file

@JakobDev
Copy link
Contributor

I know, that currently you can't download through {repo}/releases/latest, as it's only a redirect. I was suggest, that you use {repo}/releases/latest/{file} instead of {repo}/releases/download-latest/{file}as route in this PR, as this is more intuitive.

@dboreham
Copy link
Author

I was suggest, that you use {repo}/releases/latest/{file} instead of {repo}/releases/download-latest/{file}as route in this PR

I didn't do that because the download route seems to be subject to different auth checks and I don't understand enough about the routing library to know if that would be problematic. But if you think it's ok, it's fine with me.

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 22, 2023
@dboreham
Copy link
Author

Pushed a commit that changes the path to {repo}/releases/latest/{file}.
Manually tested. Auth works as expected (unauthenticated access to {repo}/releases/latest/{file} is permitted while still disallowed to {repo}/releases/latest).

Now looking at how to add a test...

@dboreham
Copy link
Author

I found existing tests here: https://github.com/go-gitea/gitea/blob/main/tests/integration/api_releases_test.go but doesn't look like there is a test for download already. I will look at adding one for the existing download, and the latest release download code added in this PR.

@dboreham dboreham changed the title Add releases/download-latest route Add releases/latest{file} route Jun 23, 2023
@silverwind
Copy link
Member

Should we expose the new link on UI somewhere?

@silverwind silverwind added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Jun 23, 2023
@dboreham
Copy link
Author

Should we expose the new link on UI somewhere?

I'm not sure. GH just documents this (search for "Linking to the latest release").

@silverwind
Copy link
Member

A test would be nice, likely in tests/integration/release_test.go.

@dboreham
Copy link
Author

Closing because this PR has already been merged. It seems to achieve roughly the same goal, except presumably will cause problems if anyone ever makes a real tag named "latest". This PR handled that case by using a distinct route rather than overloading the meaning of the vTag.

@dboreham dboreham closed this Oct 11, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants