`pacman-mirrors -g` should not add multiple protocols to mirrorlist

jonathonf commented 7 years ago

Currently, pacman-mirrors -g will add each available protocol for each mirror to mirrorlist. This can result in three "duplicated" mirrors and lead to reduced resilience; it's conceivable a user ends up with e.g. nine mirrorlist entries but over only three mirrors.

Each mirror should be added only once, ideally in order HTTPS, HTTP, FTP.

Thinking, this might be easiest by reordering the mirrors JSON data and setting the first responding protocol, e.g. from:

{
        "country": "Germany",
        "url": "https://ftp.halifax.rwth-aachen.de/manjaro/",
        "protocols": [
            "ftp",
            "http",
            "https"
        ]
},

to:

{
        "country": "Germany",
        "url": "https://ftp.halifax.rwth-aachen.de/manjaro/",
        "protocols": [
            "https",
            "http",
            "ftp",
        ]
},

then e.g. test foreach protocol in protocols

fhdk commented 7 years ago

You are right and I have been thinking the same thing.

That is why the interactive mode is changed to display all protocols. This allows for a selection of protocols - leaving those out you do not which to use.

~~Also I can see there is an issue with ftp not being shown in the --interactive selection.~~

fhdk commented 7 years ago

@philmmanjaro @jonathonf @Huluti Would you please take some of your precious time and review the latest commits.

In addition to previous enhancements - I have added an option to pacman-mirrors.conf allowing the user to specify accepted protocols and priority If more than one protocol is specified all are written to the mirrorlist in the order in which they appear in .conf

Regards .

fhdk commented 7 years ago

@philmmanjaro Me and version numbering is confusing at best :) I have updated changelog with my recent modifications Will you please edit the version?

fhdk commented 7 years ago

@philmmanjaro If you can build a new pacman-mirrors-dev it will be much appreciated :)

philmmanjaro commented 7 years ago

@fhdk: current code writes in inactive mode without $branch/$repo/$arch

phil@manjaro ~/dev/git/manjaro/repositories/extra/xfce4-gtk3 $ sudo pacman-mirrors -gc Germany -i
.: Info Downloading mirrors from repo.manjaro.org

.: Info Benutzergenerierte Mirrorliste
--------------------------
.: Info Benutzerspezifische Mirrordatei wurde gesichert: /var/lib/pacman-mirrors/custom-mirrors.json
.: Info Schreibe Mirrorliste
   Germany         : https://mirror.netzspielplatz.de/manjaro/packages/unstable
.: Info Mirrordatei generiert und gesichert in: /etc/pacman.d/mirrorlist
.: Info To reset custom config run  pacman-mirrors -c all
phil@manjaro ~/dev/git/manjaro/repositories/extra/xfce4-gtk3 $ cat /etc/pacman.d/mirrorlist
##
## Manjaro Linux Custom mirrorlist
## Generated on 2017-04-17 16:45
##
## Use 'pacman-mirrors -c all' to reset
##

## Country : Germany
Server = https://mirror.netzspielplatz.de/manjaro/packages/

fhdk commented 7 years ago

I see - will check immediately

fhdk commented 7 years ago

Thanks - I missed that - it is fixed https://github.com/manjaro/pacman-mirrors/commit/734db2d71fd43f8ebca27f0ca0c40ce6820997d9

fhdk commented 7 years ago

@philmmanjaro From the forum post that caused @jonathonf to create this issue - I read that the OP tried to edit the mirrors.json in hope of eliminating an annoying issue with a ftp-mirror-protocol.

This made me think - in case downloading form repo.manjaro.org it should be enough with one fallback mirror and if downloading succeeds update the file in /usr/share/pacman-mirrors.

This makes sense since changes to the mirrors.json on web-repo then is reflected to the users package installed mirror file - if the user deletes the status.json and subsequently has network issues - it will not be an outdated mirrors.json the user has in store.

It also makes sense since it is always status.json which is used when generating the mirrorlist. Only in the rare event where a network connection is absent - the file mirrors.json is consulted.

So I have refactored some internals to remove the mirrors.json from /var/lib/ and update the mirrors.json in /usr/share/.

This should ensure that no confusion exist about the files in /var/lib/. I have also created a thread on the forum asking for user input regarding this issue.

