From 6a0c174e016acf22151b13a1163855a0c9fb2f22 Mon Sep 17 00:00:00 2001 From: Dave Dalcino Date: Thu, 16 Feb 2023 13:06:12 -0800 Subject: [PATCH 1/4] Improve warning messages re: unknown aqt arguments This improves the warnings about unknown versions, modules, and architectures of Qt, so that it is more explicit what the message actually means. This changes the purpose of `Cli._check_modules_arg` from checking if modules exist in combinations.json, to returning a list of modules that do not exist in that file. The function has been renamed as well. This change was necessary to make the warning message more informative. --- aqt/installer.py | 36 ++++++++++++++++++++++++++---------- tests/test_cli.py | 17 ++++++++++++----- tests/test_install.py | 15 ++++++++++----- 3 files changed, 48 insertions(+), 20 deletions(-) diff --git a/aqt/installer.py b/aqt/installer.py index c6cc0c77..3b4aaa11 100644 --- a/aqt/installer.py +++ b/aqt/installer.py @@ -257,13 +257,14 @@ def _check_mirror(self, mirror): return False return True - def _check_modules_arg(self, qt_version, modules): + def _select_unexpected_modules(self, qt_version: str, modules: Optional[List[str]]) -> List[str]: + """Returns a sorted list of all the requested modules that do not exist in the combinations.json file.""" if modules is None: - return True + return [] available = Settings.available_modules(qt_version) if available is None: - return False - return all([m in available for m in modules]) + return sorted(modules) + return sorted(set(modules) - set(available)) @staticmethod def _determine_qt_version( @@ -388,14 +389,23 @@ def to_archives(baseurl: str) -> QtArchives: auto_desktop_archives: List[QtPackage] = get_auto_desktop_archives() if not self._check_qt_arg_versions(qt_version): - self.logger.warning("Specified Qt version is unknown: {}.".format(qt_version)) + self.logger.warning( + f'Specified Qt version "{qt_version}" did not exist when this version of aqtinstall was released. ' + "This may not install properly, but we will try our best." + ) if not self._check_qt_arg_combination(qt_version, os_name, target, arch): self.logger.warning( - "Specified target combination is not valid or unknown: {} {} {}".format(os_name, target, arch) + f'Specified target combination "{os_name} {target} {arch}" did not exist when this version of ' + "aqtinstall was released. This may not install properly, but we will try our best." ) all_extra = True if modules is not None and "all" in modules else False - if not all_extra and not self._check_modules_arg(qt_version, modules): - self.logger.warning("Some of specified modules are unknown.") + if not all_extra: + unexpected_modules = self._select_unexpected_modules(qt_version, modules) + if unexpected_modules: + self.logger.warning( + f"Specified modules {unexpected_modules} did not exist when this version of aqtinstall was released. " + "This may not install properly, but we will try our best." + ) qt_archives: QtArchives = retry_on_bad_connection( lambda base_url: QtArchives( @@ -465,7 +475,10 @@ def _run_src_doc_examples(self, flavor, args, cmd_name: Optional[str] = None): archives = args.archives all_extra = True if modules is not None and "all" in modules else False if not self._check_qt_arg_versions(qt_version): - self.logger.warning("Specified Qt version is unknown: {}.".format(qt_version)) + self.logger.warning( + f'Specified Qt version "{qt_version}" did not exist when this version of aqtinstall was released. ' + "This may not install properly, but we will try our best." + ) srcdocexamples_archives: SrcDocExamplesArchives = retry_on_bad_connection( lambda base_url: SrcDocExamplesArchives( @@ -562,7 +575,10 @@ def run_install_tool(self, args: InstallToolArgParser): for arch in archs: if not self._check_tools_arg_combination(os_name, tool_name, arch): - self.logger.warning("Specified target combination is not valid: {} {} {}".format(os_name, tool_name, arch)) + self.logger.warning( + f'Specified target combination "{os_name} {tool_name} {arch}" did not exist when this version of ' + "aqtinstall was released. This may not install properly, but we will try our best." + ) tool_archives: ToolArchives = retry_on_bad_connection( lambda base_url: ToolArchives( diff --git a/tests/test_cli.py b/tests/test_cli.py index 055c949f..3ae35f16 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -53,13 +53,20 @@ def test_cli_help(capsys): assert expected_help(out) -def test_cli_check_module(): +@pytest.mark.parametrize( + "qt_version, modules, unexpected_modules", + ( + ("5.11.3", ["qtcharts", "qtwebengine"], []), + ("5.11.3", ["not_exist"], ["not_exist"]), + ("5.11.3", ["qtcharts", "qtwebengine", "not_exist"], ["not_exist"]), + ("5.11.3", None, []), + ("5.15.0", ["Unknown"], ["Unknown"]), + ), +) +def test_cli_select_unexpected_modules(qt_version: str, modules: Optional[List[str]], unexpected_modules: List[str]): cli = Cli() cli._setup_settings() - assert cli._check_modules_arg("5.11.3", ["qtcharts", "qtwebengine"]) - assert not cli._check_modules_arg("5.7", ["not_exist"]) - assert cli._check_modules_arg("5.14.0", None) - assert not cli._check_modules_arg("5.15.0", ["Unknown"]) + assert cli._select_unexpected_modules(qt_version, modules) == unexpected_modules def test_cli_check_combination(): diff --git a/tests/test_install.py b/tests/test_install.py index 9cae23d4..81a650a3 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1052,7 +1052,8 @@ def test_install( ( "install-qt windows desktop 5.16.0 win32_mingw73", None, - "WARNING : Specified Qt version is unknown: 5.16.0.\n" + 'WARNING : Specified Qt version "5.16.0" did not exist when this version of aqtinstall was released. ' + "This may not install properly, but we will try our best.\n" "ERROR : Failed to locate XML data for Qt version '5.16.0'.\n" "==============================Suggested follow-up:==============================\n" "* Please use 'aqt list-qt windows desktop' to show versions available.\n", @@ -1060,7 +1061,8 @@ def test_install( ( "install-qt windows desktop 5.15.0 bad_arch", "windows-5150-update.xml", - "WARNING : Specified target combination is not valid or unknown: windows desktop bad_arch\n" + 'WARNING : Specified target combination "windows desktop bad_arch" did not exist when this version of ' + "aqtinstall was released. This may not install properly, but we will try our best.\n" "ERROR : The packages ['qt_base'] were not found while parsing XML of package information!\n" "==============================Suggested follow-up:==============================\n" "* Please use 'aqt list-qt windows desktop --arch 5.15.0' to show architectures available.\n", @@ -1068,7 +1070,8 @@ def test_install( ( "install-qt windows desktop 5.15.0 win32_mingw73 -m nonexistent foo", "windows-5150-update.xml", - "WARNING : Some of specified modules are unknown.\n" + "WARNING : Specified modules ['foo', 'nonexistent'] did not exist when this version of aqtinstall " + "was released. This may not install properly, but we will try our best.\n" "ERROR : The packages ['foo', 'nonexistent', 'qt_base'] were not found" " while parsing XML of package information!\n" "==============================Suggested follow-up:==============================\n" @@ -1106,7 +1109,8 @@ def test_install( ( "install-tool windows desktop tools_vcredist nonexistent", "windows-desktop-tools_vcredist-update.xml", - "WARNING : Specified target combination is not valid: windows tools_vcredist nonexistent\n" + 'WARNING : Specified target combination "windows tools_vcredist nonexistent" did not exist when this version of ' + "aqtinstall was released. This may not install properly, but we will try our best.\n" "ERROR : The package 'nonexistent' was not found while parsing XML of package information!\n" "==============================Suggested follow-up:==============================\n" "* Please use 'aqt list-tool windows desktop tools_vcredist' to show tool variants available.\n", @@ -1114,7 +1118,8 @@ def test_install( ( "install-tool windows desktop tools_nonexistent nonexistent", None, - "WARNING : Specified target combination is not valid: windows tools_nonexistent nonexistent\n" + 'WARNING : Specified target combination "windows tools_nonexistent nonexistent" did not exist when this ' + "version of aqtinstall was released. This may not install properly, but we will try our best.\n" "ERROR : Failed to locate XML data for the tool 'tools_nonexistent'.\n" "==============================Suggested follow-up:==============================\n" "* Please use 'aqt list-tool windows desktop' to show tools available.\n", From a6db5da83d866930dfbcb84e87ffa25b1388fcf0 Mon Sep 17 00:00:00 2001 From: Dave Dalcino Date: Thu, 16 Feb 2023 14:12:04 -0800 Subject: [PATCH 2/4] Refactor warning messages into reusable functions This will reduce code duplication and make the warning messages easier to improve in the future, since they will only need to be changed in one location. --- aqt/installer.py | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/aqt/installer.py b/aqt/installer.py index 3b4aaa11..d574958c 100644 --- a/aqt/installer.py +++ b/aqt/installer.py @@ -213,6 +213,21 @@ def _check_qt_arg_versions(self, version): def _check_qt_arg_version_offline(self, version): return version in Settings.available_offline_installer_version + def _warning_unknown_qt_version(self, qt_version: str) -> str: + return self._warning_on_bad_combination(f'Qt version "{qt_version}"') + + def _warning_unknown_target_arch_combo(self, args: List[str]) -> str: + return self._warning_on_bad_combination(f"target combination \"{' '.join(args)}\"") + + def _warning_unexpected_modules(self, unexpected_modules: List[str]) -> str: + return self._warning_on_bad_combination(f"modules {unexpected_modules}") + + def _warning_on_bad_combination(self, combo_message: str) -> str: + return ( + f"Specified {combo_message} did not exist when this version of aqtinstall was released. " + "This may not install properly, but we will try our best." + ) + def _set_sevenzip(self, external): sevenzip = external if sevenzip is None: @@ -389,23 +404,14 @@ def to_archives(baseurl: str) -> QtArchives: auto_desktop_archives: List[QtPackage] = get_auto_desktop_archives() if not self._check_qt_arg_versions(qt_version): - self.logger.warning( - f'Specified Qt version "{qt_version}" did not exist when this version of aqtinstall was released. ' - "This may not install properly, but we will try our best." - ) + self.logger.warning(self._warning_unknown_qt_version(qt_version)) if not self._check_qt_arg_combination(qt_version, os_name, target, arch): - self.logger.warning( - f'Specified target combination "{os_name} {target} {arch}" did not exist when this version of ' - "aqtinstall was released. This may not install properly, but we will try our best." - ) + self.logger.warning(self._warning_unknown_target_arch_combo([os_name, target, arch])) all_extra = True if modules is not None and "all" in modules else False if not all_extra: unexpected_modules = self._select_unexpected_modules(qt_version, modules) if unexpected_modules: - self.logger.warning( - f"Specified modules {unexpected_modules} did not exist when this version of aqtinstall was released. " - "This may not install properly, but we will try our best." - ) + self.logger.warning(self._warning_unexpected_modules(unexpected_modules)) qt_archives: QtArchives = retry_on_bad_connection( lambda base_url: QtArchives( @@ -475,10 +481,7 @@ def _run_src_doc_examples(self, flavor, args, cmd_name: Optional[str] = None): archives = args.archives all_extra = True if modules is not None and "all" in modules else False if not self._check_qt_arg_versions(qt_version): - self.logger.warning( - f'Specified Qt version "{qt_version}" did not exist when this version of aqtinstall was released. ' - "This may not install properly, but we will try our best." - ) + self.logger.warning(self._warning_unknown_qt_version(qt_version)) srcdocexamples_archives: SrcDocExamplesArchives = retry_on_bad_connection( lambda base_url: SrcDocExamplesArchives( @@ -575,10 +578,7 @@ def run_install_tool(self, args: InstallToolArgParser): for arch in archs: if not self._check_tools_arg_combination(os_name, tool_name, arch): - self.logger.warning( - f'Specified target combination "{os_name} {tool_name} {arch}" did not exist when this version of ' - "aqtinstall was released. This may not install properly, but we will try our best." - ) + self.logger.warning(self._warning_unknown_target_arch_combo([os_name, tool_name, arch])) tool_archives: ToolArchives = retry_on_bad_connection( lambda base_url: ToolArchives( From 9b102de2b40b6ccd769baec467a1f223857f9bd0 Mon Sep 17 00:00:00 2001 From: Dave Dalcino Date: Thu, 16 Feb 2023 14:53:56 -0800 Subject: [PATCH 3/4] Test Cli._select_unexpected_modules for missing qt This adds coverage for the situation where there is no known list of modules, since the Qt version is unknown. --- tests/test_cli.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index 3ae35f16..91a568bd 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -68,6 +68,9 @@ def test_cli_select_unexpected_modules(qt_version: str, modules: Optional[List[s cli._setup_settings() assert cli._select_unexpected_modules(qt_version, modules) == unexpected_modules + nonexistent_qt = "5.16.0" + assert cli._select_unexpected_modules(nonexistent_qt, modules) == sorted(modules or []) + def test_cli_check_combination(): cli = Cli() From 265c95d11635ef217718ec9840b05d9a70c246c5 Mon Sep 17 00:00:00 2001 From: Dave Dalcino Date: Thu, 16 Feb 2023 14:55:16 -0800 Subject: [PATCH 4/4] Simplify Cli._select_unexpected_modules logic This function simply returns a set difference, so there's no need for it to be any more complicated than that. --- aqt/installer.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/aqt/installer.py b/aqt/installer.py index d574958c..4af89c81 100644 --- a/aqt/installer.py +++ b/aqt/installer.py @@ -274,12 +274,8 @@ def _check_mirror(self, mirror): def _select_unexpected_modules(self, qt_version: str, modules: Optional[List[str]]) -> List[str]: """Returns a sorted list of all the requested modules that do not exist in the combinations.json file.""" - if modules is None: - return [] available = Settings.available_modules(qt_version) - if available is None: - return sorted(modules) - return sorted(set(modules) - set(available)) + return sorted(set(modules or []) - set(available or [])) @staticmethod def _determine_qt_version(