Merge pull request #957 from eoyilmaz/956-startup-does-not-prompt-to-download-locate-argyllcms-when-missing

[#956] Fix ArgyllCMS missing prompt not firing on silent startup for wx and Qt

Author: eoyilmaz

DisplayCAL/VERSION

@@ -1 +1 @@
-3.10.0.dev78
+3.10.0.dev79

DisplayCAL/display_cal.py

@@ -406,12 +406,13 @@ def get_download_url(newversion: str) -> str | None:
     return None
 
 
-def is_new_update() -> bool | tuple:
+def is_new_update() -> bool | tuple | None:
     """Check for new updates on GitHub.
 
     Returns:
-        bool | tuple: The latest version tuple if a new update is available,
-        False otherwise.
+        bool | tuple | None: The latest version tuple if a new update is
+        available, False if already up to date, None if the check failed
+        (network or parsing error).
     """
     global RELEASE_DATA
     print("Checking for updates...")
@@ -423,15 +424,15 @@ def is_new_update() -> bool | tuple:
         response.raise_for_status()
     except requests.RequestException as e:
         print(f"Error checking for updates: Network error - {e!s}")
-        return False
+        return None
 
     try:
         data = response.json()
         RELEASE_DATA = data
         latest_version_tuple = tuple(int(n) for n in data["tag_name"].split("."))
     except (KeyError, ValueError, IndexError) as e:
         print(f"Error checking for updates: Parsing error - {e!s}")
-        return False
+        return None
 
     current_version = VERSION_TUPLE[:3]
     if latest_version_tuple > current_version:
@@ -500,45 +501,41 @@ def app_update_check(
             silent=silent,
         )
         if resp is False:
-            if silent:
-                # Check if we need to run instrument setup
-                wx.CallAfter(
-                    parent.check_instrument_setup, check_donation, (parent, snapshot)
-                )
-            return
-        data = resp.read()
-        if not wx.GetApp():
-            return
-        try:
-            new_version_tuple = tuple(int(n) for n in data.decode().split("."))
-        except ValueError:
-            print(lang.getstr("update_check.fail.version", DOMAIN))
-            if not silent:
-                wx.CallAfter(
-                    InfoDialog,
-                    parent,
-                    msg=lang.getstr("update_check.fail.version", DOMAIN),
-                    ok=lang.getstr("ok"),
-                    bitmap=get_icon(32, "dialog-error"),
-                    log=False,
-                )
+            # Fetch failed: treat as "no update" and fall through to the
+            # "up to date" branches below (which also check ArgyllCMS and
+            # instrument setup), regardless of silent/non-silent.
+            new_version_tuple = curversion_tuple
+        else:
+            data = resp.read()
+            if not wx.GetApp():
                 return
-            new_version_tuple = (0, 0, 0, 0)
+            try:
+                new_version_tuple = tuple(int(n) for n in data.decode().split("."))
+            except ValueError:
+                print(lang.getstr("update_check.fail.version", DOMAIN))
+                if not silent:
+                    wx.CallAfter(
+                        InfoDialog,
+                        parent,
+                        msg=lang.getstr("update_check.fail.version", DOMAIN),
+                        ok=lang.getstr("ok"),
+                        bitmap=get_icon(32, "dialog-error"),
+                        log=False,
+                    )
+                    return
+                new_version_tuple = (0, 0, 0, 0)
     else:
         # Stable
         print(lang.getstr("update_check"))
         curversion_tuple = VERSION_TUPLE
         chglog_file = "CHANGES.html"
         resp = is_new_update()
-        if resp is False:
-            if silent:
-                # Check if we need to run instrument setup
-                wx.CallAfter(
-                    parent.check_instrument_setup, check_donation, (parent, snapshot)
-                )
-                return
-            # Non-silent with no update available: fall through to the
-            # "up to date" branches below using the current version.
+        if resp is False or resp is None:
+            # No update available, or the check itself failed (network/parse
+            # error): either way fall through to the "up to date" branches
+            # below using the current version. This also applies during a
+            # silent startup check, since those branches are what eventually
+            # reach the ArgyllCMS / instrument-setup checks further down.
             resp = curversion_tuple
         if not wx.GetApp():
             return

DisplayCAL/ui/main_window.py

@@ -177,7 +177,9 @@
     check_argyll_bin,
     check_set_argyll_bin,
     get_argyll_instrument_config,
+    get_argyll_latest_version,
     get_argyll_util,
+    get_homebrew_argyll_bin,
     make_argyll_compatible_path,
 )
 from DisplayCAL.argyll_instruments import get_canonical_instrument_name
@@ -282,6 +284,7 @@
     UntetheredController,
     WorkerRunController,
 )
+from DisplayCAL.update_check import resolve_argyll_download_url
 from DisplayCAL.util_decimal import stripzeros
 from DisplayCAL.util_dict import dict_sort
 from DisplayCAL.util_os import get_program_file, launch_file, waccess, which
@@ -502,6 +505,52 @@ def run(self) -> None:  # noqa: D102 (QThread override)
         self.done.emit(result)
 
 
+class _ArgyllDownloadThread(QThread):
+    """Download and extract an ArgyllCMS release archive off the GUI thread.
+
+    Qt port of wx's ``Worker.process_argyll_download`` /
+    ``Worker.extract_archive`` pair (``worker.py``), combined into one
+    thread run behind a single progress dialog instead of wx's two chained
+    ``worker.start()`` calls (download, then extract).
+    """
+
+    #: Emitted with the list of extracted paths, or an ``Exception``.
+    done = Signal(object)
+
+    def __init__(self, worker: Worker, url: str, parent: QWidget | None = None) -> None:
+        super().__init__(parent)
+        self._worker = worker
+        self._url = url
+
+    def run(self) -> None:  # noqa: D102 (QThread override)
+        result = self._worker.download(self._url)
+        if isinstance(result, Exception):
+            self.done.emit(result)
+            return
+        if not result or not (
+            result.lower().endswith(".zip") or result.lower().endswith(".tgz")
+        ):
+            self.done.emit(
+                Exception(f"{lang.getstr('error.file_type_unsupported')}\n{result}")
+            )
+            return
+        try:
+            extracted = self._worker.extract_archive(result)
+        except Exception as exception:  # noqa: BLE001  (reported on GUI thread)
+            self.done.emit(exception)
+            return
+        if (
+            isinstance(extracted, Exception)
+            or not extracted
+            or not os.path.isdir(extracted[0])
+        ):
+            self.done.emit(
+                Exception(lang.getstr("error.no_files_extracted_from_archive", result))
+            )
+            return
+        self.done.emit(extracted)
+
+
 #: Sentinel returned by :meth:`MainWindow._current_cal_choice` when the user
 #: cancels, distinguishable from its other possible results (``None``,
 #: ``False``, or a ``.cal`` path) -- the Qt stand-in for wx's ``wx.ID_CANCEL``.
@@ -1310,6 +1359,10 @@ def __init__(self, worker: Worker | None = None) -> None:
         #: as opposed to :attr:`_install_profile_window`'s standalone flow.
         self._profile_install_thread: _ProfileInstallThread | None = None
         self._profile_install_progress: QProgressDialog | None = None
+        #: Background ArgyllCMS download+extract driven by the missing-Argyll
+        #: startup prompt (:meth:`_prompt_missing_argyll`).
+        self._argyll_download_thread: _ArgyllDownloadThread | None = None
+        self._argyll_download_progress: QProgressDialog | None = None
         #: Services :meth:`Worker.authenticate`'s sudo password prompt for any
         #: elevated (local-system/network) install scope chosen in that dialog.
         self.worker.password_prompt = PasswordPromptAdapter(parent=self)
@@ -1879,6 +1932,93 @@ def _select_install_profile_action_handler(self) -> None:
         self._install_profile_window.raise_()
         self._install_profile_window.activateWindow()
 
+    def _prompt_missing_argyll(self) -> None:
+        """Startup prompt for missing ArgyllCMS binaries.
+
+        Qt port of the ``wx.SingleChoiceDialog`` half of wx's
+        ``argyll.set_argyll_bin()``, shown from
+        ``_run_instrument_setup_and_donation_check`` when
+        ``check_argyll_bin()`` fails. "Download" drives a real in-app
+        download + extract (see :meth:`_download_and_install_argyll`),
+        matching wx; cancelling just dismisses the prompt (Argyll stays
+        unconfigured until the user acts again, matching wx's own cancel
+        behaviour).
+        """
+        box = QMessageBox(self)
+        box.setWindowTitle(APPNAME)
+        box.setIcon(QMessageBox.Warning)
+        box.setText(lang.getstr("dialog.argyll.notfound.choice"))
+        download_button = box.addButton(
+            lang.getstr("download"), QMessageBox.AcceptRole
+        )
+        browse_button = box.addButton(lang.getstr("browse"), QMessageBox.ActionRole)
+        brew_argyll_bin = get_homebrew_argyll_bin()
+        homebrew_button = None
+        if brew_argyll_bin:
+            homebrew_button = box.addButton(
+                lang.getstr("argyll.use_homebrew", brew_argyll_bin),
+                QMessageBox.ActionRole,
+            )
+        box.addButton(lang.getstr("cancel"), QMessageBox.RejectRole)
+        message_box.exec_box(box)
+        clicked = box.clickedButton()
+        if clicked is download_button:
+            self._download_and_install_argyll()
+        elif clicked is browse_button:
+            self._set_argyll_bin_handler()
+        elif homebrew_button is not None and clicked is homebrew_button:
+            setcfg("argyll.dir", brew_argyll_bin)
+            writecfg()
+
+    def _download_and_install_argyll(self) -> None:
+        """Download the latest ArgyllCMS release and configure ``argyll.dir``.
+
+        Qt port of wx's ``app_update_confirm`` ArgyllCMS-download branch
+        plus ``Worker.process_argyll_download``/``set_argyll_bin``: resolves
+        the platform-specific release archive URL, downloads and extracts it
+        on a background thread behind an indeterminate progress dialog (the
+        same pattern as :meth:`_install_profile_direct`), then points
+        ``argyll.dir`` at the extracted ``bin`` folder. Falls back to an
+        error dialog with a "go to website" style message on failure --
+        Argyll stays unconfigured, same as a cancelled/failed wx download.
+        """
+        newversion = get_argyll_latest_version()
+        url = resolve_argyll_download_url(newversion, getcfg("argyll.domain"))
+        self._argyll_download_progress = QProgressDialog(
+            lang.getstr("downloading"), "", 0, 0, self
+        )
+        self._argyll_download_progress.setWindowTitle(APPNAME)
+        self._argyll_download_progress.setCancelButton(None)
+        self._argyll_download_progress.show()
+        self._argyll_download_thread = _ArgyllDownloadThread(
+            self.worker, url, parent=self
+        )
+        self._argyll_download_thread.done.connect(self._on_argyll_download_done)
+        self._argyll_download_thread.start()
+
+    def _on_argyll_download_done(self, result: object) -> None:
+        """Handle the background ArgyllCMS download+extract result.
+
+        Args:
+            result (object): The list of extracted paths, or an
+                ``Exception`` on failure.
+        """
+        self._argyll_download_thread = None
+        if self._argyll_download_progress is not None:
+            self._argyll_download_progress.close()
+            self._argyll_download_progress = None
+        if isinstance(result, Exception):
+            message_box.critical(self, APPNAME, str(result))
+            return
+        setcfg("argyll.dir", os.path.join(result[0], "bin"))
+        writecfg()
+        # Qt port of wx's own post-download behaviour: ``set_argyll_bin_handler``
+        # (``display_cal.py``) calls ``check_update_controls`` once Argyll
+        # becomes available, which re-enumerates displays/instruments rather
+        # than leaving the user to notice and click "Detect display devices
+        # and instruments" themselves.
+        self.detect_displays_and_ports_btn_handler()
+
     def _set_argyll_bin_handler(self) -> None:
         """File menu "Locate ArgyllCMS executables..." handler.
 
@@ -3012,6 +3152,8 @@ def run_post_launch_checks(self) -> None:
 
     def _run_instrument_setup_and_donation_check(self) -> None:
         """Qt port of ``MainFrame.check_instrument_setup``'s dispatch."""
+        if not check_argyll_bin():
+            self._prompt_missing_argyll()
         needs = instrument_setup.resolve_instrument_setup_needs(
             self.worker, self._ccmx_catalog.instruments.values()
         )
@@ -3597,6 +3739,13 @@ def _build_display_instrument_tab(self) -> QWidget:
         display_row = QHBoxLayout()
         display_form = QFormLayout()
         self.display_ctrl = QComboBox()
