Updater class now used for all updates checking

* Updater.__init__() now takes 'provider' argument to specify if we use
 the internal checking code, or the available external code.
* EDMC.py changed to utilise this with internal provider.
* EDMarketConnector.py changed to use internal provider if not frozen,
 else the internal provider.
* Corrected the darwin/MacOS toggling of auto updates checking to actually
 use the Sparkle, not WinSparkle, API call.
* Updater.check_appcast() does the internal checking:
    * class EDMCVersion to hold the information.
    * Returns None on any error, or if it didn't find a newer version.
    * Returns an EDMCVersion object if it found a newer version.

EDMC.py

@@ -12,11 +12,9 @@ import os
 # workaround for https://github.com/EDCD/EDMarketConnector/issues/568
 os.environ["EDMC_NO_UI"] = "1"
 
-from os.path import dirname, getmtime, join
+from os.path import getmtime, join
 from time import time, sleep
-from xml.etree import ElementTree
 import re
-import semantic_version
 
 import l10n
 l10n.Translations.install_dummy()
@@ -30,7 +28,8 @@ import loadout
 import edshipyard
 import shipyard
 import stats
-from config import appcmdname, appversion, update_feed, config
+from config import appcmdname, appversion, config
+from update import Updater, EDMCVersion
 from monitor import monitor
 
 sys.path.append(config.internal_plugin_dir)
@@ -63,41 +62,12 @@ try:
     args = parser.parse_args()
 
     if args.version:
-        newversion = None
+        updater = Updater(provider='internal')
-        try:
+        newversion: EDMCVersion = updater.check_appcast()
-            r = requests.get(update_feed, timeout = 10)
-        except requests.RequestException as ex:
-            sys.stderr.write('Error retrieving update_feed file: {}\n'.format(str(ex)))
-        else:
-            try:
-                feed = ElementTree.fromstring(r.text)
-            except SyntaxError as ex:
-                sys.stderr.write('Syntax error in update_feed file: {}\n'.format(str(ex)))
-            else:
-                # Want to make items['<version>'] = {'os': '...', 'title': '...' }
-                items = dict(
-                    [
-                        (
-                            item.find('enclosure').attrib.get('{http://www.andymatuschak.org/xml-namespaces/sparkle}version'),
-                            {'title': item.find('title').text,
-                             'os': item.find('enclosure').attrib.get('{http://www.andymatuschak.org/xml-namespaces/sparkle}os'),
-                            }
-                        ) for item in feed.findall('channel/item')
-                    ]
-                )
-
-                # Filter on the sparkle:os attribute
-                os_map = {'darwin': 'macos', 'win32': 'windows', 'linux': 'linux'}  # Map sys.platform to sparkle:os
-                items = { k:v for (k,v) in items.items() if v['os'] == os_map[sys.platform]}
-
-                # Look for any remaining version greater than appversion
-                simple_spec = semantic_version.SimpleSpec('>' + appversion)
-                newversion = simple_spec.select([semantic_version.Version.coerce(x) for x in items])
-
         if newversion:
             print('{CURRENT} ("{UPDATE}" is available)'.format(
                 CURRENT=appversion,
-                UPDATE=items[str(newversion)]['title']))
+                UPDATE=newversion.title))
         else:
             print(appversion)
         sys.exit(EXIT_SUCCESS)
@@ -121,7 +91,7 @@ try:
                         if __debug__:
                             print('Invalid journal entry "%s"' % repr(line))
         except Exception as e:
-            sys.stderr.write("Can't read Journal file: {}\n".format(str(e)))
+            print("Can't read Journal file: {}\n".format(str(e)), file=sys.stderr)
             sys.exit(EXIT_SYS_ERR)
 
         if not monitor.cmdr:

EDMarketConnector.py

@@ -283,8 +283,11 @@ class AppWindow(object):
 
         # Load updater after UI creation (for WinSparkle)
         import update
-        self.updater = update.Updater(self.w)
+        if getattr(sys, 'frozen', False):
-        if not getattr(sys, 'frozen', False):
+            # Running in frozen .exe, so use WinSparkle
+            self.updater = update.Updater(tkroot=self.w, provider='external')
+        else:
+            self.updater = update.Updater(tkroot=self.w, provider='internal')
             self.updater.checkForUpdates()  # Sparkle / WinSparkle does this automatically for packaged apps
 
         try:

update.py

@@ -4,120 +4,69 @@ import sys
 import threading
 from traceback import print_exc
 import semantic_version
+from typing import Optional
+import tkinter as tk
 
 # ensure registry is set up on Windows before we start
-from config import appname, appversion, appversion_nobuild, update_feed, update_interval, config
+from config import appname, appversion, appversion_nobuild, update_feed
 
+class EDMCVersion(object):
+    """
+    Hold all the information about an EDMC version.
 
-if not getattr(sys, 'frozen', False):
+    Attributes
-    # Running from source
+    ----------
+    version : str
+        Full version string
+    title: str
+        Title of the release
+    sv: semantic_version.base.Version
+        semantic_version object for this version
+    """
+    def __init__(self, version: str, title: str, sv: semantic_version.base.Version):
+        self.version: str = version
+        self.title: str = title
+        self.sv: semantic_version.base.Version = sv
 
-    #TODO: Update this to use Semantic Version as per EDMC.py args.version check
+class Updater(object):
-    class Updater(object):
+    """
+    Updater class to handle checking for updates, whether using internal code
+    or an external library such as WinSparkle on win32.
+    """
 
-        def __init__(self, master):
+    def shutdown_request(self) -> None:
-            self.root = master
+        """
+        Receive (Win)Sparkle shutdown request and send it to parent.
+        :rtype: None
+        """
+        self.root.event_generate('<<Quit>>', when="tail")
 
-        def setAutomaticUpdatesCheck(self, onoroff):
+    def use_internal(self) -> bool:
+        """
+        :return: if internal update checks should be used.
+        :rtype: bool
+        """
+        if self.provider == 'internal':
+            return True
+
+        return False
+
+    def __init__(self, tkroot: tk.Tk=None, provider: str='internal'):
+        """
+        :param tkroot: reference to the root window of the GUI
+        :param provider: 'internal' or other string if not
+        """
+        self.root: tk.Tk = tkroot
+        self.provider: str = provider
+        self.thread: threading.Thread = None
+
+        if self.use_internal():
                 return
 
-        def checkForUpdates(self):
+        if sys.platform == 'win32':
-            thread = threading.Thread(target = self.worker, name = 'update worker')
-            thread.daemon = True
-            thread.start()
-
-        def check_appcast(self) -> dict:
-            import requests
-            from xml.etree import ElementTree
-
-            newversion = None
-            try:
-                r = requests.get(update_feed, timeout=10)
-            except requests.RequestException as ex:
-                sys.stderr.write('Error retrieving update_feed file: {}\n'.format(str(ex)))
-            else:
-                try:
-                    feed = ElementTree.fromstring(r.text)
-                except SyntaxError as ex:
-                    sys.stderr.write('Syntax error in update_feed file: {}\n'.format(str(ex)))
-                else:
-
-                    items = dict()
-                    for item in feed.findall('channel/item'):
-                        ver = item.find('enclosure').attrib.get('{http://www.andymatuschak.org/xml-namespaces/sparkle}version')
-                        sv = semantic_version.Version.coerce(ver)
-
-                        os = item.find('enclosure').attrib.get('{http://www.andymatuschak.org/xml-namespaces/sparkle}os')
-                        os_map = {'darwin': 'macos', 'win32': 'windows', 'linux' : 'linux'}  # Map sys.platform to sparkle:os
-                        if os == os_map[sys.platform]:
-                            items[sv] = {
-                                'version': ver,
-                                'title': item.find('title').text,
-                            }
-
-                    # Look for any remaining version greater than appversion
-                    simple_spec = semantic_version.SimpleSpec('>' + appversion)
-                    newversion = simple_spec.select(items.keys())
-
-            return items[newversion]
-
-        def worker(self):
-
-            newversion = self.check_appcast()
-
-            if newversion:
-                self.root.nametowidget('.{}.status'.format(appname.lower()))['text'] = newversion['title'] + ' is available'
-                self.root.update_idletasks()
-
-        def close(self):
-            pass
-
-elif sys.platform=='darwin':
-
-    import objc
-
-    class Updater(object):
-
-        # http://sparkle-project.org/documentation/customization/
-
-        def __init__(self, master):
-            try:
-                objc.loadBundle('Sparkle', globals(), join(dirname(sys.executable), os.pardir, 'Frameworks', 'Sparkle.framework'))
-                self.updater = SUUpdater.sharedUpdater()
-            except:
-                # can't load framework - not frozen or not included in app bundle?
-                print_exc()
-                self.updater = None
-
-        def setAutomaticUpdatesCheck(self, onoroff):
-            if self.updater:
-                self.updater.win_sparkle_set_automatic_check_for_updates(onoroff)
-
-        def checkForUpdates(self):
-            if self.updater:
-                self.updater.checkForUpdates_(None)
-
-        def close(self):
-            self.updater = None
-
-
-elif sys.platform=='win32':
-
             import ctypes
 
-    # https://github.com/vslavik/winsparkle/blob/master/include/winsparkle.h#L272
-    root = None
-
-    def shutdown_request():
-        root.event_generate('<<Quit>>', when="tail")
-
-    class Updater(object):
-
-        # https://github.com/vslavik/winsparkle/wiki/Basic-Setup
-
-        def __init__(self, master):
             try:
-                sys.frozen	# don't want to try updating python.exe
                 self.updater = ctypes.cdll.WinSparkle
 
                 # Set the appcast URL
@@ -132,9 +81,9 @@ elif sys.platform=='win32':
 
                 # set up shutdown callback
                 global root
-                root = master
+                root = tkroot
                 self.callback_t = ctypes.CFUNCTYPE(None)  # keep reference
-                self.callback_fn = self.callback_t(shutdown_request)
+                self.callback_fn = self.callback_t(self.shutdown_request)
                 self.updater.win_sparkle_set_shutdown_request_callback(self.callback_fn)
 
                 # Get WinSparkle running
@@ -144,15 +93,115 @@ elif sys.platform=='win32':
                 print_exc()
                 self.updater = None
 
-        def setAutomaticUpdatesCheck(self, onoroff):
+            return
-            if self.updater:
+
+        if sys.platform == 'darwin':
+            import objc
+            try:
+                objc.loadBundle('Sparkle', globals(), join(dirname(sys.executable), os.pardir, 'Frameworks', 'Sparkle.framework'))
+                self.updater = SUUpdater.sharedUpdater()
+            except:
+                # can't load framework - not frozen or not included in app bundle?
+                print_exc()
+                self.updater = None
+
+    def setAutomaticUpdatesCheck(self, onoroff: bool) -> None:
+        """
+        Helper to set (Win)Sparkle to perform automatic update checks, or not.
+        :param onoroff: bool for if we should have the library check or not.
+        :return: None
+        """
+        if self.use_internal():
+            return
+
+        if sys.platform == 'win32' and self.updater:
             self.updater.win_sparkle_set_automatic_check_for_updates(onoroff)
 
-        def checkForUpdates(self):
+        if sys.platform == 'darwin' and self.updater:
-            if self.updater:
+            self.updater.SUEnableAutomaticChecks(onoroff)
+
+    def checkForUpdates(self) -> None:
+        """
+        Trigger the requisite method to check for an update.
+        :return: None
+        """
+        if self.use_internal():
+            self.thread = threading.Thread(target = self.worker, name = 'update worker')
+            self.thread.daemon = True
+            self.thread.start()
+
+        elif sys.platform == 'win32' and self.updater:
             self.updater.win_sparkle_check_update_with_ui()
 
-        def close(self):
+        elif sys.platform == 'darwin' and self.updater:
-            if self.updater:
+            self.updater.checkForUpdates_(None)
-                self.updater.win_sparkle_cleanup()
+
-            self.updater = None
+    def check_appcast(self) -> Optional[EDMCVersion]:
+        """
+        Manually (no Sparkle or WinSparkle) check the update_feed appcast file
+        to see if any listed version is semantically greater than the current
+        running version.
+        :return: EDMCVersion or None if no newer version found
+        """
+        import requests
+        from xml.etree import ElementTree
+
+        newversion = None
+        items = {}
+        try:
+            r = requests.get(update_feed, timeout=10)
+        except requests.RequestException as ex:
+            print('Error retrieving update_feed file: {}'.format(str(ex)), file=sys.stderr)
+        else:
+            try:
+                feed = ElementTree.fromstring(r.text)
+            except SyntaxError as ex:
+                print('Syntax error in update_feed file: {}'.format(str(ex)), file=sys.stderr)
+            else:
+
+                for item in feed.findall('channel/item'):
+                    ver = item.find('enclosure').attrib.get('{http://www.andymatuschak.org/xml-namespaces/sparkle}version')
+                    # This will change A.B.C.D to A.B.C+D
+                    sv = semantic_version.Version.coerce(ver)
+
+                    items[sv] = EDMCVersion(version=ver, # sv might have mangled version
+                                            title=item.find('title').text,
+                                            sv=sv
+                    )
+
+                # Look for any remaining version greater than appversion
+                simple_spec = semantic_version.SimpleSpec('>' + appversion)
+                newversion = simple_spec.select(items.keys())
+
+        if newversion:
+            return items[newversion]
+        return None
+
+    def worker(self) -> None:
+        """
+        Thread worker to perform internal update checking and update GUI
+        status if a newer version is found.
+        :return: None
+        """
+        newversion = self.check_appcast()
+
+        if newversion:
+            # TODO: Surely we can do better than this
+            #       nametowidget('.{}.status'.format(appname.lower()))['text']
+            self.root.nametowidget('.{}.status'.format(appname.lower()))['text'] = newversion.title + ' is available'
+            self.root.update_idletasks()
+
+    def close(self) -> None:
+        """
+        Handles the EDMarketConnector.AppWindow.onexit() request.
+
+        NB: We just 'pass' here because:
+         1) We might have a worker() going, but no way to make that
+            co-operative to respond to a "please stop now" message.
+         2) If we're running frozen then we're using (Win)Sparkle to check
+            and *it* might have asked this whole application to quit, in
+            which case we don't want to ask *it* to quit
+
+        :return: None
+        """
+        pass