From 7c41c8f8e182f0a3a09a109972a666c03d9592eb Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 24 Jul 2026 11:25:31 -0500 Subject: [PATCH 1/4] Refactor Python allow-list to use the constraints files instead --- .../app/test_python_constraints.py | 125 ++++++++++++++ AddonManagerTest/app/test_utilities.py | 64 +++++++ addonmanager_python_constraints.py | 156 ++++++++++++++++++ addonmanager_python_deps.py | 10 +- addonmanager_utilities.py | 42 +++-- 5 files changed, 372 insertions(+), 25 deletions(-) create mode 100644 AddonManagerTest/app/test_python_constraints.py create mode 100644 addonmanager_python_constraints.py diff --git a/AddonManagerTest/app/test_python_constraints.py b/AddonManagerTest/app/test_python_constraints.py new file mode 100644 index 0000000..1e9ca73 --- /dev/null +++ b/AddonManagerTest/app/test_python_constraints.py @@ -0,0 +1,125 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# SPDX-FileCopyrightText: 2026 The FreeCAD project association AISBL +# SPDX-FileNotice: Part of the AddonManager. + +################################################################################ +# # +# This addon is free software: you can redistribute it and/or modify # +# it under the terms of the GNU Lesser General Public License as # +# published by the Free Software Foundation, either version 2.1 # +# of the License, or (at your option) any later version. # +# # +# This addon is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty # +# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. # +# See the GNU Lesser General Public License for more details. # +# # +# You should have received a copy of the GNU Lesser General Public # +# License along with this addon. If not, see https://www.gnu.org/licenses # +# # +################################################################################ + +import os +import tempfile +import unittest +from unittest.mock import patch + +from addonmanager_python_constraints import PythonConstraints + +SAMPLE_CONSTRAINTS = """# Header comment, may contain non-ascii like the real file +# SPDX-License-Identifier: CC0-1.0 + +kicad-python==0.7.1 +numpy==2.4.6 +Some_Package==1.2.3 +not-a-pin +""" + + +class TestPythonConstraints(unittest.TestCase): + + def setUp(self): + self.temp_dir = tempfile.TemporaryDirectory() + self.cache_path = os.path.join(self.temp_dir.name, "constraints-cache.txt") + + def tearDown(self): + self.temp_dir.cleanup() + + def _patched(self, location, fetched_bytes): + """Build a PythonConstraints with the location, remote fetch, and cache file + redirected for testing.""" + constraints = PythonConstraints() + patches = [ + patch( + "addonmanager_python_constraints.resolve_constraints_location", + return_value=location, + ), + patch( + "addonmanager_python_constraints.blocking_get", + return_value=fetched_bytes, + ), + patch.object(PythonConstraints, "_cache_file", return_value=self.cache_path), + ] + for active_patch in patches: + active_patch.start() + self.addCleanup(active_patch.stop) + return constraints + + def test_parse_ignores_comments_blanks_and_unpinned(self): + versions = PythonConstraints._parse(SAMPLE_CONSTRAINTS) + self.assertEqual(versions["kicad-python"], "0.7.1") + self.assertEqual(versions["numpy"], "2.4.6") + self.assertNotIn("not-a-pin", versions) + self.assertEqual(len(versions), 3) + + def test_parse_normalizes_names(self): + versions = PythonConstraints._parse(SAMPLE_CONSTRAINTS) + self.assertIn("some-package", versions) + self.assertNotIn("Some_Package", versions) + + def test_fetch_from_https_populates_and_caches(self): + constraints = self._patched( + "https://example.test/3.13/constraints.txt", SAMPLE_CONSTRAINTS.encode("utf-8") + ) + self.assertEqual(constraints.version_for("kicad-python"), "0.7.1") + self.assertIn("numpy", constraints.allowed_packages()) + self.assertTrue(os.path.exists(self.cache_path)) + + def test_falls_back_to_cache_when_fetch_is_empty(self): + with open(self.cache_path, "w", encoding="utf-8") as cache_file: + cache_file.write("cached-package==9.9.9\n") + constraints = self._patched("https://example.test/3.13/constraints.txt", b"") + self.assertEqual(constraints.version_for("cached-package"), "9.9.9") + + def test_version_for_normalizes_query(self): + constraints = self._patched( + "https://example.test/3.13/constraints.txt", SAMPLE_CONSTRAINTS.encode("utf-8") + ) + self.assertEqual(constraints.version_for("kicad_python"), "0.7.1") + + def test_disabled_constraints_yield_empty_allow_list(self): + constraints = self._patched(None, b"") + self.assertEqual(constraints.allowed_packages(), set()) + self.assertIsNone(constraints.version_for("numpy")) + + def test_local_path_is_read_from_disk(self): + local_file = os.path.join(self.temp_dir.name, "constraints.txt") + with open(local_file, "w", encoding="utf-8") as handle: + handle.write("local-package==4.5.6\n") + constraints = self._patched(local_file, b"") + self.assertEqual(constraints.version_for("local-package"), "4.5.6") + + def test_is_enabled_reflects_location(self): + with patch( + "addonmanager_python_constraints.resolve_constraints_location", return_value=None + ): + self.assertFalse(PythonConstraints.is_enabled()) + with patch( + "addonmanager_python_constraints.resolve_constraints_location", + return_value="https://example.test/3.13/constraints.txt", + ): + self.assertTrue(PythonConstraints.is_enabled()) + + +if __name__ == "__main__": + unittest.main() diff --git a/AddonManagerTest/app/test_utilities.py b/AddonManagerTest/app/test_utilities.py index c10086f..fbb1c1d 100644 --- a/AddonManagerTest/app/test_utilities.py +++ b/AddonManagerTest/app/test_utilities.py @@ -25,6 +25,7 @@ from unittest.mock import MagicMock, patch, mock_open import os import subprocess +import sys from AddonManagerTest.app.mocks import MockAddon as Addon @@ -35,6 +36,7 @@ GITLAB, IDENTIFIED_HOSTS_PREFERENCE, construct_git_url, + create_pip_call, forget_git_host, forget_git_hosts, get_assigned_string_literal, @@ -44,10 +46,12 @@ get_zip_url, git_host_of, identify_git_host, + pep503_normalize, process_date_string_to_python_datetime, recognized_git_location, reload_git_hosts, remember_git_host, + resolve_constraints_location, run_interruptable_subprocess, ) @@ -314,6 +318,66 @@ def test_process_date_string_to_python_datetime_invalid_separators(self): process_date_string_to_python_datetime(f"2024{separator}01{separator}31") +class TestPep503Normalize(unittest.TestCase): + """Tests for PEP 503 package-name normalization.""" + + def test_normalizes_case_underscores_and_dots(self): + self.assertEqual(pep503_normalize("KiCad_Python"), "kicad-python") + self.assertEqual(pep503_normalize("Some.Package"), "some-package") + self.assertEqual(pep503_normalize("already-normalized"), "already-normalized") + + +class TestConstraintsLocation(unittest.TestCase): + """Tests for resolving and applying the pip constraints file location.""" + + def _relative(self) -> str: + return f"{sys.version_info.major}.{sys.version_info.minor}/constraints.txt" + + def test_disabled_when_preference_is_empty(self): + Preferences().set("pip_constraints_path", "") + self.assertIsNone(resolve_constraints_location()) + + def test_https_base_appends_versioned_relative_path(self): + Preferences().set("pip_constraints_path", "https://example.test/Data/Python/") + self.assertEqual( + resolve_constraints_location(), + "https://example.test/Data/Python/" + self._relative(), + ) + + def test_https_base_without_trailing_slash(self): + Preferences().set("pip_constraints_path", "https://example.test/Data/Python") + self.assertEqual( + resolve_constraints_location(), + "https://example.test/Data/Python/" + self._relative(), + ) + + def test_local_base_uses_os_path(self): + Preferences().set("pip_constraints_path", os.path.join("local", "constraints")) + expected = os.path.join("local", "constraints", self._relative().replace("/", os.path.sep)) + self.assertEqual(resolve_constraints_location(), expected) + + @patch("addonmanager_utilities.resolve_constraints_location") + @patch("addonmanager_utilities.fci.get_python_exe") + def test_create_pip_call_adds_constraint_on_install( + self, mock_python_exe: MagicMock, mock_resolve: MagicMock + ): + mock_python_exe.return_value = "python3" + mock_resolve.return_value = "https://example.test/3.13/constraints.txt" + call = create_pip_call(["install", "somepackage"]) + self.assertIn("--constraint", call) + self.assertIn("https://example.test/3.13/constraints.txt", call) + + @patch("addonmanager_utilities.resolve_constraints_location") + @patch("addonmanager_utilities.fci.get_python_exe") + def test_create_pip_call_omits_constraint_when_disabled( + self, mock_python_exe: MagicMock, mock_resolve: MagicMock + ): + mock_python_exe.return_value = "python3" + mock_resolve.return_value = None + call = create_pip_call(["install", "somepackage"]) + self.assertNotIn("--constraint", call) + + class TestGitHostDetection(unittest.TestCase): """Tests for identifying the software that an unrecognized git host is running.""" diff --git a/addonmanager_python_constraints.py b/addonmanager_python_constraints.py new file mode 100644 index 0000000..80417da --- /dev/null +++ b/addonmanager_python_constraints.py @@ -0,0 +1,156 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# SPDX-FileCopyrightText: 2026 The FreeCAD project association AISBL +# SPDX-FileNotice: Part of the AddonManager. + +################################################################################ +# # +# This addon is free software: you can redistribute it and/or modify # +# it under the terms of the GNU Lesser General Public License as # +# published by the Free Software Foundation, either version 2.1 # +# of the License, or (at your option) any later version. # +# # +# This addon is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty # +# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. # +# See the GNU Lesser General Public License for more details. # +# # +# You should have received a copy of the GNU Lesser General Public # +# License along with this addon. If not, see https://www.gnu.org/licenses # +# # +################################################################################ + +"""Single source of truth for FreeCAD's vetted Python package versions. + +We publish one constraints file per Python minor version that pins every allowed +package to an exact version, including transitive dependencies. This module fetches, caches, +and parses that file so the rest of the Addon Manager can decide which packages may be +installed and which installed packages differ from the vetted version. No other source of +package policy is consulted. The standard source for these files is +https://github.com/FreeCAD/Addons/Data/Python/{version}/constraints.txt +""" + +import sys +from typing import Dict, Optional, Set +from urllib.parse import urlparse + +import addonmanager_freecad_interface as fci +from addonmanager_utilities import ( + blocking_get, + get_cache_file_name, + pep503_normalize, + resolve_constraints_location, +) + + +class PythonConstraints: + """The set of vetted Python package versions for the running Python interpreter.""" + + _CACHE_FILE_TEMPLATE = "constraints-py{major}{minor}.txt" + + def __init__(self) -> None: + self._versions: Dict[str, str] = {} + self._loaded = False + + @staticmethod + def is_enabled() -> bool: + """Return True if constraints are configured, or False if the user disabled them by + clearing the 'pip_constraints_path' preference.""" + return resolve_constraints_location() is not None + + def constrained_versions(self) -> Dict[str, str]: + """Return a mapping of normalized package name to its vetted version.""" + self._ensure_loaded() + return dict(self._versions) + + def allowed_packages(self) -> Set[str]: + """Return the set of normalized package names that may be installed.""" + self._ensure_loaded() + return set(self._versions.keys()) + + def version_for(self, package_name: str) -> Optional[str]: + """Return the vetted version for a package, or None if it is not constrained.""" + self._ensure_loaded() + return self._versions.get(pep503_normalize(package_name)) + + def reload(self) -> None: + """Fetch and parse the constraints file, falling back to the on-disk cache.""" + self._versions = self._parse(self._fetch_or_read_cache()) + self._loaded = True + + def _ensure_loaded(self) -> None: + if not self._loaded: + self.reload() + + def _fetch_or_read_cache(self) -> str: + """Return the raw constraints text, preferring a fresh fetch and caching it, and + falling back to the previously cached copy when the fetch yields nothing.""" + location = resolve_constraints_location() + if location is None: + return "" + text = self._fetch(location) + if text: + self._write_cache(text) + return text + return self._read_cache() + + @staticmethod + def _fetch(location: str) -> str: + """Retrieve the raw constraints text from a remote https URL or a local path.""" + if urlparse(location).scheme == "https": + data = blocking_get(location) + return data.decode("utf-8") if data else "" + try: + with open(location, encoding="utf-8") as constraints_file: + return constraints_file.read() + except OSError: + return "" + + @classmethod + def _cache_file(cls) -> str: + """Return the full path to this Python version's cached constraints file.""" + name = cls._CACHE_FILE_TEMPLATE.format( + major=sys.version_info.major, minor=sys.version_info.minor + ) + return get_cache_file_name(name) + + def _write_cache(self, text: str) -> None: + try: + with open(self._cache_file(), "w", encoding="utf-8") as cache_file: + cache_file.write(text) + except OSError as error: + fci.Console.PrintLog(f"Could not cache constraints file: {error}\n") + + def _read_cache(self) -> str: + try: + with open(self._cache_file(), encoding="utf-8") as cache_file: + return cache_file.read() + except OSError: + fci.Console.PrintLog("No cached constraints file available\n") + return "" + + @staticmethod + def _parse(text: str) -> Dict[str, str]: + """Parse 'name==version' lines into a mapping of normalized name to version, ignoring + comments, blank lines, and any line without an exact-version pin.""" + versions: Dict[str, str] = {} + for line in text.splitlines(): + stripped = line.strip() + if not stripped or stripped.startswith("#") or "==" not in stripped: + continue + name, _, version = stripped.partition("==") + name = name.strip() + version = version.strip() + if name and version: + versions[pep503_normalize(name)] = version + return versions + + +_shared_constraints: Optional[PythonConstraints] = None + + +def get_constraints() -> PythonConstraints: + """Return the process-wide shared constraints, loading them on first use.""" + global _shared_constraints + if _shared_constraints is None: + _shared_constraints = PythonConstraints() + return _shared_constraints diff --git a/addonmanager_python_deps.py b/addonmanager_python_deps.py index e045451..92328b3 100644 --- a/addonmanager_python_deps.py +++ b/addonmanager_python_deps.py @@ -35,6 +35,7 @@ create_pip_call, run_interruptable_subprocess, get_pip_target_directory, + pep503_normalize, translate, using_system_pip_installation_location, ) @@ -91,15 +92,6 @@ class PackageInfo: dependencies: List[str] -def pep503_normalize(package_name: str) -> str: - """Given a Python package name, normalize it per PEP 503, making it all lowercase, and replacing - underscores and dots with dashes.""" - - result = package_name.replace("_", "-") - result = result.replace(".", "-") - return result.lower() - - def parse_pip_list_output(all_packages, outdated_packages) -> List[PackageInfo]: """Parses the output from pip into a dictionary with update information in it. The pip output should be an array of lines of text.""" diff --git a/addonmanager_utilities.py b/addonmanager_utilities.py index bdd0b1f..2d91cba 100644 --- a/addonmanager_utilities.py +++ b/addonmanager_utilities.py @@ -588,6 +588,28 @@ def get_cache_file_name(file: str) -> str: return os.path.join(am_path, file) +def pep503_normalize(package_name: str) -> str: + """Normalize a Python package name per PEP 503, lowercasing it and replacing underscores + and dots with dashes.""" + result = package_name.replace("_", "-") + result = result.replace(".", "-") + return result.lower() + + +def resolve_constraints_location() -> Optional[str]: + """Return the pip constraints file location for the running Python version, or None if the + user has disabled constraints by clearing the 'pip_constraints_path' preference.""" + base = fci.Preferences().get("pip_constraints_path") + if not base: + return None + relative_path = f"{sys.version_info.major}.{sys.version_info.minor}/constraints.txt" + if urlparse(base).scheme == "https": + if not base.endswith("/"): + base += "/" + return base + relative_path + return os.path.join(base, relative_path.replace("/", os.path.sep)) + + def blocking_get(url: str, method=None) -> bytes: """Wrapper around three possible ways of accessing data, depending on the current run mode and Python installation. Blocks until complete, and returns the text results of the call if it @@ -800,25 +822,13 @@ def create_pip_call(args: List[str]) -> List[str]: call_args.extend(["--proxy", f"http://{host}:{port}"]) if "install" in args: - constraints = fci.Preferences().get("pip_constraints_path") - if not constraints: + constraints = resolve_constraints_location() + if constraints: + args.extend(["--constraint", constraints]) + else: fci.Console.PrintWarning( "pip constraints explicitly disabled by unsetting 'pip_constraints_path'\n" ) - else: - parsed_url = urlparse(constraints) - major = sys.version_info.major - minor = sys.version_info.minor - expected_rel_path = f"{major}.{minor}/constraints.txt" - if parsed_url.scheme == "https": - # The only supported remote scheme is https, and this is the default setup - if not constraints.endswith("/"): - constraints += "/" - constraints += expected_rel_path - else: - # If it wasn't https, treat it like it's a local path - constraints = os.path.join(constraints, expected_rel_path.replace("/", os.path.sep)) - args.extend(["--constraint", constraints]) call_args.extend(args) return call_args From 43eb29b741545d1f412dd9f7e625fbb2ffddeeff Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 24 Jul 2026 12:43:13 -0500 Subject: [PATCH 2/4] Migrate the installer to constraints from ALLOWED --- AddonManagerTest/gui/test_installer_gui.py | 57 ++++++++++++++++++++-- CMakeLists.txt | 1 + addonmanager_installer.py | 41 ++-------------- addonmanager_installer_gui.py | 15 ++++-- 4 files changed, 70 insertions(+), 44 deletions(-) diff --git a/AddonManagerTest/gui/test_installer_gui.py b/AddonManagerTest/gui/test_installer_gui.py index df2b877..3e57cf9 100644 --- a/AddonManagerTest/gui/test_installer_gui.py +++ b/AddonManagerTest/gui/test_installer_gui.py @@ -30,6 +30,7 @@ from PySideWrapper import QtWidgets, QtCore from Addon import Addon, MissingDependencies +from addonmanager_installer import AddonInstaller from addonmanager_installer_gui import ( AddonInstallerGUI, AddonDependencyInstallerGUI, @@ -43,10 +44,23 @@ translate = fci.translate +def patch_constraints(test_case, allowed=("in-the-allowlist",)): + """Give a test a deterministic, network-free constraints source and reset the installer's + cached allow-list so each test starts from a known state.""" + AddonInstaller.allowed_packages = set() + fake_constraints = MagicMock() + fake_constraints.allowed_packages.return_value = set(allowed) + constraints_patch = patch( + "addonmanager_installer.get_constraints", return_value=fake_constraints + ) + constraints_patch.start() + test_case.addCleanup(constraints_patch.stop) + + class TestAddonInstallerGUI(unittest.TestCase): def setUp(self): - pass + patch_constraints(self) def tearDown(self): pass @@ -170,7 +184,12 @@ def test_installer_is_called_if_dependencies_are_ok( class TestAddonDependencyInstallerGUI(unittest.TestCase): def setUp(self): - pass + patch_constraints(self) + constraints_enabled = patch( + "addonmanager_installer_gui.PythonConstraints.is_enabled", return_value=True + ) + constraints_enabled.start() + self.addCleanup(constraints_enabled.stop) def tearDown(self): pass @@ -410,6 +429,38 @@ def test_an_empty_addon_list_is_held_to_the_allow_list(self): self.assertEqual("AddonManager_RequirementFailedDialog", dialog_name) self.assertNotIn("not_in_the_allowlist", deps.python_requires) + def test_underscore_dependency_matches_dashed_allow_entry(self): + """A dependency declared with underscores matches a normalized (dashed) allow-list + entry, so it is not wrongly refused.""" + deps = self.create_mock_deps(python_requires=["kicad_python"]) + gui = AddonDependencyInstallerGUI([self._addon(from_custom_repository=False)], deps) + gui.installer = self.MockAddonInstaller([]) + gui.installer.allowed_packages = ["kicad-python"] + + with patch("addonmanager_installer_gui.MessageDialog.show_modal") as mock_dialog: + stop_installation = gui._handle_disallowed_python() + + self.assertFalse(stop_installation) + mock_dialog.assert_not_called() + self.assertIn("kicad_python", deps.python_requires) + + def test_disabled_constraints_do_not_gate_required_packages(self): + """With constraints disabled there is no authoritative list, so a package that is not on + the allow-list is allowed through rather than being stripped.""" + deps = self.create_mock_deps(python_requires=["not_in_the_allowlist"]) + gui = AddonDependencyInstallerGUI([self._addon(from_custom_repository=False)], deps) + gui.installer = self.MockAddonInstaller([]) + + with ( + patch("addonmanager_installer_gui.PythonConstraints.is_enabled", return_value=False), + patch("addonmanager_installer_gui.MessageDialog.show_modal") as mock_dialog, + ): + stop_installation = gui._handle_disallowed_python() + + self.assertFalse(stop_installation) + mock_dialog.assert_not_called() + self.assertIn("not_in_the_allowlist", deps.python_requires) + def test_custom_repo_unreviewed_optional_package_is_offered(self): """An optional package is offered in the dependency dialog for the user to accept or refuse, so a custom repository's optional packages are not dropped either.""" @@ -557,7 +608,7 @@ class MockAddonInstaller(QtCore.QObject): def __init__(self, addons: List[Addon]): super().__init__() self.addons = addons - self.allowed_packages = ["in_the_allowlist"] + self.allowed_packages = ["in-the-allowlist"] @patch("addonmanager_installer_gui.utils.blocking_get", MagicMock(return_value=None)) @patch("addonmanager_installer_gui.AddonInstaller") diff --git a/CMakeLists.txt b/CMakeLists.txt index c78309d..5c5de63 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -54,6 +54,7 @@ SET(AddonManager_SRCS addonmanager_package_details_controller.py addonmanager_preferences_defaults.json addonmanager_preferences_migrations.py + addonmanager_python_constraints.py addonmanager_python_deps_commands.py addonmanager_python_deps_gui.py addonmanager_python_deps.py diff --git a/addonmanager_installer.py b/addonmanager_installer.py index 54a4c95..9cc79a8 100644 --- a/addonmanager_installer.py +++ b/addonmanager_installer.py @@ -39,6 +39,7 @@ from Addon import Addon import addonmanager_utilities as utils +from addonmanager_python_constraints import get_constraints from addonmanager_installation_manifest import InstallationManifest from addonmanager_metadata import get_branch_from_metadata from addonmanager_git import initialize_git, GitFailed @@ -145,10 +146,9 @@ def __init__(self, addon: Addon, allow_list: List[str] = None): self.git_manager = None if allow_list is not None: - AddonInstaller.allowed_packages = set(allow_list if allow_list is not None else []) + AddonInstaller.allowed_packages = set(allow_list) elif not AddonInstaller.allowed_packages: - AddonInstaller._load_local_allowed_packages_list() - AddonInstaller._update_allowed_packages_list() + AddonInstaller.allowed_packages = get_constraints().allowed_packages() self.installation_path = fci.DataPaths().mod_dir self.macro_installation_path = fci.DataPaths().macro_dir @@ -194,41 +194,6 @@ def run(self, install_method: InstallationMethod = InstallationMethod.ANY) -> bo self.finished.emit() return success - @classmethod - def _load_local_allowed_packages_list(cls) -> None: - """Read in the local allowlist, in case the remote one is unavailable.""" - cls.allowed_packages.clear() - allow_file = os.path.join(os.path.dirname(__file__), "ALLOWED_PYTHON_PACKAGES.txt") - if os.path.exists(allow_file): - with open(allow_file, encoding="utf8") as f: - lines = f.readlines() - for line in lines: - if line and len(line) > 0 and line[0] != "#": - cls.allowed_packages.add(line.strip().lower()) - - @classmethod - def _update_allowed_packages_list(cls) -> None: - """Get a new remote copy of the allowed packages list from GitHub.""" - fci.Console.PrintLog("Attempting to fetch remote copy of ALLOWED_PYTHON_PACKAGES.txt...\n") - p = utils.blocking_get( - "https://raw.githubusercontent.com/" - "FreeCAD/FreeCAD-addons/master/ALLOWED_PYTHON_PACKAGES.txt" - ) - if p: - fci.Console.PrintLog( - "Overriding local ALLOWED_PYTHON_PACKAGES.txt with newer remote version\n" - ) - p = p.decode("utf8") - lines = p.split("\n") - cls.allowed_packages.clear() # Unset the locally defined list - for line in lines: - if line and len(line) > 0 and line[0] != "#": - cls.allowed_packages.add(line.strip().lower()) - else: - fci.Console.PrintLog( - "Could not fetch remote ALLOWED_PYTHON_PACKAGES.txt, using local copy\n" - ) - def _determine_install_method( self, addon_url: str, install_method: InstallationMethod ) -> Optional[InstallationMethod]: diff --git a/addonmanager_installer_gui.py b/addonmanager_installer_gui.py index c2085c7..3ca449f 100644 --- a/addonmanager_installer_gui.py +++ b/addonmanager_installer_gui.py @@ -36,6 +36,7 @@ from addonmanager_installer import AddonInstaller, MacroInstaller from addonmanager_dependency_installer import DependencyInstaller from addonmanager_metadata import Version +from addonmanager_python_constraints import PythonConstraints import addonmanager_utilities as utils from Addon import Addon, MissingDependencies @@ -507,11 +508,15 @@ def run(self): def _handle_disallowed_python(self) -> bool: """Determine if we are missing any required Python packages that are not in the allowed packages list. If so, display a message to the user and return True if they want to cancel. - Otherwise, return False.""" + Otherwise, return False. When constraints are disabled there is no authoritative list to + check against, so nothing is disallowed.""" + + if not PythonConstraints.is_enabled(): + return False bad_packages = [] for dep in self.deps.python_requires: - if dep.lower() not in self.installer.allowed_packages: + if utils.pep503_normalize(dep) not in self.installer.allowed_packages: bad_packages.append(dep) if bad_packages and self._all_from_custom_repositories(): @@ -714,6 +719,10 @@ def _check_python_version(self) -> bool: return False def _clean_up_optional(self): + if not PythonConstraints.is_enabled(): + # With constraints disabled there is no authoritative list, so every optional package + # is offered to the user rather than being filtered out here. + return if self._all_from_custom_repositories(): # The allow-list does not apply to a repository the user chose to trust. Each optional # package is offered in the dependency dialog for the user to accept or refuse, so @@ -721,7 +730,7 @@ def _clean_up_optional(self): return good_packages = [] for dep in self.deps.python_optional: - if dep in self.installer.allowed_packages: + if utils.pep503_normalize(dep) in self.installer.allowed_packages: good_packages.append(dep) else: fci.Console.PrintWarning( From 71d806c648a9a4f59d9acc96f489a54a4b3adc80 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 24 Jul 2026 12:53:10 -0500 Subject: [PATCH 3/4] Remove direct PyPI access and use constraints files exclusively --- AddonManagerTest/app/test_python_deps.py | 131 +++++++++-------------- addonmanager_python_deps.py | 103 +++++++----------- 2 files changed, 85 insertions(+), 149 deletions(-) diff --git a/AddonManagerTest/app/test_python_deps.py b/AddonManagerTest/app/test_python_deps.py index fe2b177..391ad4f 100644 --- a/AddonManagerTest/app/test_python_deps.py +++ b/AddonManagerTest/app/test_python_deps.py @@ -30,7 +30,6 @@ PythonPackageListModel, parse_pip_list_output, call_pip, - pip_has_dry_run_support, PipFailed, ) @@ -70,13 +69,13 @@ def test_call_pip_splits_results(self, mock_run_subprocess: MagicMock): self.assertEqual(len(result), 3) def test_parse_pip_list_output_no_input(self): - results_dict = parse_pip_list_output("", "") + results_dict = parse_pip_list_output("", {}) self.assertEqual(len(results_dict), 0) def test_parse_pip_list_output_all_packages_no_updates(self): results_list = parse_pip_list_output( ["Package Version", "---------- -------", "gitdb 4.0.9", "setuptools 41.2.0"], - [], + {}, ) self.assertEqual(len(results_list), 2) self.assertEqual("gitdb", results_list[0].name) @@ -86,47 +85,32 @@ def test_parse_pip_list_output_all_packages_no_updates(self): self.assertEqual("41.2.0", results_list[1].installed_version) self.assertEqual("", results_list[1].available_version) - def test_parse_pip_list_output_all_packages_with_updates(self): + def test_parse_pip_list_output_update_available_when_constrained_version_differs(self): + """An update is available when the constrained version differs from what is installed; + a package without a constraint, or already at its constrained version, shows no update.""" results_list = parse_pip_list_output( [ - "Package Version Type", - "---------- ------- -----", - "pip 21.0.1 wheel", - "setuptools 41.2.0 wheel", - ], - [ - "Package Version Latest Type", - "---------- ------- ------ -----", - "pip 21.0.1 22.1.2 wheel", + "Package Version", + "---------- -------", + "pip 21.0.1", + "numpy 2.4.6", + "setuptools 41.2.0", ], + {"pip": "22.1.2", "numpy": "2.4.6"}, ) - self.assertEqual(len(results_list), 2) - self.assertEqual("pip", results_list[0].name) - self.assertEqual("21.0.1", results_list[0].installed_version) - self.assertEqual("22.1.2", results_list[0].available_version) - self.assertEqual("setuptools", results_list[1].name) - self.assertEqual("41.2.0", results_list[1].installed_version) - - @patch("addonmanager_python_deps.run_interruptable_subprocess") - def test_pip_has_dry_run_support_true(self, mock_run_subprocess: MagicMock): - result_mock = MagicMock() - result_mock.stdout = ( - "pip 25.0 from /opt/homebrew/lib/python3.13/site-packages/pip (python 3.13)" + by_name = {package.name: package for package in results_list} + self.assertEqual("22.1.2", by_name["pip"].available_version) + self.assertEqual("", by_name["numpy"].available_version) + self.assertEqual("", by_name["setuptools"].available_version) + + def test_parse_pip_list_output_normalizes_names_for_constraint_lookup(self): + """A constraint keyed by the normalized name still matches an installed package whose + reported name uses different separators or casing.""" + results_list = parse_pip_list_output( + ["Package Version", "----------- -------", "KiCad_Python 0.6.0"], + {"kicad-python": "0.7.1"}, ) - result_mock.returncode = 0 - mock_run_subprocess.return_value = result_mock - result = pip_has_dry_run_support() - self.assertTrue(result) - - @patch("addonmanager_python_deps.run_interruptable_subprocess") - def test_pip_has_dry_run_support_false(self, mock_run_subprocess: MagicMock): - result_mock = MagicMock() - # Dry run support was added in 23.1 - result_mock.stdout = "pip 23.0 from /usr/bin/python3.11/site-packages/pip (python 3.11)" - result_mock.returncode = 0 - mock_run_subprocess.return_value = result_mock - result = pip_has_dry_run_support() - self.assertFalse(result) + self.assertEqual("0.7.1", results_list[0].available_version) class TestPythonPackageListModel(unittest.TestCase): @@ -136,24 +120,29 @@ def test_instantiation(self): self.assertIsNotNone(model) def test_reset_package_list_resets_model(self): - fake_outdated = "Package Version Latest Type\n---------- ------- ------ -----\nnumpy 1.24.0 1.25.2 wheel" fake_all = "Package Version\n---------- -------\nnumpy 1.24.0\npandas 2.1.0" def fake_call_pip(args): - if "-o" in args: - return fake_outdated.splitlines() - elif "list" in args: + if "list" in args: return fake_all.splitlines() raise ValueError(f"Unexpected pip args: {args}") - with patch("addonmanager_python_deps.call_pip", side_effect=fake_call_pip): + fake_constraints = MagicMock() + fake_constraints.constrained_versions.return_value = {"numpy": "1.25.2"} + + with ( + patch("addonmanager_python_deps.call_pip", side_effect=fake_call_pip), + patch("addonmanager_python_deps.get_constraints", return_value=fake_constraints), + ): model = PythonPackageListModel([]) catcher = SignalCatcher() model.modelReset.connect(catcher.catch_signal) model.reset_package_list() self.assertTrue(catcher.caught) self.assertEqual("numpy", model.package_list[0].name) + self.assertEqual("1.25.2", model.package_list[0].available_version) self.assertEqual("pandas", model.package_list[1].name) + self.assertEqual("", model.package_list[1].available_version) class MinimalAddon: def __init__(self, name, python_requires=None, python_optional=None): @@ -161,10 +150,9 @@ def __init__(self, name, python_requires=None, python_optional=None): self.python_requires = python_requires if python_requires else [] self.python_optional = python_optional if python_optional else [] - @patch("addonmanager_python_deps.pip_has_dry_run_support", return_value=False) - def test_determine_new_python_dependencies_without_dry_run_no_existing(self, _): - """With no dry-run support, the returned set is just the union of the two lists (if no - packages are installed).""" + def test_determine_new_python_dependencies_no_existing(self): + """With nothing installed, the returned set is the union of every addon's required and + optional dependencies.""" addon_1 = self.MinimalAddon("addon_1", ["py_req_1", "py_req_2"], ["py_opt_1", "py_opt_2"]) addon_2 = self.MinimalAddon("addon_2", ["py_req_3", "py_req_4"], ["py_opt_2", "py_opt_3"]) @@ -176,19 +164,17 @@ def test_determine_new_python_dependencies_without_dry_run_no_existing(self, _): python_deps, ) - @patch("addonmanager_python_deps.pip_has_dry_run_support", return_value=False) - def test_determine_new_python_dependencies_without_dry_run_with_existing(self, _): - """With no dry-run support, the returned set is just the union of the two lists, minus the - packages that are already installed.""" + def test_determine_new_python_dependencies_with_existing(self): + """Dependencies that are already installed are excluded from the returned set.""" addon_1 = self.MinimalAddon("addon_1", ["py_req_1", "py_req_2"], ["py_opt_1", "py_opt_2"]) addon_2 = self.MinimalAddon("addon_2", ["py_req_3", "py_req_4"], ["py_opt_2", "py_opt_3"]) addons = [addon_1, addon_2] model = PythonPackageListModel([]) model.package_list = [ - PackageInfo("py_req_1", "1", "", []), - PackageInfo("py_req_2", "1", "", []), - PackageInfo("py_opt_1", "1", "", []), + PackageInfo("py-req-1", "1", "", []), + PackageInfo("py-req-2", "1", "", []), + PackageInfo("py-opt-1", "1", "", []), ] python_deps = model.determine_new_python_dependencies(addons) self.assertEqual( @@ -196,36 +182,17 @@ def test_determine_new_python_dependencies_without_dry_run_with_existing(self, _ python_deps, ) - @patch("addonmanager_python_deps.call_pip") - @patch("addonmanager_python_deps.pip_has_dry_run_support", return_value=True) - def test_determine_new_python_dependencies_with_dry_run(self, _, mock_call_pip: MagicMock): - """If pip supports dry-run, then this method should use pip to get a complete list of new - dependencies. It should ONLY return new ones, not existing ones, and should include - resolution of the complete dependency chain as determined by pip. Updates are not - reported, only newly required packages.""" - - addon_1 = self.MinimalAddon("addon_1", ["py_req_1", "py_req_2"], ["py_opt_1", "py_opt_2"]) - addon_2 = self.MinimalAddon("addon_2", ["py_req_3", "py_req_4"], ["py_opt_2", "py_opt_3"]) - - mock_call_pip.return_value = [ - "Would install requests-2.31.0", - "Would install numpy-1.25.2", - "Would update urllib3-1.26.15 to urllib3-2.0.4", - "Would update chardet-4.0.0 to charset_normalizer-3.3.0", - "Would install idna-3.4", - "Ignoring already satisfied: pandas", - ] + def test_determine_new_python_dependencies_normalizes_installed_names(self): + """An installed package matches a declared dependency even when their names differ only + by PEP 503 normalization, so it is not reported as new.""" + addon = self.MinimalAddon("addon", ["KiCad_Python"], []) - addons = [addon_1, addon_2] model = PythonPackageListModel([]) - python_deps = model.determine_new_python_dependencies(addons) - self.assertEqual( - {"idna", "requests", "numpy"}, - python_deps, - ) + model.package_list = [PackageInfo("kicad-python", "0.7.1", "", [])] + python_deps = model.determine_new_python_dependencies(addon) + self.assertEqual(set(), python_deps) - @patch("addonmanager_python_deps.pip_has_dry_run_support", return_value=False) - def test_determine_new_python_dependencies_single_addon_given(self, _): + def test_determine_new_python_dependencies_single_addon_given(self): """Ensure the code still works with only a single addon passed in""" addon_1 = self.MinimalAddon("addon_1", ["py_req_1", "py_req_2"], ["py_opt_1", "py_opt_2"]) diff --git a/addonmanager_python_deps.py b/addonmanager_python_deps.py index 92328b3..0939e15 100644 --- a/addonmanager_python_deps.py +++ b/addonmanager_python_deps.py @@ -41,6 +41,7 @@ ) import addonmanager_freecad_interface as fci +from addonmanager_python_constraints import get_constraints from PySideWrapper import QtCore @@ -51,16 +52,6 @@ class PipFailed(Exception): """Exception thrown when pip times out or otherwise fails to return valid results""" -def pip_has_dry_run_support() -> bool: - """Returns True if pip supports the --dry-run option, False otherwise.""" - try: - pip_version_string = call_pip(["--version"])[0] - version_str = pip_version_string.split()[1] - return Version(version_str) >= Version("23.1") - except PipFailed: - return False - - 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.""" @@ -92,21 +83,17 @@ class PackageInfo: dependencies: List[str] -def parse_pip_list_output(all_packages, outdated_packages) -> List[PackageInfo]: - """Parses the output from pip into a dictionary with update information in it. The pip - output should be an array of lines of text.""" - - # All Packages output looks like this: - # Package Version - # ---------- ------- - # gitdb 4.0.9 - # setuptools 41.2.0 +def parse_pip_list_output(all_packages, constrained_versions: Dict[str, str]) -> List[PackageInfo]: + """Parse 'pip list --path' output into package information, marking an update as available + whenever the vetted (constrained) version differs from the installed one. The pip output + should be an array of lines of text. - # Outdated Packages output looks like this: - # Package Version Latest Type - # ---------- ------- ------ ----- - # pip 21.0.1 22.1.2 wheel - # setuptools 41.2.0 63.2.0 wheel + All Packages output looks like this: + Package Version + ---------- ------- + gitdb 4.0.9 + setuptools 41.2.0 + """ packages: Dict[str, PackageInfo] = {} skip_counter = 0 @@ -118,26 +105,24 @@ def parse_pip_list_output(all_packages, outdated_packages) -> List[PackageInfo]: if len(entries) > 1: package_name = pep503_normalize(entries[0]) installed_version = entries[1] - packages[package_name] = PackageInfo(package_name, installed_version, "", []) - - skip_counter = 0 - for line in outdated_packages: - if skip_counter < 2: - skip_counter += 1 - continue - entries = line.split() - if len(entries) > 1: - package_name = pep503_normalize(entries[0]) - available_version = entries[2] - if package_name not in packages: - raise RuntimeError( - "all_packages does not contain all packages in outdated_packages" - ) - packages[package_name].available_version = available_version + available_version = _available_update( + installed_version, constrained_versions.get(package_name) + ) + packages[package_name] = PackageInfo( + package_name, installed_version, available_version, [] + ) return list(packages.values()) +def _available_update(installed_version: str, constrained_version: Optional[str]) -> str: + """Return the constrained version when it is set and differs from the installed one, + signaling that an update to the vetted version is available, otherwise an empty string.""" + if constrained_version and constrained_version != installed_version: + return constrained_version + return "" + + class PipCommand(Enum): Install = 0 Upgrade = 1 @@ -196,9 +181,9 @@ def _install_or_update(self) -> None: def _list(self) -> None: try: - outdated_packages_stdout = call_pip(["list", "-o", "--path", self.vendor_path]) all_packages_stdout = call_pip(["list", "--path", self.vendor_path]) - self.package_list = parse_pip_list_output(all_packages_stdout, outdated_packages_stdout) + constrained_versions = get_constraints().constrained_versions() + self.package_list = parse_pip_list_output(all_packages_stdout, constrained_versions) except PipFailed as e: self.error = str(e) @@ -413,36 +398,20 @@ def _cleanup_old_package_versions(self): fci.Console.PrintWarning(f"Error processing versions for {package_name}: {e}\n") def determine_new_python_dependencies(self, addons) -> Set[str]: - """Given a list of Addon objects, finds the Python dependencies for those addons. Also - accepts a single Addon object, in which case only its dependencies are evaluated. If using - a recent version of pip, the dry-run option is used to determine which packages would be - installed, otherwise just lists the dependencies as they are listed in the - addon metadata, filtered to only show new ones.""" + """Given a single Addon or a list of Addons, return the declared Python dependencies + (required and optional) that are not already installed. Names are compared using PEP 503 + normalization, and the original declared names are returned.""" if not isinstance(addons, Iterable): addons = [addons] - python_dependencies = set() + declared_dependencies = set() for addon in addons: - python_dependencies.update(addon.python_requires) - python_dependencies.update(addon.python_optional) - - result = set() - # If we have at least pip 23.1, we can use dry-run: - if pip_has_dry_run_support(): - command = ["install", "--upgrade", "--dry-run"] - command.extend(python_dependencies) - output = call_pip(command) - - for line in output: - match = re.match(r"Would install ([\w\-.]+)-([\d.]+)", line) - if match: - name, _ = match.groups() - result.add(name) - else: - result.update(python_dependencies) - result.difference_update(set([p.name for p in self.package_list])) - return result + declared_dependencies.update(addon.python_requires) + declared_dependencies.update(addon.python_optional) + + installed = {package.name for package in self.package_list} + return {dep for dep in declared_dependencies if pep503_normalize(dep) not in installed} def all_dependencies_installed(self, addon) -> bool: """Returns True if all dependencies for the given addon are installed, or False if not.""" From 226ad7e9da823396ada2311e91b07684cde59345 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 24 Jul 2026 15:06:53 -0500 Subject: [PATCH 4/4] Cleanup old ALLOW list files --- ALLOWED_PYTHON_PACKAGES.txt | 93 ------------------------------------- CMakeLists.txt | 1 - 2 files changed, 94 deletions(-) delete mode 100644 ALLOWED_PYTHON_PACKAGES.txt diff --git a/ALLOWED_PYTHON_PACKAGES.txt b/ALLOWED_PYTHON_PACKAGES.txt deleted file mode 100644 index f4404a3..0000000 --- a/ALLOWED_PYTHON_PACKAGES.txt +++ /dev/null @@ -1,93 +0,0 @@ -# SPDX-License-Identifier: LGPL-2.1-or-later -# SPDX-FileNotice: Part of the AddonManager. - -# This file is a backup copy of the allow-list for python packages. The primary copy is downloaded on each run of the -# Addon Manager from https://github.com/FreeCAD/FreeCAD-Addons. This copy is only used in the event that the online -# version is not available. - -# Note that this is NOT a requirements.txt-format file, no version information may be -# specified, and no wildcards are supported. - -# Allow these packages to be installed: -PyOpenGL -PyYAML -aiofiles -asyncua -atomicwrites -autobahn -awkward -backoff -beautifulsoup4 -blinker -capytaine -certifi -certify -charset-normalizer -comtypes -distro -docutils -etabs-api -ezdxf -geomdl -gmsh -gmsh-dev -hausdorff -idna -ifcopenshell -lxml -markdown -matplotlib -meshio -monotonic -msgpack -networkx -nine -nodeeditor -numba -numpy -ocp -olefile -openexr -openpyxl -pandas -pillow -ply -posthog -protobuf -py-slvs -pycollada -pydocx -pyg4ometry -pygit2 -pyjwt -pynastran -pyopenxr -pyoptools -pypresence -python-dateutil -python-docx -python-dotenv -pyvista -qtpy -qtrangeslider -requests -rhino3dm -scikit-image -scikit-learn -scikit-sparse -scipy -sentry-sdk -shapely -six -streamdeck -trimesh -triangle -tzlocal -urllib3 -vedo -vermin -wakatime-cli -xgbxml -xlrd -xlutils -xlwt diff --git a/CMakeLists.txt b/CMakeLists.txt index 5c5de63..2c67709 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -73,7 +73,6 @@ SET(AddonManager_SRCS AddonManagerOptions.py AddonManagerOptions.ui AddonStats.py - ALLOWED_PYTHON_PACKAGES.txt compact_view.py compact_view.ui composite_view.py