Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log a warning when aqtinstall falls back to an external 7z extraction tool #705

Merged
merged 8 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 12 additions & 15 deletions aqt/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,21 +233,27 @@ def _warning_on_bad_combination(self, combo_message: str) -> str:
"This may not install properly, but we will try our best."
)

def _set_sevenzip(self, external):
def _set_sevenzip(self, external: Optional[str]) -> Optional[str]:
sevenzip = external
fallback = Settings.zipcmd
if sevenzip is None:
return None

if EXT7Z:
self.logger.warning(f"The py7zr module failed to load. Falling back to '{fallback}' for .7z extraction.")
self.logger.warning("You can use the '--external | -E' flags to select your own extraction tool.")
sevenzip = fallback
else:
# Just use py7zr
return None
try:
subprocess.run(
[sevenzip, "--help"],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
return sevenzip
except FileNotFoundError as e:
raise CliInputError("Specified 7zip command executable does not exist: {!r}".format(sevenzip)) from e

return sevenzip
qualifier = "Specified" if sevenzip == external else "Fallback"
raise CliInputError(f"{qualifier} 7zip command executable does not exist: '{sevenzip}'") from e

@staticmethod
def _set_arch(arch: Optional[str], os_name: str, target: str, qt_version_or_spec: str) -> str:
Expand Down Expand Up @@ -358,9 +364,6 @@ def run_install_qt(self, args: InstallArgParser):
timeout = (Settings.connection_timeout, Settings.response_timeout)
modules = args.modules
sevenzip = self._set_sevenzip(args.external)
if EXT7Z and sevenzip is None:
# override when py7zr is not exist
sevenzip = self._set_sevenzip("7z")
if args.base is not None:
if not self._check_mirror(args.base):
raise CliInputError(
Expand Down Expand Up @@ -475,9 +478,6 @@ def _run_src_doc_examples(self, flavor, args, cmd_name: Optional[str] = None):
else:
timeout = (Settings.connection_timeout, Settings.response_timeout)
sevenzip = self._set_sevenzip(args.external)
if EXT7Z and sevenzip is None:
# override when py7zr is not exist
sevenzip = self._set_sevenzip(Settings.zipcmd)
modules = getattr(args, "modules", None) # `--modules` is invalid for `install-src`
archives = args.archives
all_extra = True if modules is not None and "all" in modules else False
Expand Down Expand Up @@ -549,9 +549,6 @@ def run_install_tool(self, args: InstallToolArgParser):
else:
base_dir = output_dir
sevenzip = self._set_sevenzip(args.external)
if EXT7Z and sevenzip is None:
# override when py7zr is not exist
sevenzip = self._set_sevenzip(Settings.zipcmd)
version = getattr(args, "version", None)
if version is not None:
Cli._validate_version_str(version, allow_minus=True)
Expand Down
61 changes: 56 additions & 5 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import pytest

from aqt.exceptions import CliInputError
from aqt.helper import Settings
from aqt.installer import Cli
from aqt.metadata import MetadataFactory, SimpleSpec, Version

Expand Down Expand Up @@ -382,13 +383,63 @@ def _mocked_run(*args):
)


def test_cli_set_7zip(monkeypatch):
@pytest.mark.parametrize("external_tool_exists", (True, False))
def test_set_7zip_checks_external_tool_when_specified(monkeypatch, capsys, external_tool_exists: bool):
cli = Cli()
cli._setup_settings()
with pytest.raises(CliInputError) as err:
cli._set_sevenzip("some_nonexistent_binary")
assert err.type == CliInputError
assert format(err.value) == "Specified 7zip command executable does not exist: 'some_nonexistent_binary'"
external = "my_7z_extractor"

def mock_subprocess_run(args, **kwargs):
assert args[0] == external
if not external_tool_exists:
raise FileNotFoundError()

monkeypatch.setattr("aqt.installer.subprocess.run", mock_subprocess_run)
monkeypatch.setattr("aqt.installer.EXT7Z", False)
if external_tool_exists:
assert external == cli._set_sevenzip(external)
else:
with pytest.raises(CliInputError) as err:
cli._set_sevenzip(external)
assert format(err.value) == format(f"Specified 7zip command executable does not exist: '{external}'")
assert capsys.readouterr()[1] == ""


@pytest.mark.parametrize("fallback_exists", (True, False))
def test_set_7zip_uses_fallback_when_py7zr_missing(monkeypatch, capsys, fallback_exists: bool):
cli = Cli()
cli._setup_settings()
external, fallback = None, Settings.zipcmd

def mock_subprocess_run(args, **kwargs):
assert args[0] == fallback
if not fallback_exists:
raise FileNotFoundError()

monkeypatch.setattr("aqt.installer.subprocess.run", mock_subprocess_run)
monkeypatch.setattr("aqt.installer.EXT7Z", True)
if fallback_exists:
assert fallback == cli._set_sevenzip(external)
else:
with pytest.raises(CliInputError) as err:
cli._set_sevenzip(external)
assert format(err.value) == format(f"Fallback 7zip command executable does not exist: '{fallback}'")
assert f"Falling back to '{fallback}'" in capsys.readouterr()[1]


@pytest.mark.parametrize("fallback_exists", (True, False))
def test_set_7zip_chooses_p7zr_when_ext_missing(monkeypatch, capsys, fallback_exists: bool):
cli = Cli()
cli._setup_settings()
external = None

def mock_subprocess_run(args, **kwargs):
assert False, "Should not try to run anything"

monkeypatch.setattr("aqt.installer.subprocess.run", mock_subprocess_run)
monkeypatch.setattr("aqt.installer.EXT7Z", False)
assert cli._set_sevenzip(external) is None
assert capsys.readouterr()[1] == ""


@pytest.mark.parametrize(
Expand Down