From fb159a8255b999bf17e2c23fc481c9cec970ad0d Mon Sep 17 00:00:00 2001 From: Daniel Girtler Date: Wed, 22 Jan 2025 04:40:54 +1100 Subject: [PATCH] Fix 3081 - Local mirrorlist parsing bug (#3104) * Fix 3081 - shortcircuit on empty mirror options * Update * Update --- archinstall/lib/mirrors.py | 5 +- archinstall/lib/models/mirrors.py | 28 +++++++---- tests/conftest.py | 15 ++++++ .../data/mirrorlists/test_multiple_countries | 16 ++++++ tests/data/mirrorlists/test_no_country | 12 +++++ tests/data/mirrorlists/test_with_country | 13 +++++ tests/test_mirrorlist.py | 50 +++++++++++++++++++ 7 files changed, 128 insertions(+), 11 deletions(-) create mode 100644 tests/data/mirrorlists/test_multiple_countries create mode 100644 tests/data/mirrorlists/test_no_country create mode 100644 tests/data/mirrorlists/test_with_country create mode 100644 tests/test_mirrorlist.py diff --git a/archinstall/lib/mirrors.py b/archinstall/lib/mirrors.py index f85d877f..b92bc985 100644 --- a/archinstall/lib/mirrors.py +++ b/archinstall/lib/mirrors.py @@ -322,8 +322,11 @@ class MirrorMenu(AbstractSubMenu): def select_mirror_regions(preset: list[MirrorRegion]) -> list[MirrorRegion]: mirror_list_handler.load_mirrors() - available_regions = mirror_list_handler.get_mirror_regions() + + if not available_regions: + return [] + preset_regions = [region for region in available_regions if region in preset] items = [MenuItem(region.name, value=region) for region in available_regions] diff --git a/archinstall/lib/models/mirrors.py b/archinstall/lib/models/mirrors.py index f899bc5e..5798c211 100644 --- a/archinstall/lib/models/mirrors.py +++ b/archinstall/lib/models/mirrors.py @@ -145,7 +145,11 @@ class MirrorRegion: class MirrorListHandler: - def __init__(self) -> None: + def __init__( + self, + local_mirrorlist: Path = Path('/etc/pacman.d/mirrorlist'), + ) -> None: + self._local_mirrorlist = local_mirrorlist self._status_mappings: dict[str, list[MirrorStatusEntryV3]] | None = None def _mappings(self) -> dict[str, list[MirrorStatusEntryV3]]: @@ -190,7 +194,7 @@ class MirrorListHandler: return False def load_local_mirrors(self) -> None: - with Path('/etc/pacman.d/mirrorlist').open('r') as fp: + with self._local_mirrorlist.open('r') as fp: mirrorlist = fp.read() self._status_mappings = self._parse_locale_mirrors(mirrorlist) @@ -236,23 +240,26 @@ class MirrorListHandler: lines = mirrorlist.splitlines() # remove empty lines - lines = [line for line in lines if line] + # lines = [line for line in lines if line] mirror_list: dict[str, list[MirrorStatusEntryV3]] = {} current_region = '' - for idx, line in enumerate(lines): + + for line in lines: line = line.strip() - if line.lower().startswith('server'): + if line.startswith('## '): + current_region = line.replace('## ', '').strip() + mirror_list.setdefault(current_region, []) + + if line.startswith('Server = '): if not current_region: - for i in range(idx - 1, 0, -1): - if lines[i].startswith('##'): - current_region = lines[i].replace('#', '').strip() - mirror_list.setdefault(current_region, []) - break + current_region = 'Local' + mirror_list.setdefault(current_region, []) url = line.removeprefix('Server = ') + mirror_entry = MirrorStatusEntryV3( url=url.removesuffix('$repo/os/$arch'), protocol=urllib.parse.urlparse(url).scheme, @@ -267,6 +274,7 @@ class MirrorListHandler: ipv6=True, details='Locally defined mirror', ) + mirror_list[current_region].append(mirror_entry) return mirror_list diff --git a/tests/conftest.py b/tests/conftest.py index f0cabfca..66c7eae6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,3 +11,18 @@ def config_fixture() -> Path: @pytest.fixture(scope='session') def creds_fixture() -> Path: return Path(__file__).parent / 'data' / 'test_creds.json' + + +@pytest.fixture(scope='session') +def mirrorlist_no_country_fixture() -> Path: + return Path(__file__).parent / 'data' / 'mirrorlists' / 'test_no_country' + + +@pytest.fixture(scope='session') +def mirrorlist_with_country_fixture() -> Path: + return Path(__file__).parent / 'data' / 'mirrorlists' / 'test_with_country' + + +@pytest.fixture(scope='session') +def mirrorlist_multiple_countries_fixture() -> Path: + return Path(__file__).parent / 'data' / 'mirrorlists' / 'test_multiple_countries' diff --git a/tests/data/mirrorlists/test_multiple_countries b/tests/data/mirrorlists/test_multiple_countries new file mode 100644 index 00000000..d971ea71 --- /dev/null +++ b/tests/data/mirrorlists/test_multiple_countries @@ -0,0 +1,16 @@ +################################################################################ +################# Arch Linux mirrorlist generated by Reflector ################# +################################################################################ + +# With: reflector @/etc/xdg/reflector/reflector.conf +# When: 2025-01-11 05:37:57 UTC +# From: https://archlinux.org/mirrors/status/json/ +# Retrieved: 2025-01-11 05:37:17 UTC +# Last Check: 2025-01-11 05:17:17 UTC + +## United States +Server = https://geo.mirror.pkgbuild.com/$repo/os/$arch +Server = https://america.mirror.pkgbuild.com/$repo/os/$arch + +## Australia +Server = https://au.mirror.pkgbuild.com/$repo/os/$arch diff --git a/tests/data/mirrorlists/test_no_country b/tests/data/mirrorlists/test_no_country new file mode 100644 index 00000000..b42fc041 --- /dev/null +++ b/tests/data/mirrorlists/test_no_country @@ -0,0 +1,12 @@ +################################################################################ +################# Arch Linux mirrorlist generated by Reflector ################# +################################################################################ + +# With: reflector @/etc/xdg/reflector/reflector.conf +# When: 2025-01-11 05:37:57 UTC +# From: https://archlinux.org/mirrors/status/json/ +# Retrieved: 2025-01-11 05:37:17 UTC +# Last Check: 2025-01-11 05:17:17 UTC + +Server = https://geo.mirror.pkgbuild.com/$repo/os/$arch +Server = https://america.mirror.pkgbuild.com/$repo/os/$arch diff --git a/tests/data/mirrorlists/test_with_country b/tests/data/mirrorlists/test_with_country new file mode 100644 index 00000000..72111914 --- /dev/null +++ b/tests/data/mirrorlists/test_with_country @@ -0,0 +1,13 @@ +################################################################################ +################# Arch Linux mirrorlist generated by Reflector ################# +################################################################################ + +# With: reflector @/etc/xdg/reflector/reflector.conf +# When: 2025-01-11 05:37:57 UTC +# From: https://archlinux.org/mirrors/status/json/ +# Retrieved: 2025-01-11 05:37:17 UTC +# Last Check: 2025-01-11 05:17:17 UTC + +## United States +Server = https://geo.mirror.pkgbuild.com/$repo/os/$arch +Server = https://america.mirror.pkgbuild.com/$repo/os/$arch diff --git a/tests/test_mirrorlist.py b/tests/test_mirrorlist.py new file mode 100644 index 00000000..b7ddfa13 --- /dev/null +++ b/tests/test_mirrorlist.py @@ -0,0 +1,50 @@ +from pathlib import Path + +from archinstall.lib.models.mirrors import MirrorListHandler + + +def test_mirrorlist_no_country(mirrorlist_no_country_fixture: Path) -> None: + handler = MirrorListHandler(local_mirrorlist=mirrorlist_no_country_fixture) + handler.load_local_mirrors() + + regions = handler.get_mirror_regions() + + assert len(regions) == 1 + assert regions[0].name == 'Local' + assert regions[0].urls == [ + 'https://geo.mirror.pkgbuild.com/$repo/os/$arch', + 'https://america.mirror.pkgbuild.com/$repo/os/$arch' + ] + + +def test_mirrorlist_with_country(mirrorlist_with_country_fixture: Path) -> None: + handler = MirrorListHandler(local_mirrorlist=mirrorlist_with_country_fixture) + handler.load_local_mirrors() + + regions = handler.get_mirror_regions() + + assert len(regions) == 1 + assert regions[0].name == 'United States' + assert regions[0].urls == [ + 'https://geo.mirror.pkgbuild.com/$repo/os/$arch', + 'https://america.mirror.pkgbuild.com/$repo/os/$arch' + ] + + +def test_mirrorlist_multiple_countries(mirrorlist_multiple_countries_fixture: Path) -> None: + handler = MirrorListHandler(local_mirrorlist=mirrorlist_multiple_countries_fixture) + handler.load_local_mirrors() + + regions = handler.get_mirror_regions() + + assert len(regions) == 2 + assert regions[0].name == 'United States' + assert regions[0].urls == [ + 'https://geo.mirror.pkgbuild.com/$repo/os/$arch', + 'https://america.mirror.pkgbuild.com/$repo/os/$arch' + ] + + assert regions[1].name == 'Australia' + assert regions[1].urls == [ + 'https://au.mirror.pkgbuild.com/$repo/os/$arch' + ]