Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Only show the latest version in the Arch index #33262

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

ExplodingDragon
Copy link
Contributor

@ExplodingDragon ExplodingDragon commented Jan 14, 2025

Only show the latest version of the package in the arch repo.

closes #33534

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 14, 2025
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 14, 2025
@github-actions github-actions bot added modifies/go Pull requests that update Go code docs-update-needed The document needs to be updated synchronously labels Jan 14, 2025
@ExplodingDragon ExplodingDragon changed the title Feat(package): Keep only the latest version in Arch DB WIP: Keep only the latest version in Arch DB Jan 14, 2025
@ExplodingDragon ExplodingDragon changed the title WIP: Keep only the latest version in Arch DB Keep only the latest version in Arch Repo Jan 15, 2025
@ExplodingDragon ExplodingDragon marked this pull request as ready for review January 15, 2025 07:30
@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 Jan 20, 2025
@wxiaoguang
Copy link
Contributor

Actually I have some questions about the design.

  • KEEP_LATEST_VERSION sounds like "only keep the package with latest version, and delete others"
  • After reading code, I think it means that "only show the package with latest version in the package index"

So the questions are:

  • Should we delete the old packages? Or not delete, just hide them from the index (this PR does)?
  • Could it ("deleting old packages" or "not-deleting but just hide old packages from index") be a general behavior for other package registries?
  • The config name sounds misleading: I think it should clarify what's the expected result in the name: deleting, or just hiding.

@ExplodingDragon ExplodingDragon changed the title Keep only the latest version in Arch Repo Only show the latest version in the Arch index Jan 21, 2025
@ExplodingDragon
Copy link
Contributor Author

Actually I have some questions about the design.

* `KEEP_LATEST_VERSION` sounds like "only keep the package with latest version, and delete others"

* After reading code, I think it means that "only show the package with latest version in the package index"

So the questions are:

* Should we delete the old packages? Or not delete, just hide them from the index (this PR does)?

* Could it ("deleting old packages" or "not-deleting but just hide old packages from index") be a general behavior for other package registries?

* The config name sounds misleading: I think it should clarify what's the expected result in the name: deleting, or just hiding.

@wxiaoguang Sorry, I misspoke. KEEP_LATEST_VERSION will change to SHOW_LATEST_VERSION.

@wxiaoguang
Copy link
Contributor

Are there some reference documents for the "only show latest version in index" behavior? (The question is why it is needed to be done on server side, since client could always figure out the latest version)

@ExplodingDragon
Copy link
Contributor Author

ExplodingDragon commented Jan 21, 2025

Are there some reference documents for the "only show latest version in index" behavior? (The question is why it is needed to be done on server side, since client could always figure out the latest version)

@wxiaoguang No, this comes from my subjective opinion. Having too many packages will make the index larger, and most of the time, there isn't much demand for downloading older versions of software packages. Other package registries are facing the same situation.

Just like Arch Linux's rolling release model, which only keeps the latest version.

@lunny lunny added this to the 1.24.0 milestone Jan 21, 2025
@wxiaoguang
Copy link
Contributor

So for this case, it doesn't need a new option, just "only show latest packages" for arch/alpine?

@wxiaoguang As mentioned earlier (for some software package repositories like Kubernetes), there are users who need to keep historical versions.

Yes, so I said just "only show latest packages" for arch/alpine? It should follow the official behavior.

@ExplodingDragon what do you think? Should we follow the official behavior by default, or still introduce new config options?

@ExplodingDragon
Copy link
Contributor Author

what do you think? Should we follow the official behavior by default, or still introduce new config options?

@wxiaoguang Sorry for the late reply. I think adding the new option seems better. I’m not sure if anyone actually needs the old version installed—even if it’s just 1% of people.

@lunny
Copy link
Member

lunny commented Feb 5, 2025

what do you think? Should we follow the official behavior by default, or still introduce new config options?

@wxiaoguang Sorry for the late reply. I think adding the new option seems better. I’m not sure if anyone actually needs the old version installed—even if it’s just 1% of people.

How about make 99% as default? And we can have a break label.

@wxiaoguang
Copy link
Contributor

Maybe we need to add a flexible org/user setting system first (#33262 (comment)) before introducing this change.

@wxiaoguang
Copy link
Contributor

I think we need to merge it without the config option, only show the latest version.

-> Arch package registry: client error when two versions of a package are present #33534

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Feb 9, 2025

I still do not understand why SHOW_LATEST_VERSION_ARCH must be introduced.

  • Does any client really depend on "all versions" or does any other registry server show "all versions"?
  • What would go wrong if we force to always show latest package?

If there is a must and a strong reason to introduce SHOW_LATEST_VERSION_ARCH, it should be well documented: what's the real use case, how/why other clients/servers do it.


There are already too many config options, keep introducing unclear options would make the whole system more difficult to maintain.

@balki
Copy link
Contributor

balki commented Feb 9, 2025

+1 to show only the latest version in db by default without config option. Also lets not delete old versions from gitea (at least have an option to prevent). It is possible to download the package using browser and install using pacman -U.

services/packages/arch/vercmp.go

Not sure this is required. Just using the last uploaded package should be good (order by time of upload). This would also allow to revert back to older version by deleting and re-uploading old version.

@ExplodingDragon
Copy link
Contributor Author

There are already too many config options, keep introducing unclear options would make the whole system more difficult to maintain.

@wxiaoguang There’s no good reason to keep this option, this is a bug in Gitea, it’s intentional from upstream pacman. ( #33534 (comment))

Not sure this is required. Just using the last uploaded package should be good (order by time of upload). This would also allow to revert back to older version by deleting and re-uploading old version.

@balki Keeping the latest version around makes way more sense. Using the last upload timestamp is just gonna cause chaos.

@github-actions github-actions bot removed the docs-update-needed The document needs to be updated synchronously label Feb 10, 2025
@ExplodingDragon
Copy link
Contributor Author

Since this is a bug, we need to backport it to version 1.23.

@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 Feb 10, 2025
@wxiaoguang
Copy link
Contributor

@KN4CK3R do you have more suggestions for this change?

@lunny lunny added the backport/v1.23 This PR should be backported to Gitea 1.23 label Feb 10, 2025
@KN4CK3R
Copy link
Member

KN4CK3R commented Feb 10, 2025

services/packages/arch/vercmp.go

Not sure this is required. Just using the last uploaded package should be good (order by time of upload). This would also allow to revert back to older version by deleting and re-uploading old version.

I would go with that easy approach too because it will cover most (but sure, not all) use cases. To get the latest package version the SearchLatestVersions function can be used. But if you insist to replicate the full version comparison I will not block it.

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 10, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
backport/v1.23 This PR should be backported to Gitea 1.23 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arch package registry: client error when two versions of a package are present
7 participants