Also I would ask you to rebuild the package for me, please.

jonathonf commented 7 years ago

From the forum post that caused @jonathonf to create this issue

Just to clarify, the forum post was created after this issue. I identified this entirely on my own. :)

So I have refactored some internals to remove the mirrors.json from /var/lib/ and update the mirrors.json in /usr/share/.

That should probably be the other way round - /usr/share should not change outside of package updates, /var is the place for 'variable' data. More: http://www.pathname.com/fhs/pub/fhs-2.3.html

fhdk commented 7 years ago

I know the defines of fhs but since the file exist solely because the package installed it, then when the package itself updates the file - it should not pose a problem.

The manjaro-tools package install a lot of files to /usr/share/manjaro-tools and the binaries are coded in a way that they expect that targets are only accessible as root.

All guides around manjaro-tools suggests you modify the files in /usr/share - I have a problem with that since changes I would make will be lost on update.

The similar cannot be said for pacman-mirrors as the very package process download the latest available mirrors.json from manjaro-web-repo and the changes is not made by third party but the packages itself which fetches the same mirrors.json.

jonathonf commented 7 years ago

I know the defines of fhs but since the file exist solely because the package installed it, then when the package itself updates the file - it should not pose a problem.

Respectfully, I disagree. Files in /usr/share shouldn't be variable - they should be whatever is installed by the package. Packages are free to install to /var/lib though (and many do).

The manjaro-tools package install a lot of files to /usr/share/manjaro-tools and the binaries are coded in a way that they expect that targets are only accessible as root.

I'm not quite sure what difference that makes - if the downloaded mirrors.json used by pacman-mirrors is in /var/lib how does that affect other tools?

if other tools modify files under /usr/share then they have bugs that should be fixed.

fhdk commented 7 years ago

I get your point and generally I would agree.

Say - the pacman-mirrors package is updated once a week - the binary does not but the mirrors.json does - then it is ok to overwrite '/usr/share` with the updated mirrors.conf.

A lot of programs has self-update mechanisms which update vital files - so the user does not have to reinstall the package for minor updates eg. pacman-mirrors.

How is the two above processes different? Except for the obvious of a package vs an app updating the same file?

fhdk commented 7 years ago

The fhs does not have a standard saying that a program cannot update its own files in /usr/*/

I read /usr as a storage for readonly user data - mirrors.json is a read-only data file /usr/share/ as Architecture-independent (shared) data - mirrors.json is shared among all users

/var as files whose content is expected to continually change logs, spool /var/lib as state information - mirrors.json is a data file of mirrors/protocols and contains no info on the mirrors state nor does it not change very often - status.json does change as often as manjaro-web-repo does a mirror-state-check

Based on the above I would consider mirrors.json as read-only data file and it place in /usr/share as the right place - even those datafiles changes over time - so I would not consider updating the mirrors.json as a violation of the fhs standard - as it does not dictate those files never changes - only that they are readonly to the user and if the user modifies one it must be in the userspace.

However I will agree to let the code do a check against the existing file and only update if changes are detected.

fhdk commented 7 years ago

@philmmanjaro Based on info and comments from various parties - the latest changes from the CHANGELOG is

/var/lib/pacman-mirrors/mirrors.json is causing confusion so it has been removed.
- Only one fallback is needed /usr/share/pacman-mirrors/mirrors.json.
- If a new mirrors.json is available - existing will be updated by pacman-mirrors.
Improvement on default mirrorlist.
- mirror protocols are reverse sorted (https,http,ftps,ftp).
- if several protocols exist only the first is written to mirrorlist. Thus ssl enabled protocols get priority.
Improvement on mirror protocol selection #90.
- If a mirror offers more than one protocol - only the first is written to the mirrorlist.
- Added # Protocols = to pacman-mirrors.conf.
- Defined protocols are honored in the order in which they appear.

When you have had time to review it - I would appreciate an updated build

fhdk commented 7 years ago

@philmmanjaro

Added protocol option to api.
- Possible to control protocols from CLI

When you have the time - please have a look and afterwards upload a new package?

manjaro / pacman-mirrors

`pacman-mirrors -g` should not add multiple protocols to mirrorlist #90