Fix errors on selection of additional packages (#959)

* Fix errors on selection of additional packages

* Fix flake8

* Added the new /groups/search/json/?name=x endpoint merged today

* Fixed flake8 complaint

* Forgot to do json.loads() on the HTTP request result

* Update package selection

* Fix flake8

Co-authored-by: Daniel Girtler <girtler.daniel@gmail.com>
Co-authored-by: Anton Hvornum <anton@hvornum.se>

archinstall/__init__.py

@@ -21,13 +21,12 @@ from .lib.models.dataclasses import (
 	LocalPackage
 )
 from .lib.packages.packages import (
-	find_group,
+	group_search,
 	package_search,
-	IsGroup,
 	find_package,
 	find_packages,
 	installed_package,
-	validate_package_list
+	validate_package_list,
 )
 from .lib.profiles import *
 from .lib.services import *

archinstall/lib/menu/text_input.py

@@ -0,0 +1,17 @@
+import readline
+
+
+class TextInput:
+	def __init__(self, prompt: str, prefilled_text=''):
+		self._prompt = prompt
+		self._prefilled_text = prefilled_text
+
+	def _hook(self):
+		readline.insert_text(self._prefilled_text)
+		readline.redisplay()
+
+	def run(self) -> str:
+		readline.set_pre_input_hook(self._hook)
+		result = input(self._prompt)
+		readline.set_pre_input_hook()
+		return result

archinstall/lib/packages/packages.py

@@ -1,17 +1,18 @@
+import json
 import ssl
 import urllib.request
-import json
-from typing import Dict, Any
+from typing import Dict, Any, Tuple, List
+
+from ..exceptions import PackageError, SysCallError
 from ..general import SysCommand
 from ..models.dataclasses import PackageSearch, PackageSearchResult, LocalPackage
-from ..exceptions import PackageError, SysCallError, RequirementError
 
 BASE_URL_PKG_SEARCH = 'https://archlinux.org/packages/search/json/?name={package}'
 # BASE_URL_PKG_CONTENT = 'https://archlinux.org/packages/search/json/'
-BASE_GROUP_URL = 'https://archlinux.org/groups/x86_64/{group}/'
+BASE_GROUP_URL = 'https://archlinux.org/groups/search/json/?name={group}'
 
 
-def find_group(name :str) -> bool:
+def group_search(name :str) -> List[PackageSearchResult]:
 	# TODO UPSTREAM: Implement /json/ for the groups search
 	ssl_context = ssl.create_default_context()
 	ssl_context.check_hostname = False
@@ -20,15 +21,15 @@ def find_group(name :str) -> bool:
 		response = urllib.request.urlopen(BASE_GROUP_URL.format(group=name), context=ssl_context)
 	except urllib.error.HTTPError as err:
 		if err.code == 404:
-			return False
+			return []
 		else:
 			raise err
 
 	# Just to be sure some code didn't slip through the exception
-	if response.code == 200:
-		return True
+	data = response.read().decode('UTF-8')
+
+	return [PackageSearchResult(**package) for package in json.loads(data)['results']]
 
-	return False
 
 def package_search(package :str) -> PackageSearch:
 	"""
@@ -49,28 +50,24 @@ def package_search(package :str) -> PackageSearch:
 
 	return PackageSearch(**json.loads(data))
 
-class IsGroup(BaseException):
-	pass
 
-def find_package(package :str) -> PackageSearchResult:
+def find_package(package :str) -> List[PackageSearchResult]:
 	data = package_search(package)
+	results = []
 
-	if not data.results:
-		# Check if the package is actually a group
-		if find_group(package):
-			# TODO: Until upstream adds a JSON result for group searches
-			# there is no way we're going to parse HTML reliably.
-			raise IsGroup("Implement group search")
-
-		raise PackageError(f"Could not locate {package} while looking for repository category")
+	for result in data.results:
+		if result.pkgname == package:
+			results.append(result)
 
 	# If we didn't find the package in the search results,
 	# odds are it's a group package
-	for result in data.results:
-		if result.pkgname == package:
-			return result
+	if not results:
+		# Check if the package is actually a group
+		for result in group_search(package):
+			results.append(result)
+
+	return results
 
-	raise PackageError(f"Could not locate {package} in result while looking for repository category")
 
 def find_packages(*names :str) -> Dict[str, Any]:
 	"""
@@ -78,23 +75,25 @@ def find_packages(*names :str) -> Dict[str, Any]:
 	The function itself is rather slow, so consider not sending to
 	many packages to the search query.
 	"""
-	return {package: find_package(package) for package in names}
+	result = {}
+	for package in names:
+		for found_package in find_package(package):
+			result[package] = found_package
+
+	return result
 
 
-def validate_package_list(packages: list) -> bool:
+def validate_package_list(packages :list) -> Tuple[list, list]:
 	"""
 	Validates a list of given packages.
-	Raises `RequirementError` if one or more packages are not found.
+	return: Tuple of lists containing valid packavges in the first and invalid
+	packages in the second entry
 	"""
-	invalid_packages = [
-		package
-		for package in packages
-		if not find_package(package)['results'] and not find_group(package)
-	]
-	if invalid_packages:
-		raise RequirementError(f"Invalid package names: {invalid_packages}")
+	valid_packages = {package for package in packages if find_package(package)}
+	invalid_packages = set(packages) - valid_packages
+
+	return list(valid_packages), list(invalid_packages)
 
-	return True
 
 def installed_package(package :str) -> LocalPackage:
 	package_info = {}
@@ -105,5 +104,5 @@ def installed_package(package :str) -> LocalPackage:
 				package_info[key.strip().lower().replace(' ', '_')] = value.strip()
 	except SysCallError:
 		pass
-	
-	return LocalPackage(**package_info)
+
+	return LocalPackage(**package_info)

archinstall/lib/user_interaction.py

@@ -12,6 +12,8 @@ from collections.abc import Iterable
 from typing import List, Any, Optional, Dict, Union, TYPE_CHECKING
 
 # https://stackoverflow.com/a/39757388/929999
+from .menu.text_input import TextInput
+
 if TYPE_CHECKING:
 	from .disk.partition import Partition
 
@@ -32,6 +34,7 @@ from .translation import Translation
 from .disk.validators import fs_types
 from .packages.packages import validate_package_list
 
+
 # TODO: These can be removed after the move to simple_menu.py
 def get_terminal_height() -> int:
 	return shutil.get_terminal_size().lines
@@ -390,28 +393,33 @@ def ask_for_audio_selection(desktop :bool = True) -> str:
 	return selected_audio
 
 
-# TODO: Remove? Moved?
-def ask_additional_packages_to_install(packages :List[str] = None) -> List[str]:
+def ask_additional_packages_to_install(pre_set_packages :List[str] = []) -> List[str]:
 	# Additional packages (with some light weight error handling for invalid package names)
 	print(_('Only packages such as base, base-devel, linux, linux-firmware, efibootmgr and optional profile packages are installed.'))
 	print(_('If you desire a web browser, such as firefox or chromium, you may specify it in the following prompt.'))
 
-	while True:
-		packages = [p for p in input(
-			_('Write additional packages to install (space separated, leave blank to skip): ')
-		).split(' ') if len(p)]
+	def read_packages(already_defined: list = []) -> list:
+		display = ' '.join(already_defined)
+		input_packages = TextInput(
+			_('Write additional packages to install (space separated, leave blank to skip): '),
+			display
+		).run()
+		return input_packages.split(' ') if input_packages else []
 
+	pre_set_packages = pre_set_packages if pre_set_packages else []
+	packages = read_packages(pre_set_packages)
+
+	while True:
 		if len(packages):
 			# Verify packages that were given
-			try:
-				print(_("Verifying that additional packages exist (this might take a few seconds)"))
-				validate_package_list(packages)
-				break
-			except RequirementError as e:
-				log(e, fg='red')
-		else:
-			# no additional packages were selected, which we'll allow
-			break
+			print(_("Verifying that additional packages exist (this might take a few seconds)"))
+			valid, invalid = validate_package_list(packages)
+
+			if invalid:
+				log(f"Some packages could not be found in the repository: {invalid}", level=logging.WARNING, fg='red')
+				packages = read_packages(valid)
+				continue
+		break
 
 	return packages
 

examples/guided.py

@@ -241,10 +241,12 @@ if not (archinstall.check_mirror_reachable() or archinstall.arguments.get('skip-
 	exit(1)
 
 if not archinstall.arguments.get('offline', False):
+	latest_version_archlinux_keyring = max([k.pkg_version for k in archinstall.find_package('archlinux-keyring')])
+
 	# If we want to check for keyring updates
 	# and the installed package version is lower than the upstream version
 	if archinstall.arguments.get('skip-keyring-update', False) is False and \
-		archinstall.installed_package('archlinux-keyring') < archinstall.find_package('archlinux-keyring'):
+		archinstall.installed_package('archlinux-keyring').version < latest_version_archlinux_keyring:
 
 		# Then we update the keyring in the ISO environment
 		if not archinstall.update_keyring():