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

Conversation

ddalcino
Copy link
Contributor

@ddalcino ddalcino commented Aug 27, 2023

Inspired by #696 (comment). That comment suggested a place to add a log message, but I was unable to log the message there without disregarding the user’s selected log settings, which are not available until the Cli object is built.

When aqtinstall fails to load the py7zr module, it falls back to whatever 7z extraction tool the user has specified in the settings.ini file, without notifying the user. This PR adds the following log message when that happens:

WARNING: The py7zr module failed to load. Falling back to '7z' for .7z extraction.
WARNING: You can use the  '--external | -E' flags to select your own extraction tool.

This PR also does a little refactoring of the Cli._set_sevenzip method to improve readability.

Side effect
There was an existing bug here, where the install-qt subcommand (but none of the other commands) would ignore the 7zcmd parameter in the settings.ini file, and use the hardcoded "7z" instead. I'm pretty sure this was a bug; if not I can change it back.

aqtinstall/aqt/installer.py

Lines 361 to 363 in 72da54f

if EXT7Z and sevenzip is None:
# override when py7zr is not exist
sevenzip = self._set_sevenzip("7z")

@ddalcino ddalcino force-pushed the topic/ddalcino/cli/warn-ext7zr branch from 0a8efd8 to 2bc3766 Compare August 28, 2023 12:29
@miurahr
Copy link
Owner

miurahr commented Sep 11, 2023

Thank you for the improvement.

@miurahr miurahr merged commit c059593 into miurahr:master Sep 11, 2023
58 checks passed
@ddalcino ddalcino deleted the topic/ddalcino/cli/warn-ext7zr branch September 17, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants