Addon Manager: Fixes for license handling

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

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()

src/Mod/AddonManager/Resources/AddonManager.qrc

@@ -69,11 +69,11 @@
 		<file>licenses/Apache-2.0.txt</file>
 		<file>licenses/BSD-2-Clause.txt</file>
 		<file>licenses/BSD-3-Clause.txt</file>
-		<file>licenses/CC0v1.txt</file>
-		<file>licenses/GPLv2.txt</file>
-		<file>licenses/GPLv3.txt</file>
-		<file>licenses/LGPLv2.1.txt</file>
-		<file>licenses/LGPLv3.txt</file>
+		<file>licenses/CC0-1.0.txt</file>
+		<file>licenses/GPL-2.0-or-later.txt</file>
+		<file>licenses/GPL-3.0-or-later.txt</file>
+		<file>licenses/LGPL-2.1-or-later.txt</file>
+		<file>licenses/LGPL-3.0-or-later.txt</file>
 		<file>licenses/MIT.txt</file>
 		<file>licenses/MPL-2.0.txt</file>
         <file>licenses/spdx.json</file>

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]]:

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()
 

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"<pre>(.*?)</pre>", 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

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

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:

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")

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 (