+        # Qt's default AdjustToContentsOnFirstShow policy measures the combo
+        # once, while it's still empty (displays are only populated later by
+        # detect_displays_and_ports_btn_handler()), and never re-measures --
+        # leaving it stuck too narrow for the real display names. Recompute
+        # on every content change instead, matching wx's Choice/ComboBox,
+        # which auto-sizes to its current items without extra plumbing.
+        self.display_ctrl.setSizeAdjustPolicy(QComboBox.AdjustToContents)
         self.display_ctrl.currentIndexChanged.connect(self.display_ctrl_handler)
         # No row label: the group box is already titled "Display" right
         # above this combo, so a per-row "Display" label would just repeat it.
@@ -3654,6 +3803,9 @@ def _build_display_instrument_tab(self) -> QWidget:
         instrument_outer = QVBoxLayout(instrument_box)
         instrument_form = QFormLayout()
         self.comport_ctrl = QComboBox()
+        # Same AdjustToContents fix as display_ctrl above -- instruments are
+        # also only populated after the initial (empty) first show.
+        self.comport_ctrl.setSizeAdjustPolicy(QComboBox.AdjustToContents)
         self.comport_ctrl.currentIndexChanged.connect(self.comport_ctrl_handler)
         self.measurement_mode_ctrl = QComboBox()
         self.measurement_mode_ctrl.currentIndexChanged.connect(

DisplayCAL/update_check.py

@@ -26,9 +26,16 @@
   already);
 * the in-app auto-download-and-run-the-installer flow
   (``app_update_confirm``'s ``worker.start(consumer, worker.download, ...)``
-  branch) — the Qt dialog offers a direct asset download URL (opened in the
-  browser) or a "go to website" fallback instead of driving Argyll/DisplayCAL
-  installers itself, which is a large, separate feature in its own right.
+  branch) for the *update-available* dialogs — the Qt
+  ``_UpdateAvailableDialog`` offers a direct asset download URL (opened in
+  the browser) or a "go to website" fallback instead of driving
+  Argyll/DisplayCAL installers itself. The *missing*-ArgyllCMS startup
+  prompt (``MainWindow._prompt_missing_argyll``) is a separate flow and
+  does drive a real in-app download + extract, via
+  :func:`resolve_argyll_download_url` below plus a small Qt-only
+  ``_ArgyllDownloadThread`` (``main_window.py``) — installing ArgyllCMS in
+  the first place is table-stakes for a working app, unlike an optional
+  version bump.
 """
 
 from __future__ import annotations
@@ -120,6 +127,41 @@ def resolve_app_download_url(release_data: dict, newversion: str) -> str | None:
     return None
 
 
+def resolve_argyll_download_url(newversion: str, domain: str) -> str:
+    """Return the ArgyllCMS release archive URL for the current platform.
+
+    Toolkit-neutral port of the ArgyllCMS branch of wx's
+    ``app_update_confirm`` (``display_cal.py``): same
+    ``argyll.domain``-relative GitHub Releases layout and per-platform
+    suffix table, simplified to ``platform.machine()`` detection (no
+    Windows registry lookup) like :func:`resolve_app_download_url`.
+    Confirmed against the real ``eoyilmaz/argyllcms-binaries`` release
+    assets, which are named exactly ``Argyll_V{version}{suffix}``.
+
+    Args:
+        newversion: The ArgyllCMS version string (e.g. ``"3.5.0"``).
+        domain: The ``argyll.domain`` config value (a GitHub repo URL).
+    """
+    machine = platform.machine().lower()
+    if sys.platform == "win32":
+        if machine in ("arm64", "aarch64"):
+            suffix = "_win_arm64_exe.zip"
+        elif machine in ("amd64", "x86_64"):
+            suffix = "_win64_exe.zip"
+        else:
+            suffix = "_win32_exe.zip"
+    elif sys.platform == "darwin":
+        if machine in ("arm64", "aarch64"):
+            suffix = "_macOS11_arm64_bin.tgz"
+        else:
+            suffix = "_osx10.6_x86_64_bin.tgz"
+    elif machine in ("x86_64", "amd64") or platform.architecture()[0] == "64bit":
+        suffix = "_linux_x86_64_bin.tgz"
+    else:
+        suffix = "_linux_x86_bin.tgz"
+    return f"{domain}/releases/download/{newversion}/Argyll_V{newversion}{suffix}"
+
+
 def _format_changelog(html: str, domain: str) -> str:
     """Rewrite anchor-only ``href``s and demote heading tags, matching wx."""
     html = re.sub(