From abc8a30d0ae6ae8ca1b55769c7beaa479e9d8919 Mon Sep 17 00:00:00 2001 From: Kapale Om Shripad Date: Tue, 30 Dec 2025 18:33:25 +0530 Subject: [PATCH 1/6] Fix: Clean up old Python package versions after update in Flatpak When updating Python dependencies with pip using --target directory, old package version metadata (.dist-info directories) are not always removed. This causes FreeCAD to detect the old version instead of the newly installed version, particularly in Flatpak installations. This fix adds cleanup logic to remove old package version metadata after a successful update, keeping only the newest version of each package. Fixes #26510 (cherry picked from commit 96800d7c1905fcfc2796ab9ea70ef77eb26d3085) --- addonmanager_python_deps.py | 51 +++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/addonmanager_python_deps.py b/addonmanager_python_deps.py index 4bbe091f..b39f6263 100644 --- a/addonmanager_python_deps.py +++ b/addonmanager_python_deps.py @@ -359,6 +359,57 @@ def update_call_finished(self): self.update_complete.emit() if not using_system_pip_installation_location(): shutil.rmtree(self.vendor_path + ".old") + # Clean up old package versions that may remain after update + self._cleanup_old_package_versions() + + def _cleanup_old_package_versions(self): + """Remove old package version metadata directories after an update. + + When pip updates packages with --target, it doesn't always remove old + version metadata (.dist-info directories). This can cause version detection + to find the old version instead of the new one, especially in Flatpak + installations where multiple versions accumulate. + """ + if not os.path.exists(self.vendor_path): + return + + # Group all dist-info directories by package name + package_versions = {} + for item in os.listdir(self.vendor_path): + item_path = os.path.join(self.vendor_path, item) + if os.path.isdir(item_path) and item.endswith('.dist-info'): + # Extract package name and version from directory name + # Format is typically: package_name-version.dist-info + match = re.match(r'^(.+?)-(\d+.+?)\.dist-info$', item) + if match: + package_name = match.group(1).lower().replace('_', '-') + version_str = match.group(2) + + if package_name not in package_versions: + package_versions[package_name] = [] + package_versions[package_name].append((version_str, item_path)) + + # For each package with multiple versions, keep only the newest + for package_name, versions in package_versions.items(): + if len(versions) > 1: + # Sort by version, newest last + try: + versions.sort(key=lambda x: Version(x[0])) + # Remove all but the newest version + for version_str, path in versions[:-1]: + try: + shutil.rmtree(path) + fci.Console.PrintLog( + f"Removed old version metadata for {package_name}: {version_str}\n" + ) + except (OSError, PermissionError) as e: + fci.Console.PrintWarning( + f"Could not remove old version metadata {path}: {e}\n" + ) + except Exception as e: + 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 From fee8aa1303e54cb77f534b8eac5759f9106aa1e3 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 31 Dec 2025 03:18:46 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci (cherry picked from commit 3ab0ce7fd5aa8116cbbcc3ce676d11a895163816) --- addonmanager_python_deps.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/addonmanager_python_deps.py b/addonmanager_python_deps.py index b39f6263..5188b4d8 100644 --- a/addonmanager_python_deps.py +++ b/addonmanager_python_deps.py @@ -364,7 +364,7 @@ def update_call_finished(self): def _cleanup_old_package_versions(self): """Remove old package version metadata directories after an update. - + When pip updates packages with --target, it doesn't always remove old version metadata (.dist-info directories). This can cause version detection to find the old version instead of the new one, especially in Flatpak @@ -372,23 +372,23 @@ def _cleanup_old_package_versions(self): """ if not os.path.exists(self.vendor_path): return - + # Group all dist-info directories by package name package_versions = {} for item in os.listdir(self.vendor_path): item_path = os.path.join(self.vendor_path, item) - if os.path.isdir(item_path) and item.endswith('.dist-info'): + if os.path.isdir(item_path) and item.endswith(".dist-info"): # Extract package name and version from directory name # Format is typically: package_name-version.dist-info - match = re.match(r'^(.+?)-(\d+.+?)\.dist-info$', item) + match = re.match(r"^(.+?)-(\d+.+?)\.dist-info$", item) if match: - package_name = match.group(1).lower().replace('_', '-') + package_name = match.group(1).lower().replace("_", "-") version_str = match.group(2) - + if package_name not in package_versions: package_versions[package_name] = [] package_versions[package_name].append((version_str, item_path)) - + # For each package with multiple versions, keep only the newest for package_name, versions in package_versions.items(): if len(versions) > 1: @@ -407,9 +407,7 @@ def _cleanup_old_package_versions(self): f"Could not remove old version metadata {path}: {e}\n" ) except Exception as e: - fci.Console.PrintWarning( - f"Error processing versions for {package_name}: {e}\n" - ) + 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 From 2f0dd886e31faa098d691998afd214eb8ceccc7e Mon Sep 17 00:00:00 2001 From: Kapale Om Shripad Date: Mon, 5 Jan 2026 11:23:28 +0530 Subject: [PATCH 3/6] Add comprehensive tests for _cleanup_old_package_versions method Added unit tests covering: - Removal of old versions while keeping newest - Single version packages (no removal) - Empty and nonexistent directories - Permission error handling - PEP 503 package name normalization - Non-.dist-info directory filtering - Invalid version format handling Tests use proper mocking to avoid filesystem operations and follow existing test patterns in the codebase. These tests verify the cleanup logic works correctly across various scenarios including edge cases like permission errors and malformed directory names. All tests use mocks to ensure fast, reliable execution without touching the actual filesystem. (cherry picked from commit 7079ecb583d225ea3ab5d7ba3eb4bc589b584409) --- AddonManagerTest/app/test_python_deps.py | 167 +++++++++++++++++++++++ 1 file changed, 167 insertions(+) diff --git a/AddonManagerTest/app/test_python_deps.py b/AddonManagerTest/app/test_python_deps.py index e5bed28e..78b77b4d 100644 --- a/AddonManagerTest/app/test_python_deps.py +++ b/AddonManagerTest/app/test_python_deps.py @@ -268,3 +268,170 @@ def test_update_packages_pip_failure(self, mock_print_error, mock_print_log, moc mock_call_pip.assert_called_once() mock_print_error.assert_called_once_with("upgrade failed\n") + + class TestCleanupOldPackageVersions(unittest.TestCase): + """Tests for the _cleanup_old_package_versions method""" + + @patch("addonmanager_python_deps.os.path.exists") + @patch("addonmanager_python_deps.os.listdir") + @patch("addonmanager_python_deps.shutil.rmtree") + @patch("addonmanager_python_deps.fci.Console.PrintLog") + def test_cleanup_removes_old_versions_keeps_newest( + self, mock_print_log, mock_rmtree, mock_listdir, mock_exists + ): + """Test that old package versions are removed and newest is kept""" + mock_exists.return_value = True + mock_listdir.return_value = [ + "requests-2.28.0.dist-info", + "requests-2.31.0.dist-info", + "numpy-1.24.0.dist-info", + "numpy-1.26.0.dist-info", + "numpy-1.25.2.dist-info", + "other_file.txt", + ] + + model = PythonPackageListModel([]) + model.vendor_path = "/fake/path" + model._cleanup_old_package_versions() + + # Should remove old versions but keep newest + self.assertEqual(mock_rmtree.call_count, 3) + removed_paths = [call[0][0] for call in mock_rmtree.call_args_list] + + # Check old versions were removed + self.assertIn("/fake/path/requests-2.28.0.dist-info", removed_paths) + self.assertIn("/fake/path/numpy-1.24.0.dist-info", removed_paths) + self.assertIn("/fake/path/numpy-1.25.2.dist-info", removed_paths) + + # Verify logging happened + self.assertEqual(mock_print_log.call_count, 3) + + @patch("addonmanager_python_deps.os.path.exists") + @patch("addonmanager_python_deps.os.listdir") + @patch("addonmanager_python_deps.shutil.rmtree") + def test_cleanup_single_version_no_removal(self, mock_rmtree, mock_listdir, mock_exists): + """Test that packages with only one version are not touched""" + mock_exists.return_value = True + mock_listdir.return_value = [ + "requests-2.31.0.dist-info", + "numpy-1.26.0.dist-info", + "pandas-2.1.0.dist-info", + ] + + model = PythonPackageListModel([]) + model.vendor_path = "/fake/path" + model._cleanup_old_package_versions() + + # No removals should happen when only one version exists per package + mock_rmtree.assert_not_called() + + @patch("addonmanager_python_deps.os.path.exists") + def test_cleanup_nonexistent_directory(self, mock_exists): + """Test graceful handling when vendor path doesn't exist""" + mock_exists.return_value = False + + model = PythonPackageListModel([]) + model.vendor_path = "/nonexistent/path" + model._cleanup_old_package_versions() + + @patch("addonmanager_python_deps.os.path.exists") + @patch("addonmanager_python_deps.os.listdir") + @patch("addonmanager_python_deps.shutil.rmtree") + def test_cleanup_empty_directory(self, mock_rmtree, mock_listdir, mock_exists): + """Test handling of empty vendor directory""" + mock_exists.return_value = True + mock_listdir.return_value = [] + + model = PythonPackageListModel([]) + model.vendor_path = "/fake/path" + model._cleanup_old_package_versions() + mock_rmtree.assert_not_called() + + @patch("addonmanager_python_deps.os.path.exists") + @patch("addonmanager_python_deps.os.listdir") + @patch("addonmanager_python_deps.os.path.isdir") + @patch("addonmanager_python_deps.shutil.rmtree") + @patch("addonmanager_python_deps.fci.Console.PrintWarning") + def test_cleanup_handles_permission_error( + self, mock_print_warning, mock_rmtree, mock_isdir, mock_listdir, mock_exists + ): + """Test that permission errors are handled gracefully""" + mock_exists.return_value = True + mock_listdir.return_value = [ + "requests-2.28.0.dist-info", + "requests-2.31.0.dist-info", + ] + mock_isdir.return_value = True + mock_rmtree.side_effect = PermissionError("Permission denied") + + model = PythonPackageListModel([]) + model.vendor_path = "/fake/path" + model._cleanup_old_package_versions() + mock_print_warning.assert_called() + + @patch("addonmanager_python_deps.os.path.exists") + @patch("addonmanager_python_deps.os.listdir") + @patch("addonmanager_python_deps.os.path.isdir") + @patch("addonmanager_python_deps.shutil.rmtree") + def test_cleanup_normalizes_package_names( + self, mock_rmtree, mock_isdir, mock_listdir, mock_exists + ): + """Test that package names are normalized per PEP 503 (underscores to dashes)""" + mock_exists.return_value = True + mock_listdir.return_value = [ + "my_package-1.0.0.dist-info", + "my-package-2.0.0.dist-info", + ] + mock_isdir.return_value = True + + model = PythonPackageListModel([]) + model.vendor_path = "/fake/path" + model._cleanup_old_package_versions() + + # Should remove old version (they're the same package after normalization) + mock_rmtree.assert_called_once_with("/fake/path/my_package-1.0.0.dist-info") + + @patch("addonmanager_python_deps.os.path.exists") + @patch("addonmanager_python_deps.os.listdir") + @patch("addonmanager_python_deps.os.path.isdir") + @patch("addonmanager_python_deps.shutil.rmtree") + def test_cleanup_ignores_non_dist_info_directories( + self, mock_rmtree, mock_isdir, mock_listdir, mock_exists + ): + """Test that only .dist-info directories are processed""" + mock_exists.return_value = True + mock_listdir.return_value = [ + "requests-2.28.0.dist-info", + "requests-2.31.0.dist-info", + "some_package", + "__pycache__", + "random_file.txt", + ] + mock_isdir.return_value = True + + model = PythonPackageListModel([]) + model.vendor_path = "/fake/path" + model._cleanup_old_package_versions() + mock_rmtree.assert_called_once_with("/fake/path/requests-2.28.0.dist-info") + + @patch("addonmanager_python_deps.os.path.exists") + @patch("addonmanager_python_deps.os.listdir") + @patch("addonmanager_python_deps.os.path.isdir") + @patch("addonmanager_python_deps.shutil.rmtree") + @patch("addonmanager_python_deps.fci.Console.PrintWarning") + def test_cleanup_handles_invalid_version_format( + self, mock_print_warning, mock_rmtree, mock_isdir, mock_listdir, mock_exists + ): + """Test handling of malformed version strings""" + mock_exists.return_value = True + mock_listdir.return_value = [ + "badpackage-invalid.version.dist-info", + "goodpackage-1.0.0.dist-info", + "goodpackage-2.0.0.dist-info", + ] + mock_isdir.return_value = True + + model = PythonPackageListModel([]) + model.vendor_path = "/fake/path" + model._cleanup_old_package_versions() + mock_rmtree.assert_called_once_with("/fake/path/goodpackage-1.0.0.dist-info") From 8bd89337f3a6d778caa6f7d499803c62af14ce21 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 5 Jan 2026 05:54:13 +0000 Subject: [PATCH 4/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci (cherry picked from commit bd1b6ac52370aa99301c328ed6eec763469d9e67) --- AddonManagerTest/app/test_python_deps.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/AddonManagerTest/app/test_python_deps.py b/AddonManagerTest/app/test_python_deps.py index 78b77b4d..6fd3f910 100644 --- a/AddonManagerTest/app/test_python_deps.py +++ b/AddonManagerTest/app/test_python_deps.py @@ -297,12 +297,12 @@ def test_cleanup_removes_old_versions_keeps_newest( # Should remove old versions but keep newest self.assertEqual(mock_rmtree.call_count, 3) removed_paths = [call[0][0] for call in mock_rmtree.call_args_list] - + # Check old versions were removed self.assertIn("/fake/path/requests-2.28.0.dist-info", removed_paths) self.assertIn("/fake/path/numpy-1.24.0.dist-info", removed_paths) self.assertIn("/fake/path/numpy-1.25.2.dist-info", removed_paths) - + # Verify logging happened self.assertEqual(mock_print_log.call_count, 3) @@ -341,7 +341,7 @@ def test_cleanup_empty_directory(self, mock_rmtree, mock_listdir, mock_exists): """Test handling of empty vendor directory""" mock_exists.return_value = True mock_listdir.return_value = [] - + model = PythonPackageListModel([]) model.vendor_path = "/fake/path" model._cleanup_old_package_versions() @@ -363,7 +363,7 @@ def test_cleanup_handles_permission_error( ] mock_isdir.return_value = True mock_rmtree.side_effect = PermissionError("Permission denied") - + model = PythonPackageListModel([]) model.vendor_path = "/fake/path" model._cleanup_old_package_versions() @@ -380,7 +380,7 @@ def test_cleanup_normalizes_package_names( mock_exists.return_value = True mock_listdir.return_value = [ "my_package-1.0.0.dist-info", - "my-package-2.0.0.dist-info", + "my-package-2.0.0.dist-info", ] mock_isdir.return_value = True @@ -403,7 +403,7 @@ def test_cleanup_ignores_non_dist_info_directories( mock_listdir.return_value = [ "requests-2.28.0.dist-info", "requests-2.31.0.dist-info", - "some_package", + "some_package", "__pycache__", "random_file.txt", ] From ff6eae07e8e5678f6878490bdcff8d8feac66a59 Mon Sep 17 00:00:00 2001 From: Kapale Om Shripad Date: Tue, 6 Jan 2026 09:43:05 +0530 Subject: [PATCH 5/6] Fix tests for Windows compatibility and remove logging assertion - Use os.path.join() for all path construction to ensure Windows compatibility - Remove logging count assertion as suggested in review - Tests now work across all platforms (Windows, Linux, macOS) (cherry picked from commit b6cf067939d54ec36590a544c431c616554f0016) --- AddonManagerTest/app/test_python_deps.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/AddonManagerTest/app/test_python_deps.py b/AddonManagerTest/app/test_python_deps.py index 6fd3f910..ba499cb8 100644 --- a/AddonManagerTest/app/test_python_deps.py +++ b/AddonManagerTest/app/test_python_deps.py @@ -20,6 +20,7 @@ # * . * # * * # *************************************************************************** +import os import subprocess import unittest from unittest.mock import MagicMock, patch @@ -298,13 +299,10 @@ def test_cleanup_removes_old_versions_keeps_newest( self.assertEqual(mock_rmtree.call_count, 3) removed_paths = [call[0][0] for call in mock_rmtree.call_args_list] - # Check old versions were removed - self.assertIn("/fake/path/requests-2.28.0.dist-info", removed_paths) - self.assertIn("/fake/path/numpy-1.24.0.dist-info", removed_paths) - self.assertIn("/fake/path/numpy-1.25.2.dist-info", removed_paths) - - # Verify logging happened - self.assertEqual(mock_print_log.call_count, 3) + # Check old versions were removed (works on all platforms) + self.assertIn(os.path.join("/fake/path", "requests-2.28.0.dist-info"), removed_paths) + self.assertIn(os.path.join("/fake/path", "numpy-1.24.0.dist-info"), removed_paths) + self.assertIn(os.path.join("/fake/path", "numpy-1.25.2.dist-info"), removed_paths) @patch("addonmanager_python_deps.os.path.exists") @patch("addonmanager_python_deps.os.listdir") @@ -389,7 +387,7 @@ def test_cleanup_normalizes_package_names( model._cleanup_old_package_versions() # Should remove old version (they're the same package after normalization) - mock_rmtree.assert_called_once_with("/fake/path/my_package-1.0.0.dist-info") + mock_rmtree.assert_called_once_with(os.path.join("/fake/path", "my_package-1.0.0.dist-info")) @patch("addonmanager_python_deps.os.path.exists") @patch("addonmanager_python_deps.os.listdir") @@ -412,7 +410,7 @@ def test_cleanup_ignores_non_dist_info_directories( model = PythonPackageListModel([]) model.vendor_path = "/fake/path" model._cleanup_old_package_versions() - mock_rmtree.assert_called_once_with("/fake/path/requests-2.28.0.dist-info") + mock_rmtree.assert_called_once_with(os.path.join("/fake/path", "requests-2.28.0.dist-info")) @patch("addonmanager_python_deps.os.path.exists") @patch("addonmanager_python_deps.os.listdir") @@ -434,4 +432,4 @@ def test_cleanup_handles_invalid_version_format( model = PythonPackageListModel([]) model.vendor_path = "/fake/path" model._cleanup_old_package_versions() - mock_rmtree.assert_called_once_with("/fake/path/goodpackage-1.0.0.dist-info") + mock_rmtree.assert_called_once_with(os.path.join("/fake/path", "goodpackage-1.0.0.dist-info")) From 963ad66b4c7c4a13794b04ccca3605a36a9eda67 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 6 Jan 2026 04:13:16 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci (cherry picked from commit f3cd818f79945d1ed207e45ff892a3c7c46a2504) --- AddonManagerTest/app/test_python_deps.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/AddonManagerTest/app/test_python_deps.py b/AddonManagerTest/app/test_python_deps.py index ba499cb8..0de846c1 100644 --- a/AddonManagerTest/app/test_python_deps.py +++ b/AddonManagerTest/app/test_python_deps.py @@ -387,7 +387,9 @@ def test_cleanup_normalizes_package_names( model._cleanup_old_package_versions() # Should remove old version (they're the same package after normalization) - mock_rmtree.assert_called_once_with(os.path.join("/fake/path", "my_package-1.0.0.dist-info")) + mock_rmtree.assert_called_once_with( + os.path.join("/fake/path", "my_package-1.0.0.dist-info") + ) @patch("addonmanager_python_deps.os.path.exists") @patch("addonmanager_python_deps.os.listdir") @@ -410,7 +412,9 @@ def test_cleanup_ignores_non_dist_info_directories( model = PythonPackageListModel([]) model.vendor_path = "/fake/path" model._cleanup_old_package_versions() - mock_rmtree.assert_called_once_with(os.path.join("/fake/path", "requests-2.28.0.dist-info")) + mock_rmtree.assert_called_once_with( + os.path.join("/fake/path", "requests-2.28.0.dist-info") + ) @patch("addonmanager_python_deps.os.path.exists") @patch("addonmanager_python_deps.os.listdir") @@ -432,4 +436,6 @@ def test_cleanup_handles_invalid_version_format( model = PythonPackageListModel([]) model.vendor_path = "/fake/path" model._cleanup_old_package_versions() - mock_rmtree.assert_called_once_with(os.path.join("/fake/path", "goodpackage-1.0.0.dist-info")) + mock_rmtree.assert_called_once_with( + os.path.join("/fake/path", "goodpackage-1.0.0.dist-info") + )