From b446f2012def2dc10a5c3f43af162d538ac37df9 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Fri, 9 Feb 2024 19:09:59 -0500 Subject: [PATCH] Addon Manager: Fixes for license handling --- src/Mod/AddonManager/Addon.py | 2 + src/Mod/AddonManager/NetworkManager.py | 43 +++++++++++---- .../AddonManager/Resources/AddonManager.qrc | 10 ++-- .../licenses/{CC0v1.txt => CC0-1.0.txt} | 0 .../{GPLv2.txt => GPL-2.0-or-later.txt} | 0 .../{GPLv3.txt => GPL-3.0-or-later.txt} | 0 .../{LGPLv2.1.txt => LGPL-2.1-or-later.txt} | 0 .../{LGPLv3.txt => LGPL-3.0-or-later.txt} | 0 .../addonmanager_devmode_license_selector.py | 12 ++--- src/Mod/AddonManager/addonmanager_licenses.py | 53 ++++++++++++++++++- src/Mod/AddonManager/addonmanager_macro.py | 14 +++-- .../AddonManager/addonmanager_macro_parser.py | 12 +++++ src/Mod/AddonManager/addonmanager_metadata.py | 8 ++- .../addonmanager_workers_utility.py | 4 +- src/Mod/AddonManager/package_list.py | 39 ++++++++++++-- 15 files changed, 162 insertions(+), 35 deletions(-) rename src/Mod/AddonManager/Resources/licenses/{CC0v1.txt => CC0-1.0.txt} (100%) rename src/Mod/AddonManager/Resources/licenses/{GPLv2.txt => GPL-2.0-or-later.txt} (100%) rename src/Mod/AddonManager/Resources/licenses/{GPLv3.txt => GPL-3.0-or-later.txt} (100%) rename src/Mod/AddonManager/Resources/licenses/{LGPLv2.1.txt => LGPL-2.1-or-later.txt} (100%) rename src/Mod/AddonManager/Resources/licenses/{LGPLv3.txt => LGPL-3.0-or-later.txt} (100%) diff --git a/src/Mod/AddonManager/Addon.py b/src/Mod/AddonManager/Addon.py index 59a360b6e3..9af58937dd 100644 --- a/src/Mod/AddonManager/Addon.py +++ b/src/Mod/AddonManager/Addon.py @@ -227,6 +227,8 @@ class Addon: self._cached_license = self.metadata.license elif self.stats and self.stats.license: self._cached_license = self.stats.license + elif self.macro and self.macro.license: + self._cached_license = self.macro.license return self._cached_license @classmethod diff --git a/src/Mod/AddonManager/NetworkManager.py b/src/Mod/AddonManager/NetworkManager.py index d3481187f8..98a1be16be 100644 --- a/src/Mod/AddonManager/NetworkManager.py +++ b/src/Mod/AddonManager/NetworkManager.py @@ -315,7 +315,11 @@ if HAVE_QTNETWORK: reply.readyRead.connect(self.__ready_to_read) reply.downloadProgress.connect(self.__download_progress) - def submit_unmonitored_get(self, url: str) -> int: + def submit_unmonitored_get( + self, + url: str, + timeout_ms: int = QtNetwork.QNetworkRequest.DefaultTransferTimeoutConstant, + ) -> int: """Adds this request to the queue, and returns an index that can be used by calling code in conjunction with the completed() signal to handle the results of the call. All data is kept in memory, and the completed() call includes a direct handle to the bytes returned. It @@ -324,12 +328,18 @@ if HAVE_QTNETWORK: current_index = next(self.counting_iterator) # A thread-safe counter # Use a queue because we can only put things on the QNAM from the main event loop thread self.queue.put( - QueueItem(current_index, self.__create_get_request(url), track_progress=False) + QueueItem( + current_index, self.__create_get_request(url, timeout_ms), track_progress=False + ) ) self.__request_queued.emit() return current_index - def submit_monitored_get(self, url: str) -> int: + def submit_monitored_get( + self, + url: str, + timeout_ms: int = QtNetwork.QNetworkRequest.DefaultTransferTimeoutConstant, + ) -> int: """Adds this request to the queue, and returns an index that can be used by calling code in conjunction with the progress_made() and progress_completed() signals to handle the results of the call. All data is cached to disk, and progress is reported periodically @@ -340,12 +350,18 @@ if HAVE_QTNETWORK: current_index = next(self.counting_iterator) # A thread-safe counter # Use a queue because we can only put things on the QNAM from the main event loop thread self.queue.put( - QueueItem(current_index, self.__create_get_request(url), track_progress=True) + QueueItem( + current_index, self.__create_get_request(url, timeout_ms), track_progress=True + ) ) self.__request_queued.emit() return current_index - def blocking_get(self, url: str) -> Optional[QtCore.QByteArray]: + def blocking_get( + self, + url: str, + timeout_ms: int = QtNetwork.QNetworkRequest.DefaultTransferTimeoutConstant, + ) -> Optional[QtCore.QByteArray]: """Submits a GET request to the QNetworkAccessManager and block until it is complete""" current_index = next(self.counting_iterator) # A thread-safe counter @@ -353,7 +369,9 @@ if HAVE_QTNETWORK: self.synchronous_complete[current_index] = False self.queue.put( - QueueItem(current_index, self.__create_get_request(url), track_progress=False) + QueueItem( + current_index, self.__create_get_request(url, timeout_ms), track_progress=False + ) ) self.__request_queued.emit() while True: @@ -388,7 +406,7 @@ if HAVE_QTNETWORK: ) self.synchronous_complete[index] = True - def __create_get_request(self, url: str) -> QtNetwork.QNetworkRequest: + def __create_get_request(self, url: str, timeout_ms: int) -> QtNetwork.QNetworkRequest: """Construct a network request to a given URL""" request = QtNetwork.QNetworkRequest(QtCore.QUrl(url)) request.setAttribute( @@ -400,6 +418,7 @@ if HAVE_QTNETWORK: QtNetwork.QNetworkRequest.CacheLoadControlAttribute, QtNetwork.QNetworkRequest.PreferNetwork, ) + request.setTransferTimeout(timeout_ms) return request def abort_all(self): @@ -428,7 +447,8 @@ if HAVE_QTNETWORK: authenticator: QtNetwork.QAuthenticator, ): """If proxy authentication is required, attempt to authenticate. If the GUI is running this displays - a window asking for credentials. If the GUI is not running, it prompts on the command line.""" + a window asking for credentials. If the GUI is not running, it prompts on the command line. + """ if HAVE_FREECAD and FreeCAD.GuiUp: proxy_authentication = FreeCADGui.PySideUic.loadUi( os.path.join(os.path.dirname(__file__), "proxy_authentication.ui") @@ -463,6 +483,9 @@ if HAVE_QTNETWORK: def __follow_redirect(self, url): """Used with the QNetworkAccessManager to follow redirects.""" sender = self.sender() + current_index = -1 + timeout_ms = QtNetwork.QNetworkRequest.DefaultTransferTimeoutConstant + # TODO: Figure out what the actual timeout value should be from the original request if sender: for index, reply in self.replies.items(): if reply == sender: @@ -470,7 +493,8 @@ if HAVE_QTNETWORK: break sender.abort() - self.__launch_request(current_index, self.__create_get_request(url)) + if current_index != -1: + self.__launch_request(current_index, self.__create_get_request(url, timeout_ms)) def __on_ssl_error(self, reply: str, errors: List[str] = None): """Called when an SSL error occurs: prints the error information.""" @@ -620,7 +644,6 @@ def InitializeNetworkManager(): if __name__ == "__main__": - app = QtCore.QCoreApplication() InitializeNetworkManager() diff --git a/src/Mod/AddonManager/Resources/AddonManager.qrc b/src/Mod/AddonManager/Resources/AddonManager.qrc index a1f12a12d8..a2a0b75b85 100644 --- a/src/Mod/AddonManager/Resources/AddonManager.qrc +++ b/src/Mod/AddonManager/Resources/AddonManager.qrc @@ -69,11 +69,11 @@ licenses/Apache-2.0.txt licenses/BSD-2-Clause.txt licenses/BSD-3-Clause.txt - licenses/CC0v1.txt - licenses/GPLv2.txt - licenses/GPLv3.txt - licenses/LGPLv2.1.txt - licenses/LGPLv3.txt + licenses/CC0-1.0.txt + licenses/GPL-2.0-or-later.txt + licenses/GPL-3.0-or-later.txt + licenses/LGPL-2.1-or-later.txt + licenses/LGPL-3.0-or-later.txt licenses/MIT.txt licenses/MPL-2.0.txt licenses/spdx.json diff --git a/src/Mod/AddonManager/Resources/licenses/CC0v1.txt b/src/Mod/AddonManager/Resources/licenses/CC0-1.0.txt similarity index 100% rename from src/Mod/AddonManager/Resources/licenses/CC0v1.txt rename to src/Mod/AddonManager/Resources/licenses/CC0-1.0.txt diff --git a/src/Mod/AddonManager/Resources/licenses/GPLv2.txt b/src/Mod/AddonManager/Resources/licenses/GPL-2.0-or-later.txt similarity index 100% rename from src/Mod/AddonManager/Resources/licenses/GPLv2.txt rename to src/Mod/AddonManager/Resources/licenses/GPL-2.0-or-later.txt diff --git a/src/Mod/AddonManager/Resources/licenses/GPLv3.txt b/src/Mod/AddonManager/Resources/licenses/GPL-3.0-or-later.txt similarity index 100% rename from src/Mod/AddonManager/Resources/licenses/GPLv3.txt rename to src/Mod/AddonManager/Resources/licenses/GPL-3.0-or-later.txt diff --git a/src/Mod/AddonManager/Resources/licenses/LGPLv2.1.txt b/src/Mod/AddonManager/Resources/licenses/LGPL-2.1-or-later.txt similarity index 100% rename from src/Mod/AddonManager/Resources/licenses/LGPLv2.1.txt rename to src/Mod/AddonManager/Resources/licenses/LGPL-2.1-or-later.txt diff --git a/src/Mod/AddonManager/Resources/licenses/LGPLv3.txt b/src/Mod/AddonManager/Resources/licenses/LGPL-3.0-or-later.txt similarity index 100% rename from src/Mod/AddonManager/Resources/licenses/LGPLv3.txt rename to src/Mod/AddonManager/Resources/licenses/LGPL-3.0-or-later.txt diff --git a/src/Mod/AddonManager/addonmanager_devmode_license_selector.py b/src/Mod/AddonManager/addonmanager_devmode_license_selector.py index 3aa6da6973..72db565694 100644 --- a/src/Mod/AddonManager/addonmanager_devmode_license_selector.py +++ b/src/Mod/AddonManager/addonmanager_devmode_license_selector.py @@ -75,23 +75,23 @@ class LicenseSelector: "The 3-Clause BSD License", "https://opensource.org/licenses/BSD-3-Clause", ), - "CC0v1": ( + "CC0-1.0": ( "No Rights Reserved/Public Domain", "https://creativecommons.org/choose/zero/", ), - "GPLv2": ( + "GPL-2.0-or-later": ( "GNU General Public License version 2", "https://opensource.org/licenses/GPL-2.0", ), - "GPLv3": ( + "GPL-3.0-or-later": ( "GNU General Public License version 3", "https://opensource.org/licenses/GPL-3.0", ), - "LGPLv2.1": ( + "LGPL-2.1-or-later": ( "GNU Lesser General Public License version 2.1", "https://opensource.org/licenses/LGPL-2.1", ), - "LGPLv3": ( + "LGPL-3.0-or-later": ( "GNU Lesser General Public License version 3", "https://opensource.org/licenses/LGPL-3.0", ), @@ -129,7 +129,7 @@ class LicenseSelector: self.dialog.createButton.clicked.connect(self._create_clicked) # Set up the first selection to whatever the user chose last time - short_code = self.pref.GetString("devModeLastSelectedLicense", "LGPLv2.1") + short_code = self.pref.GetString("devModeLastSelectedLicense", "LGPL-2.1-or-later") self.set_license(short_code) def exec(self, short_code: str = None, license_path: str = "") -> Optional[Tuple[str, str]]: diff --git a/src/Mod/AddonManager/addonmanager_licenses.py b/src/Mod/AddonManager/addonmanager_licenses.py index 7577ec8891..1dcc40b87a 100644 --- a/src/Mod/AddonManager/addonmanager_licenses.py +++ b/src/Mod/AddonManager/addonmanager_licenses.py @@ -42,6 +42,8 @@ except ImportError: from PySide import QtCore +import addonmanager_freecad_interface as fci + class SPDXLicenseManager: """A class that loads a list of licenses from an internal Qt resource and provides access to @@ -74,15 +76,28 @@ class SPDXLicenseManager: """Check to see if the license is OSI-approved, according to the SPDX database. Returns False if the license is not in the database, or is not marked as "isOsiApproved".""" if spdx_id not in self.license_data: + fci.Console.PrintWarning( + f"WARNING: License ID {spdx_id} is not in the SPDX license " + f"list. The Addon author must correct their metadata.\n" + ) return False - return self.license_data[spdx_id]["isOsiApproved"] + return ( + "isOsiApproved" in self.license_data[spdx_id] + and self.license_data[spdx_id]["isOsiApproved"] + ) def is_fsf_libre(self, spdx_id: str) -> bool: """Check to see if the license is FSF Free/Libre, according to the SPDX database. Returns False if the license is not in the database, or is not marked as "isFsfLibre".""" if spdx_id not in self.license_data: + fci.Console.PrintWarning( + f"WARNING: License ID {spdx_id} is not in the SPDX license " + f"list. The Addon author must correct their metadata.\n" + ) return False - return self.license_data[spdx_id]["isFsfLibre"] + return ( + "isFsfLibre" in self.license_data[spdx_id] and self.license_data[spdx_id]["isFsfLibre"] + ) def name(self, spdx_id: str) -> str: if spdx_id not in self.license_data: @@ -114,6 +129,40 @@ class SPDXLicenseManager: return "" return self.license_data[spdx_id]["detailsUrl"] + def normalize(self, license_string: str) -> str: + """Given a potentially non-compliant license string, attempt to normalize it to match an + SPDX record. Takes a conservative view and tries not to over-expand stated rights (e.g. + it will select 'GPL-3.0-only' rather than 'GPL-3.0-or-later' when given just GPL3).""" + if self.name(license_string): + return license_string + fci.Console.PrintLog( + f"Attempting to normalize non-compliant license '" f"{license_string}'... " + ) + normed = license_string.replace("lgpl", "LGPL").replace("gpl", "GPL") + normed = ( + normed.replace(" ", "-") + .replace("v", "-") + .replace("GPL2", "GPL-2") + .replace("GPL3", "GPL-3") + ) + if self.name(normed): + fci.Console.PrintLog(f"found valid SPDX license ID {normed}\n") + return normed + # If it still doesn't match, try some other things + while "--" in normed: + normed = license_string.replace("--", "-") + + if self.name(normed): + fci.Console.PrintLog(f"found valid SPDX license ID {normed}\n") + return normed + if not normed.endswith(".0"): + normed += ".0" + if self.name(normed): + fci.Console.PrintLog(f"found valid SPDX license ID {normed}\n") + return normed + fci.Console.PrintLog(f"failed to normalize (typo in ID or invalid version number??)\n") + return license_string # We failed to normalize this one + _LICENSE_MANAGER = None # Internal use only, see get_license_manager() diff --git a/src/Mod/AddonManager/addonmanager_macro.py b/src/Mod/AddonManager/addonmanager_macro.py index b01a7fde78..06b981583d 100644 --- a/src/Mod/AddonManager/addonmanager_macro.py +++ b/src/Mod/AddonManager/addonmanager_macro.py @@ -67,6 +67,7 @@ class Macro: self.raw_code_url = "" self.wiki = "" self.version = "" + self.license = "" self.date = "" self.src_filename = "" self.filename_from_url = "" @@ -111,8 +112,8 @@ class Macro: def is_installed(self): """Returns True if this macro is currently installed (that is, if it exists - in the user macro directory), or False if it is not. Both the exact filename, - as well as the filename prefixed with "Macro", are considered an installation + in the user macro directory), or False if it is not. Both the exact filename + and the filename prefixed with "Macro", are considered an installation of this macro. """ if self.on_git and not self.src_filename: @@ -159,6 +160,9 @@ class Macro: code = self._fetch_raw_code(p) if not code: code = self._read_code_from_wiki(p) + if not self.license: + # The default license on the wiki is CC-BY-3.0 (which is non-Libre and not OSI-approved) + self.license = "CC-BY-3.0" if not code: self._console.PrintWarning( translate("AddonsInstaller", "Unable to fetch the code of this macro.") + "\n" @@ -227,7 +231,7 @@ class Macro: code = re.findall(r"
(.*?)
", p.replace("\n", "--endl--")) if code: # take the biggest code block - code = sorted(code, key=len)[-1] + code = str(sorted(code, key=len)[-1]) code = code.replace("--endl--", "\n") # Clean HTML escape codes. code = unescape(code) @@ -327,7 +331,7 @@ class Macro: self.other_files.append(self.icon) def _copy_other_files(self, macro_dir, warnings) -> bool: - """Copy any specified "other files" into the install directory""" + """Copy any specified "other files" into the installation directory""" base_dir = os.path.dirname(self.src_filename) for other_file in self.other_files: if not other_file: @@ -382,7 +386,7 @@ class Macro: ) def parse_wiki_page_for_icon(self, page_data: str) -> None: - """Attempt to find a url for the icon in the wiki page. Sets self.icon if + """Attempt to find the url for the icon in the wiki page. Sets 'self.icon' if found.""" # Method 1: the text "toolbar icon" appears on the page, and provides a direct diff --git a/src/Mod/AddonManager/addonmanager_macro_parser.py b/src/Mod/AddonManager/addonmanager_macro_parser.py index 86d829bbe7..5c77037574 100644 --- a/src/Mod/AddonManager/addonmanager_macro_parser.py +++ b/src/Mod/AddonManager/addonmanager_macro_parser.py @@ -36,8 +36,10 @@ except ImportError: try: import FreeCAD + from addonmanager_licenses import get_license_manager except ImportError: FreeCAD = None + get_license_manager = None class DummyThread: @@ -63,6 +65,7 @@ class MacroParser: "other_files": [""], "author": "", "date": "", + "license": "", "icon": "", "xpm": "", } @@ -83,6 +86,8 @@ class MacroParser: "__files__": "other_files", "__author__": "author", "__date__": "date", + "__license__": "license", + "__licence__": "license", # accept either spelling "__icon__": "icon", "__xpm__": "xpm", } @@ -185,6 +190,8 @@ class MacroParser: self.parse_results[value] = match_group if value == "comment": self._cleanup_comment() + elif value == "license": + self._cleanup_license() elif isinstance(self.parse_results[value], list): self.parse_results[value] = [of.strip() for of in match_group.split(",")] else: @@ -197,6 +204,11 @@ class MacroParser: if len(self.parse_results["comment"]) > 512: self.parse_results["comment"] = self.parse_results["comment"][:511] + "…" + def _cleanup_license(self): + if get_license_manager is not None: + lm = get_license_manager() + self.parse_results["license"] = lm.normalize(self.parse_results["license"]) + def _apply_special_handling(self, key: str, line: str): # Macro authors are supposed to be providing strings here, but in some # cases they are not doing so. If this is the "__version__" tag, try diff --git a/src/Mod/AddonManager/addonmanager_metadata.py b/src/Mod/AddonManager/addonmanager_metadata.py index 213b0e2c5e..1afb733454 100644 --- a/src/Mod/AddonManager/addonmanager_metadata.py +++ b/src/Mod/AddonManager/addonmanager_metadata.py @@ -30,6 +30,9 @@ from dataclasses import dataclass, field from enum import IntEnum, auto from typing import Tuple, Dict, List, Optional +from addonmanager_licenses import get_license_manager +import addonmanager_freecad_interface as fci + try: # If this system provides a secure parser, use that: import defusedxml.ElementTree as ET @@ -315,7 +318,10 @@ class MetadataReader: @staticmethod def _parse_license(child: ET.Element) -> License: file = child.attrib["file"] if "file" in child.attrib else "" - return License(name=child.text, file=file) + license_id = child.text + lm = get_license_manager() + license_id = lm.normalize(license_id) + return License(name=license_id, file=file) @staticmethod def _parse_url(child: ET.Element) -> Url: diff --git a/src/Mod/AddonManager/addonmanager_workers_utility.py b/src/Mod/AddonManager/addonmanager_workers_utility.py index eaa8e5c67e..48b8d360bd 100644 --- a/src/Mod/AddonManager/addonmanager_workers_utility.py +++ b/src/Mod/AddonManager/addonmanager_workers_utility.py @@ -55,7 +55,9 @@ class ConnectionChecker(QtCore.QThread): url = "https://api.github.com/zen" self.done = False NetworkManager.AM_NETWORK_MANAGER.completed.connect(self.connection_data_received) - self.request_id = NetworkManager.AM_NETWORK_MANAGER.submit_unmonitored_get(url) + self.request_id = NetworkManager.AM_NETWORK_MANAGER.submit_unmonitored_get( + url, timeout_ms=10000 + ) while not self.done: if QtCore.QThread.currentThread().isInterruptionRequested(): FreeCAD.Console.PrintLog("Connection check cancelled\n") diff --git a/src/Mod/AddonManager/package_list.py b/src/Mod/AddonManager/package_list.py index 3d9f310346..849aa1d457 100644 --- a/src/Mod/AddonManager/package_list.py +++ b/src/Mod/AddonManager/package_list.py @@ -560,12 +560,41 @@ class PackageListFilter(QtCore.QSortFilterProxyModel): return False # If it is not an OSI-approved license, check to see if we are hiding those - if self.hide_non_OSI_approved and not license_manager.is_osi_approved(data.license): - return False + if self.hide_non_OSI_approved or self.hide_non_FSF_libre: + if not data.license: + return False + licenses_to_check = [] + if type(data.license) is str: + licenses_to_check.append(data.license) + elif type(data.license) is list: + for license_id in data.license: + if type(license_id) is str: + licenses_to_check.append(license_id) + else: + licenses_to_check.append(license_id.name) + else: + licenses_to_check.append(data.license.name) - # If it is not an FSF Free/Libre license, check to see if we are hiding those - if self.hide_non_FSF_libre and not license_manager.is_fsf_libre(data.license): - return False + fsf_libre = False + osi_approved = False + for license_id in licenses_to_check: + if not osi_approved and license_manager.is_osi_approved(license_id): + osi_approved = True + if not fsf_libre and license_manager.is_fsf_libre(license_id): + fsf_libre = True + if self.hide_non_OSI_approved and not osi_approved: + FreeCAD.Console.PrintLog( + f"Hiding addon {data.name} because its license, {licenses_to_check}, " + f"is " + f"not OSI approved\n" + ) + return False + if self.hide_non_FSF_libre and not fsf_libre: + FreeCAD.Console.PrintLog( + f"Hiding addon {data.name} because its license, {licenses_to_check}, is " + f"not FSF Libre\n" + ) + return False # If it's not installed, check to see if it's for a newer version of FreeCAD if (