diff --git a/src/Mod/AddonManager/AddonManagerTest/app/test_dependency_installer.py b/src/Mod/AddonManager/AddonManagerTest/app/test_dependency_installer.py index e0ba09cad1..c70e08491c 100644 --- a/src/Mod/AddonManager/AddonManagerTest/app/test_dependency_installer.py +++ b/src/Mod/AddonManager/AddonManagerTest/app/test_dependency_installer.py @@ -136,12 +136,6 @@ class TestDependencyInstaller(unittest.TestCase): self.assertTrue(ff_required.called) self.assertTrue(ff_optional.called) - def test_verify_pip_no_python(self): - self.test_object._get_python = lambda: None - should_continue = self.test_object._verify_pip() - self.assertFalse(should_continue) - self.assertEqual(len(self.signals_caught), 0) - def test_verify_pip_no_pip(self): sm = SubprocessMock() sm.succeed = False diff --git a/src/Mod/AddonManager/addonmanager_dependency_installer.py b/src/Mod/AddonManager/addonmanager_dependency_installer.py index 466809e2de..edead4ff53 100644 --- a/src/Mod/AddonManager/addonmanager_dependency_installer.py +++ b/src/Mod/AddonManager/addonmanager_dependency_installer.py @@ -27,8 +27,6 @@ import os import subprocess from typing import List -from freecad.utils import get_python_exe - import addonmanager_freecad_interface as fci from addonmanager_pyside_interface import QObject, Signal, is_interruption_requested @@ -46,7 +44,7 @@ class DependencyInstaller(QObject): no_python_exe = Signal() no_pip = Signal(str) # Attempted command failure = Signal(str, str) # Short message, detailed message - finished = Signal() + finished = Signal(bool) # True if everything completed normally, otherwise false def __init__( self, @@ -65,17 +63,25 @@ class DependencyInstaller(QObject): self.python_requires = python_requires self.python_optional = python_optional self.location = location + self.required_succeeded = False + self.finished_successfully = False def run(self): """Normally not called directly, but rather connected to the worker thread's started signal.""" - if self._verify_pip(): + try: if self.python_requires or self.python_optional: - if not is_interruption_requested(): - self._install_python_packages() - if not is_interruption_requested(): - self._install_addons() - self.finished.emit() + if self._verify_pip(): + if not is_interruption_requested(): + self._install_python_packages() + else: + self.required_succeeded = True + if not is_interruption_requested(): + self._install_addons() + self.finished_successfully = self.required_succeeded + except RuntimeError: + pass + self.finished.emit(self.finished_successfully) def _install_python_packages(self): """Install required and optional Python dependencies using pip.""" @@ -87,20 +93,20 @@ class DependencyInstaller(QObject): if not os.path.exists(vendor_path): os.makedirs(vendor_path) - self._install_required(vendor_path) + self.required_succeeded = self._install_required(vendor_path) self._install_optional(vendor_path) def _verify_pip(self) -> bool: """Ensure that pip is working -- returns True if it is, or False if not. Also emits the no_pip signal if pip cannot execute.""" - python_exe = self._get_python() - if not python_exe: - return False try: proc = self._run_pip(["--version"]) fci.Console.PrintMessage(proc.stdout + "\n") + if proc.returncode != 0: + return False except subprocess.CalledProcessError: - self.no_pip.emit(f"{python_exe} -m pip --version") + call = utils.create_pip_call([]) + self.no_pip.emit(" ".join(call)) return False return True @@ -115,7 +121,6 @@ class DependencyInstaller(QObject): proc = self._run_pip( [ "install", - "--disable-pip-version-check", "--target", vendor_path, pymod, @@ -144,7 +149,6 @@ class DependencyInstaller(QObject): proc = self._run_pip( [ "install", - "--disable-pip-version-check", "--target", vendor_path, pymod, @@ -160,22 +164,13 @@ class DependencyInstaller(QObject): ) def _run_pip(self, args): - python_exe = self._get_python() - final_args = [python_exe, "-m", "pip"] - final_args.extend(args) + final_args = utils.create_pip_call(args) return self._subprocess_wrapper(final_args) @staticmethod def _subprocess_wrapper(args) -> subprocess.CompletedProcess: """Wrap subprocess call so test code can mock it.""" - return utils.run_interruptable_subprocess(args) - - def _get_python(self) -> str: - """Wrap Python access so test code can mock it.""" - python_exe = get_python_exe() - if not python_exe: - self.no_python_exe.emit() - return python_exe + return utils.run_interruptable_subprocess(args, timeout_secs=120) def _install_addons(self): for addon in self.addons: diff --git a/src/Mod/AddonManager/addonmanager_installer_gui.py b/src/Mod/AddonManager/addonmanager_installer_gui.py index 7eec964e52..c2139f4f05 100644 --- a/src/Mod/AddonManager/addonmanager_installer_gui.py +++ b/src/Mod/AddonManager/addonmanager_installer_gui.py @@ -77,14 +77,26 @@ class AddonInstallerGUI(QtCore.QObject): self.installer.failure.connect(self._installation_failed) def __del__(self): - if self.worker_thread and hasattr(self.worker_thread, "quit"): - self.worker_thread.quit() - self.worker_thread.wait(500) - if self.worker_thread.isRunning(): + self._stop_thread(self.worker_thread) + self._stop_thread(self.dependency_worker_thread) + + @staticmethod + def _stop_thread(thread: QtCore.QThread): + if thread and hasattr(thread, "quit"): + if thread.isRunning(): + FreeCAD.Console.PrintMessage( + "INTERNAL ERROR: a QThread is still running when it should have finished" + ) + + thread.requestInterruption() + thread.wait(100) + thread.quit() + thread.wait(500) + if thread.isRunning(): FreeCAD.Console.PrintError( "INTERNAL ERROR: Thread did not quit() cleanly, using terminate()\n" ) - self.worker_thread.terminate() + thread.terminate() def run(self): """Instructs this class to begin displaying the necessary dialogs to guide a user through @@ -300,13 +312,11 @@ class AddonInstallerGUI(QtCore.QObject): self.dependency_installer.no_python_exe.connect(self._report_no_python_exe) self.dependency_installer.no_pip.connect(self._report_no_pip) self.dependency_installer.failure.connect(self._report_dependency_failure) - self.dependency_installer.finished.connect(self._cleanup_dependency_worker) - self.dependency_installer.finished.connect(self._report_dependency_success) + self.dependency_installer.finished.connect(self._dependencies_finished) self.dependency_worker_thread = QtCore.QThread(self) self.dependency_installer.moveToThread(self.dependency_worker_thread) self.dependency_worker_thread.started.connect(self.dependency_installer.run) - self.dependency_installer.finished.connect(self.dependency_worker_thread.quit) self.dependency_installation_dialog = QtWidgets.QMessageBox( QtWidgets.QMessageBox.Information, @@ -319,16 +329,6 @@ class AddonInstallerGUI(QtCore.QObject): self.dependency_installation_dialog.show() self.dependency_worker_thread.start() - def _cleanup_dependency_worker(self) -> None: - return - self.dependency_worker_thread.quit() - self.dependency_worker_thread.wait(500) - if self.dependency_worker_thread.isRunning(): - FreeCAD.Console.PrintError( - "INTERNAL ERROR: Thread did not quit() cleanly, using terminate()\n" - ) - self.dependency_worker_thread.terminate() - def _report_no_python_exe(self) -> None: """Callback for the dependency installer failing to locate a Python executable.""" if self.dependency_installation_dialog is not None: @@ -409,6 +409,11 @@ class AddonInstallerGUI(QtCore.QObject): self.dependency_installation_dialog.hide() self.install() + def _dependencies_finished(self, success: bool): + if success: + self._report_dependency_success() + self.dependency_worker_thread.quit() + def _dependency_dialog_ignore_clicked(self) -> None: """Callback for when dependencies are ignored.""" self.install() diff --git a/src/Mod/AddonManager/addonmanager_python_deps_gui.py b/src/Mod/AddonManager/addonmanager_python_deps_gui.py index f8225b057d..73e0ffc244 100644 --- a/src/Mod/AddonManager/addonmanager_python_deps_gui.py +++ b/src/Mod/AddonManager/addonmanager_python_deps_gui.py @@ -33,6 +33,7 @@ import subprocess import sys from functools import partial from typing import Dict, Iterable, List, Tuple, TypedDict +from addonmanager_utilities import create_pip_call import addonmanager_freecad_interface as fci @@ -123,26 +124,21 @@ def call_pip(args: List[str]) -> List[str]: """Tries to locate the appropriate Python executable and run pip with version checking disabled. Fails if Python can't be found or if pip is not installed.""" - python_exe = get_python_exe() - pip_failed = False - if python_exe: - call_args = [python_exe, "-m", "pip", "--disable-pip-version-check"] - call_args.extend(args) - proc = None - try: - proc = utils.run_interruptable_subprocess(call_args) - except subprocess.CalledProcessError: - pip_failed = True + try: + call_args = create_pip_call(args) + except RuntimeError as exception: + raise PipFailed() from exception - if not pip_failed: - data = proc.stdout - return data.split("\n") - elif proc: - raise PipFailed(proc.stderr) - else: - raise PipFailed("pip timed out") - else: - raise PipFailed("Could not locate Python executable on this system") + try: + proc = utils.run_interruptable_subprocess(call_args) + except subprocess.CalledProcessError as exception: + raise PipFailed("pip timed out") from exception + + if proc.returncode != 0: + raise PipFailed(proc.stderr) + + data = proc.stdout + return data.split("\n") def parse_pip_list_output(all_packages, outdated_packages) -> Dict[str, Dict[str, str]]: diff --git a/src/Mod/AddonManager/addonmanager_utilities.py b/src/Mod/AddonManager/addonmanager_utilities.py index c378390e78..837dbc4917 100644 --- a/src/Mod/AddonManager/addonmanager_utilities.py +++ b/src/Mod/AddonManager/addonmanager_utilities.py @@ -50,9 +50,11 @@ import addonmanager_freecad_interface as fci try: from freecad.utils import get_python_exe except ImportError: + def get_python_exe(): return shutil.which("python") + if fci.FreeCADGui: # If the GUI is up, we can use the NetworkManager to handle our downloads. If there is no event