Fix 3081 - Local mirrorlist parsing bug (#3104)

* Fix 3081 - shortcircuit on empty mirror options

* Update

* Update

Author: svartkanin

archinstall/lib/mirrors.py

@@ -322,8 +322,11 @@ def run(self) -> MirrorConfiguration:
 
 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]

archinstall/lib/models/mirrors.py

@@ -145,7 +145,11 @@ def __eq__(self, other: object) -> bool:
 
 
 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 @@ def load_remote_mirrors(self) -> bool:
 		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 @@ def _parse_locale_mirrors(self, mirrorlist: str) -> dict[str, list[MirrorStatusE
 		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 @@ def _parse_locale_mirrors(self, mirrorlist: str) -> dict[str, list[MirrorStatusE
 					ipv6=True,
 					details='Locally defined mirror',
 				)
+
 				mirror_list[current_region].append(mirror_entry)
 
 		return mirror_list

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'

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

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

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

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'
+	]