feat(scripts/pkg): filter out outdated mirrors #169
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
df904db to
4d6a031
Compare
Member
Author
We do not fetch file content itself so I called the variable
Logging of time difference between default mirror and current mirror last update time required changing logging logic a bit but I figured it out. Now output looks like this: |
9cb1342 to
3ded0da
Compare
Member
Author
|
I have a sneaking suspicion as to why some users are running into a bit of trouble here... Currently I am rewriting it with a bit more advanced logging and I found an interesting thing: |
Member
Author
|
Only some of mirrors are more or less up-to-date, others are outdated or unusable. |
Member
|
You need to download the |
Member
Yeah, I suspected this too, that's why this was important to implement to filter them out and report to mirror owners, and permanently remove them if they remain too out of date. |
Member
Author
But why not? Seems like reading "last-modified" header of file is enough for finding out if file is up-to-date. Rsync explicitly syncs timestamps of files during sync. |
Member
Author
|
I'll check it though. |
Member
|
If a mirror url is not hosting a Release file and instead is hosting a different file or redirecting to a different url like in case of domain expiry or new host, then you are comparing last modified time of the redirected page, not the release file which doesn't even exist on the server. Look at following, no Release file exists at that url, but time is of right now. Basically header timestamp is not what should be compared, and it can be anything as per server policies. $ curl --head https://deb.kcubeterm.com/termux-main
HTTP/2 521
date: Mon, 19 May 2025 21:37:32 GMT
content-length: 0
server: cloudflare
cache-control: private, no-store |
Member
|
Also not everyone may be using rsync and/or preserving timestamps of files. |
Member
Author
|
Member
|
You are assuming 521 will always be returned. Redirection could happen to a valid url too, like homepage. |
Member
Author
|
With obtaining range 0-255 and parsing date from the release file the situation is not very much different. |
Member
Author
|
I was wrong about And even in this case the situation is not better. |
Member
Author
|
Also AFAIK in the case if you not specify What do you think? |
Member
Author
|
Member
|
I was expecting some mirrors to have problems with |
Member
Author
|
Ok, so I am putting work in this direction on hold until I get your response on this. But I still think checking Last-modified is fine because it will be or synchronized with termux's server or it will be a date of the last sync so it is still better than the current master branch solution. |
Member
|
Thanks, its not just about last modified time, we also need to ensure the url is for a valid Release file for the |
Member
Author
|
If you think download ~1MB data is fine (~13KB * 70 files, somebody still uses limited connections) I can download the whole file content as well. But I just want to mention currently |
Member
|
That is definitely not fine, only following part needs to be downloaded for validation, that is Origin: termux-main stable
Label: termux-main stable
Suite: stable
Codename: stable
Date: Wed, 21 May 2025 13:07:54 UTC
Architectures: aarch64 arm i686 x86_64
Components: main$ curl --silent --fail https://packages.termux.dev/apt/termux-main/dists/stable/Release1 | head -c 200
$ echo "${PIPESTATUS[0]} ${PIPESTATUS[1]}"
22 0
$ curl --silent --fail https://packages.termux.dev/apt/termux-main/dists/stable/Release | head -c 200
Origin: termux-main stable
Label: termux-main stable
Suite: stable
Codename: stable
Date: Wed, 21 May 2025 13:07:54 UTC
Architectures: aarch64 arm i686 x86_64
Components: main
$ echo "${PIPESTATUS[0]} ${PIPESTATUS[1]}"
0 0 |
Member
Author
|
So maybe in this case we should do |
Member
|
No, at least origin and architectures should be validated too. Regex engine should run internally in bash. Maybe |
9 tasks
Member
Author
|
I just wanted to add something simple, but somehow it spiraled into a full script refactor, I'm not even sure how! And I wonder how I have more deletions than insertions. So, there are some differences to original behaviour.
|
3ded0da to
266736f
Compare
Member
Author
|
So now the output is like this |
Member
Author
|
Probably I can make it print only hostnames instead of full mirror urls, or even hostname + location to make output a bit more useful. Should I do so? |
266736f to
61af27f
Compare
Member
Author
|
Seems like |
61af27f to
8de8a81
Compare
Open
Member
That sounds fine because, even if the full URL of the selected mirror is desired for convenience (like clicking it in a terminal emulator that supports opening URLs in order to browse it in a web browser), |
robertkirkman
approved these changes
May 27, 2025
Member
robertkirkman
left a comment
There was a problem hiding this comment.
I tested this and I tried to find anything wrong with it, but I have not found anything wrong in my opinion and this overall runs much faster than the current version.
Member
Author
|
Seems like performing However, apt does not update the CC @agnostic-apollo because you merged this change. |
Member
Lies!!! Trying to ruin my good name! |
Member
|
Will have to think more on the problem. Thanks for notification. |
Member
Author
|
Member
|
That didn't add |
Member
Author
|
Any specific reason to avoid using |
Member
|
There shouldn't be a functional difference as far I see. But there should have been an existence check added in |
8de8a81 to
cc1dea6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #4 (comment)
With this change mirrors are being automatically filtered out in the case if last sync was more than 12 hours ago which should be fine.
But I am not sure what will happen if the last change in termux-packages happens more than 12 hours ago, probably it will break mirror picking, needs some tests.
Also it implements nice indication for outdated mirrors, like (example):
Closes termux/termux-app#4517