diff --git a/AddonCatalog.py b/AddonCatalog.py index a5ca75a0..ec140df0 100644 --- a/AddonCatalog.py +++ b/AddonCatalog.py @@ -83,6 +83,8 @@ class AddonCatalogEntry: curated: bool = True # Generated by the cache system sparse_cache: bool = False # Generated by the cache system relative_cache_path: str = "" # Generated by the cache system + git_hash: Optional[str] = None # Generated by the cache system + git_tag: Optional[str] = None # Generated by the cache system def __init__(self, raw_data: Dict[str, str]) -> None: """Create an AddonDictionaryEntry from the raw JSON data""" @@ -340,6 +342,19 @@ def add_metadata_to_entry( raise RuntimeError(f"Addon {addon_id} index out of range") self._dictionary[addon_id][index].metadata = metadata + def add_git_info_to_entry( + self, addon_id: str, index: int, commit_hash: str | None, tag: str | None + ) -> None: + """Adds git commit hash and tag to an AddonCatalogEntry""" + entries = self._dictionary.get(addon_id) + if entries is None: + raise RuntimeError(f"Addon {addon_id} does not exist") + if index >= len(entries): + raise RuntimeError(f"Addon {addon_id} index out of range") + entry = entries[index] + entry.git_hash = commit_hash + entry.git_tag = tag + def get_available_branches(self, addon_id: str) -> List[str]: """For a given ID, get the list of available branches compatible with this version of FreeCAD. diff --git a/AddonCatalogCacheCreator.py b/AddonCatalogCacheCreator.py index 61f1e561..90fad047 100644 --- a/AddonCatalogCacheCreator.py +++ b/AddonCatalogCacheCreator.py @@ -26,7 +26,7 @@ import shutil import sys from dataclasses import is_dataclass, fields -from typing import Any, List, Optional +from typing import Any, List, Optional, Dict, Tuple import base64 import enum @@ -34,16 +34,16 @@ import io import json import os +import re import requests import subprocess -from typing import List import xml.etree.ElementTree import zipfile import AddonCatalog import addonmanager_metadata import addonmanager_utilities as utils - +import addonmanager_icon_utilities as icon_utils ADDON_CATALOG_URL = "https://raw.githubusercontent.com/FreeCAD/Addons/main/Data/Index.json" BASE_DIRECTORY = "./CatalogCache" @@ -114,6 +114,8 @@ def __init__(self): else: self.cwd = os.path.normpath(os.path.join(os.getcwd(), BASE_DIRECTORY)) self._cache = {} + self._sanitize_counter = 0 + self._directory_name_cache: Dict[str, str] = {} def write(self, addon_id: Optional[str] = None) -> None: original_working_directory = os.getcwd() @@ -227,8 +229,35 @@ def create_local_copy_of_single_addon( continue metadata = self.generate_cache_entry(addon_id, index, catalog_entry) self.catalog.add_metadata_to_entry(addon_id, index, metadata) + git_hash, git_tag = self.get_git_info(addon_id, index, catalog_entry) + self.catalog.add_git_info_to_entry(addon_id, index, git_hash, git_tag) self.create_zip_of_entry(addon_id, index, catalog_entry) + def get_git_info( + self, addon_id: str, index: int, catalog_entry: AddonCatalog.AddonCatalogEntry + ) -> Tuple[str | None, str | None]: + """Get git commit hash and tag if available.""" + dirname = self.get_directory_name(addon_id, index, catalog_entry) + if not os.path.exists(os.path.join(self.cwd, dirname, ".git")): + return None, None + repo = os.path.join(self.cwd, dirname) + hash_cmd = ["git", "rev-parse", "HEAD"] + tag_cmd = ["git", "describe", "--tags", "--exact-match"] + results = [] + for cmd in (hash_cmd, tag_cmd): + try: + result = subprocess.run( + cmd, + capture_output=True, + text=True, + check=True, + cwd=repo, + ) + results.append(result.stdout.strip()) + except (subprocess.CalledProcessError, OSError, FileNotFoundError): + results.append(None) + return tuple(results) + def generate_cache_entry( self, addon_id: str, index: int, catalog_entry: AddonCatalog.AddonCatalogEntry ) -> Optional[AddonCatalog.CatalogEntryMetadata]: @@ -253,7 +282,7 @@ def generate_cache_entry( with open(path_to_metadata, "r", encoding="utf-8") as f: cache_entry.metadata_txt = f.read() - dirname = CacheWriter.get_directory_name(addon_id, index, catalog_entry) + dirname = self.get_directory_name(addon_id, index, catalog_entry) if os.path.exists(os.path.join(self.cwd, dirname, ".git")): old_dir = os.getcwd() os.chdir(os.path.join(self.cwd, dirname)) @@ -293,17 +322,46 @@ def generate_cache_entry_from_package_xml( os.path.dirname(path_to_package_xml), relative_icon_path ) if os.path.exists(absolute_icon_path): + icon_data_is_good = True with open(absolute_icon_path, "rb") as f: + icon_data = None try: - cache_entry.icon_data = base64.b64encode(f.read()).decode("utf-8") + icon_data = f.read() except IOError as e: print(f"ERROR: IO Error while reading icon file {absolute_icon_path}") print(e) + icon_data_is_good = False except Exception as e: print(f"ERROR: Unknown error while reading icon file {absolute_icon_path}") print(e) + icon_data_is_good = False + if icon_data is not None: + if absolute_icon_path.lower().endswith(".svg"): + try: + if not icon_utils.is_svg_bytes(icon_data): + self.icon_errors[metadata.name] = { + "valid_icon_path": relative_icon_path, + "error_message": "SVG file does not have valid XML header", + } + icon_data_is_good = False + except icon_utils.BadIconData as e: + self.icon_errors[metadata.name] = { + "valid_icon_path": relative_icon_path, + "error_message": str(e), + } + icon_data_is_good = False + elif absolute_icon_path.lower().endswith(".png"): + if icon_utils.png_has_duplicate_iccp(icon_data): + self.icon_errors[metadata.name] = { + "valid_icon_path": relative_icon_path, + "error_message": "PNG data has duplicate iCCP chunk", + } + icon_data_is_good = False + + if icon_data_is_good: + cache_entry.icon_data = base64.b64encode(icon_data).decode("utf-8") else: - self.icon_errors[metadata.name] = relative_icon_path + self.icon_errors[metadata.name] = {"bad_icon_path": relative_icon_path} print(f"ERROR: Could not find icon file {absolute_icon_path}") return cache_entry @@ -334,16 +392,44 @@ def create_local_copy_of_single_addon_with_git_sparse( print(f"ERROR: Failed to clone or update {addon_id} from {catalog_entry.repository}.") print(f"ERROR: {e}") - @staticmethod - def get_directory_name(addon_id, index, catalog_entry): + def sanitize_directory_name(self, expected_name: str) -> str: + """Take a string and return a sanitized version suitable for use as a directory name.""" + if expected_name in self._directory_name_cache: + return self._directory_name_cache[expected_name] + self._sanitize_counter += 1 + forbidden_chars = r'<>:"|?*' + if os.path.sep == "/": + forbidden_chars += "\\\\" + else: + forbidden_chars += "/" + sanitized = re.sub(f"[{forbidden_chars}]", str(self._sanitize_counter), expected_name) + sanitized = sanitized.rstrip(" .") + reserved = { + "con", + "prn", + "aux", + "nul", + *(f"com{i}" for i in range(1, 10)), + *(f"lpt{i}" for i in range(1, 10)), + } + components = sanitized.split(os.path.sep) + for i, comp in enumerate(components): + if comp.lower() in reserved: + components[i] = comp + "-RES" + sanitized = os.path.sep.join(components) + + self._directory_name_cache[expected_name] = sanitized + return sanitized + + def get_directory_name(self, addon_id, index, catalog_entry): expected_name = os.path.join(addon_id, str(index) + "-") if catalog_entry.branch_display_name: - expected_name += catalog_entry.branch_display_name.replace("/", "-") + expected_name += catalog_entry.branch_display_name elif catalog_entry.git_ref: - expected_name += catalog_entry.git_ref.replace("/", "-") + expected_name += catalog_entry.git_ref else: expected_name += "unknown-branch-name" - return expected_name + return self.sanitize_directory_name(expected_name) def create_local_copy_of_single_addon_with_zip( self, addon_id: str, index: int, catalog_entry: AddonCatalog.AddonCatalogEntry @@ -562,7 +648,7 @@ def create_zip_of_entry( zip file is written to a file with the same name as the calculated addon cache directory in the current working directory.""" - dirname = CacheWriter.get_directory_name(addon_id, index, catalog_entry) + dirname = self.get_directory_name(addon_id, index, catalog_entry) start_dir = os.path.join(self.cwd, dirname) zip_file_path = os.path.join(self.cwd, f"{dirname}.zip") temp_file_path = zip_file_path + ".new" diff --git a/AddonManagerTest/app/test_addon_catalog_cache_creator.py b/AddonManagerTest/app/test_addon_catalog_cache_creator.py index d9306ba9..a0da9463 100644 --- a/AddonManagerTest/app/test_addon_catalog_cache_creator.py +++ b/AddonManagerTest/app/test_addon_catalog_cache_creator.py @@ -114,28 +114,32 @@ def setUp(self): def test_get_directory_name_with_branch_name(self): """If a branch display name is present, that should be appended to the name.""" + writer = accc.CacheWriter() ace = AddonCatalog.AddonCatalogEntry({"branch_display_name": "test_branch"}) - result = accc.CacheWriter.get_directory_name("test_addon", 99, ace) + result = writer.get_directory_name("test_addon", 99, ace) self.assertEqual(result, os.path.join("test_addon", "99-test_branch")) def test_get_directory_name_with_git_ref(self): """If a branch display name is present, that should be appended to the name.""" + writer = accc.CacheWriter() ace = AddonCatalog.AddonCatalogEntry({"git_ref": "test_ref"}) - result = accc.CacheWriter.get_directory_name("test_addon", 99, ace) + result = writer.get_directory_name("test_addon", 99, ace) self.assertEqual(result, os.path.join("test_addon", "99-test_ref")) def test_get_directory_name_with_branch_and_ref(self): """If a branch and git ref are both present, then the branch display name is used.""" + writer = accc.CacheWriter() ace = AddonCatalog.AddonCatalogEntry( {"branch_display_name": "test_branch", "git_ref": "test_ref"} ) - result = accc.CacheWriter.get_directory_name("test_addon", 99, ace) + result = writer.get_directory_name("test_addon", 99, ace) self.assertEqual(result, os.path.join("test_addon", "99-test_branch")) def test_get_directory_name_with_no_information(self): """If there is no branch name or git ref information, a valid directory name is still generated.""" + writer = accc.CacheWriter() ace = AddonCatalog.AddonCatalogEntry({}) - result = accc.CacheWriter.get_directory_name("test_addon", 99, ace) + result = writer.get_directory_name("test_addon", 99, ace) self.assertTrue(result.startswith(os.path.join("test_addon", "99"))) def test_find_file_with_existing_file(self): @@ -189,8 +193,7 @@ def test_generate_cache_entry_from_package_xml(self, _): @patch("AddonCatalogCacheCreator.addonmanager_metadata.MetadataReader.from_bytes") @patch("AddonCatalogCacheCreator.CacheWriter.get_icon_from_metadata") def test_generate_cache_entry_from_package_xml_with_icon(self, mock_icon, _): - """Given a metadata file that contains an icon, that icon's contents are - base64-encoded and embedded in the cache.""" + """Given a metadata file that contains an icon, that icon's contents are base64-encoded and embedded in the cache.""" file_path = os.path.abspath( os.path.join("home", "cache", "TestMod", "1-main", "package.xml") @@ -198,14 +201,15 @@ def test_generate_cache_entry_from_package_xml_with_icon(self, mock_icon, _): icon_path = os.path.abspath( os.path.join("home", "cache", "TestMod", "1-main", "icons", "TestMod.svg") ) + icon_data = "" self.fake_fs().create_file(file_path, contents="test data") - self.fake_fs().create_file(icon_path, contents="test icon data") + self.fake_fs().create_file(icon_path, contents=icon_data) mock_icon.return_value = os.path.join("icons", "TestMod.svg") writer = accc.CacheWriter() writer.cwd = os.path.abspath(os.path.join("home", "cache")) cache_entry = writer.generate_cache_entry_from_package_xml(file_path) self.assertEqual( - base64.b64encode("test icon data".encode("utf-8")).decode("utf-8"), + base64.b64encode(icon_data.encode("utf-8")).decode("utf-8"), cache_entry.icon_data, ) diff --git a/AddonManagerTest/app/test_freecad_interface.py b/AddonManagerTest/app/test_freecad_interface.py index 2088bab1..a8646062 100644 --- a/AddonManagerTest/app/test_freecad_interface.py +++ b/AddonManagerTest/app/test_freecad_interface.py @@ -75,39 +75,34 @@ def test_log_no_freecad(self): """Test that if the FreeCAD import fails, the logger is set up correctly, and implements PrintLog""" sys.modules["FreeCAD"] = None - with patch("addonmanager_freecad_interface.logging", new=MagicMock()) as mock_logging: - import addonmanager_freecad_interface as fc + import addonmanager_freecad_interface as fc + with self.assertLogs("addonmanager", level="DEBUG") as cm: fc.Console.PrintLog("Test output") - self.assertTrue(isinstance(fc.Console, fc.ConsoleReplacement)) - self.assertTrue(mock_logging.log.called) def test_message_no_freecad(self): """Test that if the FreeCAD import fails the logger implements PrintMessage""" sys.modules["FreeCAD"] = None - with patch("addonmanager_freecad_interface.logging", new=MagicMock()) as mock_logging: - import addonmanager_freecad_interface as fc + import addonmanager_freecad_interface as fc + with self.assertLogs("addonmanager", level="INFO") as cm: fc.Console.PrintMessage("Test output") - self.assertTrue(mock_logging.info.called) def test_warning_no_freecad(self): """Test that if the FreeCAD import fails the logger implements PrintWarning""" sys.modules["FreeCAD"] = None - with patch("addonmanager_freecad_interface.logging", new=MagicMock()) as mock_logging: - import addonmanager_freecad_interface as fc + import addonmanager_freecad_interface as fc + with self.assertLogs("addonmanager", level="WARNING") as cm: fc.Console.PrintWarning("Test output") - self.assertTrue(mock_logging.warning.called) def test_error_no_freecad(self): """Test that if the FreeCAD import fails the logger implements PrintError""" sys.modules["FreeCAD"] = None - with patch("addonmanager_freecad_interface.logging", new=MagicMock()) as mock_logging: - import addonmanager_freecad_interface as fc + import addonmanager_freecad_interface as fc + with self.assertLogs("addonmanager", level="ERROR") as cm: fc.Console.PrintError("Test output") - self.assertTrue(mock_logging.error.called) class TestParameters(WrapTestFreeCADImports): diff --git a/AddonManagerTest/app/test_macro_cache_creator.py b/AddonManagerTest/app/test_macro_cache_creator.py index 9d01638d..d471f5f6 100644 --- a/AddonManagerTest/app/test_macro_cache_creator.py +++ b/AddonManagerTest/app/test_macro_cache_creator.py @@ -23,6 +23,7 @@ from unittest.mock import patch, MagicMock from MacroCacheCreator import MacroCatalog, CacheWriter +import addonmanager_freecad_interface as fci class TestMacroCatalog(unittest.TestCase): @@ -84,3 +85,30 @@ def test_retrieve_macros_fetch_failure(self, mock_console, mock_add_macro, mock_ instance.retrieve_macros_from_wiki() mock_add_macro.assert_not_called() + + def test_log_error(self): + instance = MacroCatalog() + instance.log_error("macro", "Test error") + instance.log_error("macro", "Test error 2") + self.assertIn("macro", instance.macro_errors) + self.assertEqual(len(instance.macro_errors["macro"]), 2) + + def test_fetch_macros_logs_errors(self): + + def fake_git(self): + fci.Console.PrintError("git failure") + + def fake_wiki(self): + fci.Console.PrintWarning("wiki failure") + + instance = MacroCatalog() + with patch.object(type(instance), "retrieve_macros_from_git", fake_git), patch.object( + type(instance), "retrieve_macros_from_wiki", fake_wiki + ): + instance.fetch_macros() + + messages = [record["msg"] for record in instance.log_buffer] + + self.assertIn("git failure", messages) + self.assertIn("wiki failure", messages) + self.assertEqual(len(messages), 2) diff --git a/AddonManagerTest/gui/test_icon_utilities.py b/AddonManagerTest/gui/test_icon_utilities.py index ceacee8d..7264d70d 100644 --- a/AddonManagerTest/gui/test_icon_utilities.py +++ b/AddonManagerTest/gui/test_icon_utilities.py @@ -3,9 +3,11 @@ import gzip import io +import struct import unittest from types import SimpleNamespace from unittest.mock import patch +import zlib import addonmanager_icon_utilities as iu @@ -435,3 +437,85 @@ def test_defaults_initialized_once_and_selected_by_repo_type(self): ) a3 = iu.get_icon_for_addon(addon3) # type: ignore[arg-type] self.assertIs(a3, iu.cached_default_icons["package"]) + + +def build_png_data(chunk_types: list[bytes]): + + def chunk(typ, data): + return ( + struct.pack(">I", len(data)) + + typ + + data + + struct.pack(">I", zlib.crc32(typ + data) & 0xFFFFFFFF) + ) + + png = b"\x89PNG\r\n\x1a\n" + + for chunk_type in chunk_types: + if chunk_type == b"IHDR": + # IHDR: 1x1 RGBA + ihdr = struct.pack(">IIBBBBB", 1, 1, 8, 6, 0, 0, 0) + png += chunk(b"IHDR", ihdr) + + elif chunk_type == b"iCCP": + # iCCP: profile name + compression method + compressed ICC data + icc_profile = b"FakeICCProfile" + icc_compressed = zlib.compress(icc_profile) + iccp = b"icc\x00" + b"\x00" + icc_compressed + png += chunk(b"iCCP", iccp) + + elif chunk_type == b"pHYs": + # 72 DPI + phys = struct.pack(">IIB", 2835, 2835, 1) + png += chunk(b"pHYs", phys) + + elif chunk_type == b"tEXt": + text = b"Comment\x00PNG test file" + png += chunk(b"tEXt", text) + + elif chunk_type == b"IDAT": + # IDAT: white pixel + raw = b"\x00\xff\xff\xff\xff" # filter + RGBA + png += chunk(b"IDAT", zlib.compress(raw)) + + elif chunk_type == b"IEND": + # IEND + png += chunk(b"IEND", b"") + + return png + + +class TestPNGAnalysis(unittest.TestCase): + + def test_get_chunk_types_simplest_possible_png(self): + simple_chunks = [b"IHDR", b"IDAT", b"IEND"] + png_data = build_png_data(simple_chunks) + chunks = iu.get_png_chunk_types(png_data) + self.assertEqual(chunks, simple_chunks) + + def test_get_chunk_types_more_complete_png(self): + more_chunks = [b"IHDR", b"iCCP", b"pHYs", b"tEXt", b"IDAT", b"IEND"] + png_data = build_png_data(more_chunks) + chunks = iu.get_png_chunk_types(png_data) + self.assertEqual(chunks, more_chunks) + + def test_get_chunk_types_extra_elements(self): + more_chunks = [b"IHDR", b"iCCP", b"iCCP", b"pHYs", b"tEXt", b"tEXt", b"IDAT", b"IEND"] + png_data = build_png_data(more_chunks) + chunks = iu.get_png_chunk_types(png_data) + self.assertEqual(chunks, more_chunks) + + def test_valid_no_iccp(self): + simple_chunks = [b"IHDR", b"IDAT", b"IEND"] + png_data = build_png_data(simple_chunks) + self.assertFalse(iu.png_has_duplicate_iccp(png_data)) + + def test_good_iccp(self): + chunks_with_iccp = [b"IHDR", b"iCCP", b"pHYs", b"tEXt", b"IDAT", b"IEND"] + png_data = build_png_data(chunks_with_iccp) + self.assertFalse(iu.png_has_duplicate_iccp(png_data)) + + def test_duplicate_iccp(self): + chunks_with_iccp = [b"IHDR", b"iCCP", b"iCCP", b"pHYs", b"tEXt", b"IDAT", b"IEND"] + png_data = build_png_data(chunks_with_iccp) + self.assertTrue(iu.png_has_duplicate_iccp(png_data)) diff --git a/MacroCacheCreator.py b/MacroCacheCreator.py index 501a749a..556ead0f 100644 --- a/MacroCacheCreator.py +++ b/MacroCacheCreator.py @@ -22,10 +22,15 @@ """The MacroCacheCreator is an independent script run server-side to generate a cache of the macros and their metadata. Supports both git-based and wiki-based macros.""" +from collections import deque +import contextlib import hashlib +import io import json +import logging import os.path import re +import sys import urllib.parse import requests @@ -34,6 +39,9 @@ from addonmanager_macro import Macro from AddonCatalogCacheCreator import CacheWriter # Borrow the git utility method from this class +import addonmanager_icon_utilities as icon_utils + +from PySideWrapper import QtGui GIT_MACROS_URL = "https://github.com/FreeCAD/FreeCAD-macros.git" GIT_MACROS_BRANCH = "master" @@ -64,21 +72,39 @@ def __init__(self): "macros_on_wiki": 0, "macros_on_git": 0, "duplicated_macros": 0, - "errors": 0, + "macros_with_errors": 0, } + self.log_buffer = deque(maxlen=100) def fetch_macros(self): - print("Retrieving macros from git...") - self.retrieve_macros_from_git() - print("Retrieving macros from wiki...") - self.retrieve_macros_from_wiki() - print("Downloading icons...") - for macro in self.macros.values(): - try: - self.get_icon(macro) - except RuntimeError as e: - self.macro_errors[macro.name] = str(e) - self.macro_stats["errors"] = len(self.macro_errors) + logger = logging.getLogger("addonmanager") + with capture_console_output( + logger, + handlers=[DequeHandler(self.log_buffer)], + level=logging.INFO, + propagate=False, + ): + print("Retrieving macros from git...") + self.retrieve_macros_from_git() + print("Retrieving macros from wiki...") + self.retrieve_macros_from_wiki() + print("Downloading icons...") + for number, macro in enumerate(self.macros.values()): + try: + if macro.icon.startswith("/* XPM */"): + macro.xpm = macro.icon + macro.icon = "" + print( + f"{number+1}/{len(self.macros)}: {macro.name} has an XPM icon, skipping download..." + ) + continue + print( + f"{number+1}/{len(self.macros)}: Downloading icon for {macro.name} from {macro.icon}..." + ) + self.get_icon(macro) + except RuntimeError as e: + self.log_error(macro.name, str(e)) + self.macro_stats["macros_with_errors"] = len(self.macro_errors) def create_cache(self) -> str: """Create a cache from the macros in this catalog""" @@ -95,6 +121,7 @@ def retrieve_macros_from_git(self): writer.clone_or_update(GIT_MACROS_CLONE_NAME, GIT_MACROS_URL, GIT_MACROS_BRANCH) except RuntimeError as e: print(f"Failed to clone git macros from {GIT_MACROS_URL}: {e}") + self.log_error("**INTERNAL**", str(e)) return for dirpath, _, filenames in os.walk(os.path.join(os.getcwd(), GIT_MACROS_CLONE_NAME)): @@ -108,13 +135,19 @@ def retrieve_macros_from_git(self): def add_git_macro_to_cache(self, dirpath: str, filename: str): macro = Macro(filename[:-8]) # Remove ".FCMacro". if macro.name in self.macros: - print(f"Ignoring second macro named {macro.name} (found on git)\n") + self.log_error(macro.name, f"Ignoring second macro named {macro.name} (found on git)") return macro.on_git = True absolute_path_to_fcmacro = os.path.join(dirpath, filename) + self.log_buffer.clear() macro.fill_details_from_file(absolute_path_to_fcmacro) macro.src_filename = os.path.relpath(absolute_path_to_fcmacro, os.getcwd()) self.macros[macro.name] = macro + for log_entry in self.log_buffer: + level = log_entry.get("level", logging.INFO) + if level >= logging.WARNING: + self.log_error(macro.name, log_entry["msg"]) + self.log_buffer.clear() def retrieve_macros_from_wiki(self): """Retrieve macros from the wiki @@ -127,15 +160,17 @@ def retrieve_macros_from_wiki(self): p = requests.get(WIKI_MACROS_URL, headers=headers, timeout=10.0) except requests.exceptions.RequestException as e: message = f"Failed to fetch {WIKI_MACROS_URL}: {e}" - self.macro_errors["retrieve_macros_from_wiki"] = message + self.log_error("**INTERNAL**", message) return if not p.status_code == 200: - print(f"Failed to fetch {WIKI_MACROS_URL}, response code was {p.status_code}") + message = f"Failed to fetch {WIKI_MACROS_URL}, response code was {p.status_code}" + self.log_error("**INTERNAL**", message) return macros = re.findall(r'title="(Macro.*?)"', p.text) macros = [mac for mac in macros if "translated" not in mac] - for _, wiki_page_name in enumerate(macros): + for number, wiki_page_name in enumerate(macros): + print(f"{number+1}/{len(macros)}: {wiki_page_name}") macro_name = wiki_page_name[6:] # Remove "Macro ". macro_name = macro_name.replace("&", "&") if not macro_name: @@ -148,7 +183,7 @@ def add_wiki_macro_to_cache(self, macro_name): macro = Macro(macro_name) if macro.name in self.macros: self.macro_stats["duplicated_macros"] += 1 - print(f"Ignoring duplicate of '{macro.name}' (using git repo copy instead of wiki)") + self.log_error(macro.name, "Using git repo copy instead of duplicate found on wiki") return macro.on_wiki = True macro.parsed = False @@ -157,10 +192,15 @@ def add_wiki_macro_to_cache(self, macro_name): wiki_page_name = wiki_page_name.replace("&", "%26") wiki_page_name = wiki_page_name.replace("+", "%2B") url = "https://wiki.freecad.org/Macro_" + wiki_page_name + self.log_buffer.clear() macro.fill_details_from_wiki(url) + for log_entry in self.log_buffer: + level = log_entry.get("level", logging.INFO) + if level >= logging.WARNING: + self.log_error(macro.name, log_entry["msg"]) + self.log_buffer.clear() - @staticmethod - def get_icon(macro: Macro): + def get_icon(self, macro: Macro): """Downloads the macro's icon from whatever source is specified and stores its binary contents in self.icon_data""" if macro.icon.startswith("http://") or macro.icon.startswith("https://"): @@ -171,26 +211,31 @@ def get_icon(macro: Macro): p = requests.get(macro.icon, headers=headers, timeout=10.0) except requests.exceptions.RequestException as e: message = f"Failed to get data from icon URL {macro.icon}: {e}" - self.macro_errors[macro.name] = message + self.log_error(macro.name, message) macro.icon = "" return if p.status_code == 200: _, _, filename = parsed_url.path.rpartition("/") base, _, extension = filename.rpartition(".") if base.lower().startswith("file:"): - print( - f"Cannot use specified icon for {macro.name}, {macro.icon} " - "is not a direct download link" - ) + message = f"Cannot use specified icon for {macro.name}, {macro.icon} is not a direct download link" + self.log_error(macro.name, message) macro.icon = "" return macro.icon_data = p.content macro.icon_extension = extension + + if icon_utils.png_has_duplicate_iccp(macro.icon_data): + message = f"MACRO DEVELOPER WARNING: multiple iCCP chunks found in PNG icon for {macro.name}" + self.log_error(macro.name, message) + macro.icon_data = None + macro.icon = "" else: - print( + message = ( f"MACRO DEVELOPER WARNING: failed to download icon from {macro.icon}" - f" for macro {macro.name}. Status code returned: {p.status_code}\n" + + f" for macro {macro.name}. Status code returned: {p.status_code}\n" ) + self.log_error(macro.name, message) macro.icon = "" elif macro.on_git: relative_path_to_macro_directory = os.path.dirname(macro.src_filename) @@ -206,8 +251,75 @@ def get_icon(macro: Macro): macro.icon_data = icon_file.read() macro.icon_extension = relative_path_to_icon.rpartition(".")[-1] + class StderrAsError(io.StringIO): + def write(self, s): + raise RuntimeError(f"Function wrote to stderr: {s!r}") + + # Do some tests on the icon data to make sure it's valid + throw_on_write = StderrAsError() + if macro.icon and not macro.icon_data: + self.log_error(macro.name, "There is no data for the icon") + elif macro.icon.lower().endswith(".svg"): + try: + if not icon_utils.is_svg_bytes(macro.icon_data): + self.log_error(macro.name, "SVG file does not have valid XML header") + except icon_utils.BadIconData as e: + self.log_error(macro.name, str(e)) + elif macro.icon: + try: + with contextlib.redirect_stderr(throw_on_write): + test_icon = icon_utils.icon_from_bytes(macro.icon_data) + if test_icon.isNull(): + self.log_error(macro.name, "Icon data is invalid") + except (icon_utils.BadIconData, RuntimeError) as e: + self.log_error(macro.name, str(e)) + + def log_error(self, macro_name: str, error_message: str): + if macro_name not in self.macro_errors: + self.macro_errors[macro_name] = [] + self.macro_errors[macro_name].append(error_message) + + +class DequeHandler(logging.Handler): + def __init__(self, store: deque, level=logging.NOTSET): + super().__init__(level) + self.store = store + + def emit(self, record: logging.LogRecord) -> None: + self.store.append( + { + "name": record.name, + "level": record.levelno, + "msg": record.getMessage(), + "time": record.created, + "pathname": record.pathname, + "lineno": record.lineno, + "funcName": record.funcName, + } + ) + + +@contextlib.contextmanager +def capture_console_output(logger: logging.Logger, *, handlers, level=None, propagate=None): + old_handlers = list(logger.handlers) + old_level = logger.level + old_propagate = logger.propagate + + logger.handlers = list(handlers) + try: + if level is not None: + logger.setLevel(level) + if propagate is not None: + logger.propagate = propagate + yield + finally: + logger.handlers = old_handlers + logger.setLevel(old_level) + logger.propagate = old_propagate + if __name__ == "__main__": + app = QtGui.QGuiApplication(sys.argv) catalog = MacroCatalog() catalog.fetch_macros() cache = catalog.create_cache() @@ -231,3 +343,4 @@ def get_icon(macro: Macro): json.dump(catalog.macro_stats, f, indent=" ") print("Cache written to macro_cache.zip and macro_cache.zip.sha256") + app.quit() diff --git a/addonmanager_freecad_interface.py b/addonmanager_freecad_interface.py index 355d3878..4ecd5a12 100644 --- a/addonmanager_freecad_interface.py +++ b/addonmanager_freecad_interface.py @@ -107,6 +107,8 @@ def get_python_exe(): getUserMacroDir = None loadUi = None + logger = logging.getLogger("addonmanager") + try: from PySide6 import QtCore, QtWidgets @@ -154,19 +156,19 @@ class ConsoleReplacement: @staticmethod def PrintLog(arg: str) -> None: - logging.log(logging.DEBUG, arg) + logger.log(logging.DEBUG, arg) @staticmethod def PrintMessage(arg: str) -> None: - logging.info(arg) + logger.info(arg) @staticmethod def PrintWarning(arg: str) -> None: - logging.warning(arg) + logger.warning(arg) @staticmethod def PrintError(arg: str) -> None: - logging.error(arg) + logger.error(arg) Console = ConsoleReplacement() diff --git a/addonmanager_icon_utilities.py b/addonmanager_icon_utilities.py index 7d1d7fdb..1e6bc7b4 100644 --- a/addonmanager_icon_utilities.py +++ b/addonmanager_icon_utilities.py @@ -21,7 +21,8 @@ import re import os -from typing import Optional +import struct +from typing import Optional, List from PySideWrapper import QtCore, QtGui, QtSvg @@ -138,6 +139,33 @@ def scalable_icon_from_svg_bytes(svg_bytes: bytes) -> QtGui.QIcon: cached_default_icons = {} +def get_png_chunk_types(data) -> List[bytes] | None: + PNG_SIG = b"\x89PNG\r\n\x1a\n" + if not data.startswith(PNG_SIG): + return None # not a PNG + i = 8 + out = [] + # PNG structure: [len:4][type:4][data:len][crc:4] repeating + while i + 12 <= len(data): + (length,) = struct.unpack(">I", data[i : i + 4]) + chunk_total = 8 + length + 4 # length+type + data + crc + if i + chunk_total > len(data): + return None + ctype = data[i + 4 : i + 8] + out.append(ctype) + i += chunk_total + if ctype == b"IEND": + break + return out + + +def png_has_duplicate_iccp(data) -> bool: + """Returns True if the PNG contains multiple ICCP chunks. Returns false if the iCCP is OK or + if this is not a PNG at all.""" + chunk_types = get_png_chunk_types(data) + return chunk_types is not None and chunk_types.count(b"iCCP") > 1 + + def get_icon_for_addon(addon: Addon, update: bool = False) -> QtGui.QIcon: """Returns an icon for an Addon. :param addon: The addon to get an icon for. diff --git a/addonmanager_macro_parser.py b/addonmanager_macro_parser.py index f3ae096a..ed3eca91 100644 --- a/addonmanager_macro_parser.py +++ b/addonmanager_macro_parser.py @@ -203,17 +203,23 @@ def _cleanup_comment(self): self.parse_results["comment"] = self.parse_results["comment"][:511] + "…" def _cleanup_license(self): + if len(self.parse_results["license"].strip()) == 0: + fci.Console.PrintWarning( + f"No license specified for macro {self.name} -- assuming 'All rights reserved'.\n" + ) + self.parse_results["license"] = "All rights reserved" + return if get_license_manager is not None: lm = get_license_manager() normed_license = lm.normalize(self.parse_results["license"]) if not normed_license: fci.Console.PrintWarning( - f"Failed to normalize license {self.parse_results['license']} for macro '{self.name}'\n" + f"Failed to normalize license '{self.parse_results['license']}' for macro '{self.name}'\n" ) return elif normed_license != self.parse_results["license"]: fci.Console.PrintMessage( - f"Normalized license {self.parse_results['license']} for macro '{self.name}' to {normed_license}\n" + f"Normalized license '{self.parse_results['license']}' for macro '{self.name}' to {normed_license}\n" ) self.parse_results["license"] = normed_license diff --git a/package.xml b/package.xml index 6176978c..0b2ff4c9 100644 --- a/package.xml +++ b/package.xml @@ -5,8 +5,8 @@ Addon Manager Tool to install workbenches, macros, themes, etc. Resources/icons/addon_manager.svg - 2026.4.25 - 2026-04-25 + 2026.5.13 + 2026-05-13 Chris Hennes Yorik van Havre Jonathan Wiedemann