Merge branch 'checkupdates_exit_code' into 'master'

[checkupdates] Cleanup code

See merge request fdroid/fdroidserver!986

fdroidserver/checkupdates.py

@@ -45,11 +45,6 @@ from .exception import VCSException, NoSubmodulesException, FDroidException, Met
 # required.
 def check_http(app):
 
-    ignoreversions = app.UpdateCheckIgnore
-    ignoresearch = re.compile(ignoreversions).search if ignoreversions else None
-
-    try:
-
     if not app.UpdateCheckData:
         raise FDroidException('Missing Update Check Data')
 
@@ -62,8 +57,6 @@ def check_http(app):
         if not parsed.netloc or not parsed.scheme or parsed.scheme != 'https':
             raise FDroidException(_('UpdateCheckData has invalid URL: {url}').format(url=urlcode))
 
-        vercode = None
-        if urlcode:
     logging.debug("...requesting {0}".format(urlcode))
     req = urllib.request.Request(urlcode, None, headers=net.HEADERS)
     resp = urllib.request.urlopen(req, None, 20)  # nosec B310 scheme is filtered above
@@ -74,8 +67,6 @@ def check_http(app):
         raise FDroidException("No RE match for version code")
     vercode = m.group(1).strip()
 
-        version = "??"
-        if urlver:
     if urlver != '.':
         logging.debug("...requesting {0}".format(urlver))
         req = urllib.request.Request(urlver, None)
@@ -87,28 +78,20 @@ def check_http(app):
         raise FDroidException("No RE match for version")
     version = m.group(1)
 
-        if ignoresearch and version:
+    if app.UpdateCheckIgnore and re.search(app.UpdateCheckIgnore, version):
-            if not ignoresearch(version):
+        logging.info("Version {version} for {appid} is ignored".format(version=version, appid=app.id))
+        return (None, None)
+
     return (version, vercode)
-            else:
-                return (None, ("Version {version} is ignored").format(version=version))
-        else:
-            return (version, vercode)
-    except FDroidException:
-        msg = "Could not complete http check for app {0} due to unknown error: {1}".format(app.id, traceback.format_exc())
-        return (None, msg)
 
 
-# Check for a new version by looking at the tags in the source repo.
-# Whether this can be used reliably or not depends on
-# the development procedures used by the project's developers. Use it with
-# caution, because it's inappropriate for many projects.
-# Returns (None, "a message") if this didn't work, or (version, vercode, tag) for
-# the details of the current version.
 def check_tags(app, pattern):
+    """Check for a new version by looking at the tags in the source repo.
 
-    try:
+    Whether this can be used reliably or not depends on
-
+    the development procedures used by the project's developers. Use it with
+    caution, because it's inappropriate for many projects.
+    """
     if app.RepoType == 'srclib':
         build_dir = Path('build/srclib') / app.Repo
         repotype = common.getsrclibvcs(app.Repo)
@@ -117,10 +100,10 @@ def check_tags(app, pattern):
         repotype = app.RepoType
 
     if repotype not in ('git', 'git-svn', 'hg', 'bzr'):
-            return (None, 'Tags update mode only works for git, hg, bzr and git-svn repositories currently', None)
+        raise MetaDataException(_('Tags update mode only works for git, hg, bzr and git-svn repositories currently'))
 
     if repotype == 'git-svn' and ';' not in app.Repo:
-            return (None, 'Tags update mode used in git-svn, but the repo was not set up with tags', None)
+        raise MetaDataException(_('Tags update mode used in git-svn, but the repo was not set up with tags'))
 
     # Set up vcs interface and make sure we have the latest code...
     vcs = common.getvcs(app.RepoType, app.Repo, build_dir)
@@ -141,14 +124,14 @@ def check_tags(app, pattern):
     else:
         tags = vcs.gettags()
     if not tags:
-            return (None, "No tags found", None)
+        raise FDroidException(_('No tags found'))
 
     logging.debug("All tags: " + ','.join(tags))
     if pattern:
         pat = re.compile(pattern)
         tags = [tag for tag in tags if pat.match(tag)]
         if not tags:
-                return (None, "No matching tags found", None)
+            raise FDroidException(_('No matching tags found'))
         logging.debug("Matching tags: " + ','.join(tags))
 
     if len(tags) > 5 and repotype == 'git':
@@ -226,26 +209,16 @@ def check_tags(app, pattern):
         except VCSException:
             pass
         return (hver, hcode, htag)
-        return (None, "Couldn't find any version information", None)
+    raise FDroidException(_("Couldn't find any version information"))
-
-    except VCSException as vcse:
-        msg = "VCS error while scanning app {0}: {1}".format(app.id, vcse)
-        return (None, msg, None)
-    except Exception:
-        msg = "Could not scan app {0} due to unknown error: {1}".format(app.id, traceback.format_exc())
-        return (None, msg, None)
 
 
-# Check for a new version by looking at the AndroidManifest.xml at the HEAD
-# of the source repo. Whether this can be used reliably or not depends on
-# the development procedures used by the project's developers. Use it with
-# caution, because it's inappropriate for many projects.
-# Returns (None, "a message") if this didn't work, or (version, vercode) for
-# the details of the current version.
 def check_repomanifest(app, branch=None):
+    """Check for a new version by looking at the AndroidManifest.xml at the HEAD of the source repo.
 
-    try:
+    Whether this can be used reliably or not depends on
-
+    the development procedures used by the project's developers. Use it with
+    caution, because it's inappropriate for many projects.
+    """
     if app.RepoType == 'srclib':
         build_dir = Path('build/srclib') / app.Repo
         repotype = common.getsrclibvcs(app.Repo)
@@ -290,22 +263,13 @@ def check_repomanifest(app, branch=None):
                 hver = version
 
     if not hpak:
-            return (None, "Couldn't find package ID")
+        raise FDroidException(_("Couldn't find package ID"))
     if hver:
         return (hver, hcode)
-        return (None, "Couldn't find any version information")
+    raise FDroidException(_("Couldn't find any version information"))
-
-    except VCSException as vcse:
-        msg = "VCS error while scanning app {0}: {1}".format(app.id, vcse)
-        return (None, msg)
-    except Exception:
-        msg = "Could not scan app {0} due to unknown error: {1}".format(app.id, traceback.format_exc())
-        return (None, msg)
 
 
 def check_repotrunk(app):
-
-    try:
     if app.RepoType == 'srclib':
         build_dir = Path('build/srclib') / app.Repo
         repotype = common.getsrclibvcs(app.Repo)
@@ -314,7 +278,7 @@ def check_repotrunk(app):
         repotype = app.RepoType
 
     if repotype not in ('git-svn', ):
-            return (None, 'RepoTrunk update mode only makes sense in git-svn repositories')
+        raise MetaDataException(_('RepoTrunk update mode only makes sense in git-svn repositories'))
 
     # Set up vcs interface and make sure we have the latest code...
     vcs = common.getvcs(app.RepoType, app.Repo, build_dir)
@@ -323,12 +287,6 @@ def check_repotrunk(app):
 
     ref = vcs.getref()
     return (ref, ref)
-    except VCSException as vcse:
-        msg = "VCS error while scanning app {0}: {1}".format(app.id, vcse)
-        return (None, msg)
-    except Exception:
-        msg = "Could not scan app {0} due to unknown error: {1}".format(app.id, traceback.format_exc())
-        return (None, msg)
 
 
 # Check for a new version by looking at the Google Play Store.
@@ -461,34 +419,24 @@ def checkupdates_app(app):
     commitmsg = None
 
     tag = None
-    msg = None
     vercode = None
-    noverok = False
     mode = app.UpdateCheckMode
     if mode.startswith('Tags'):
         pattern = mode[5:] if len(mode) > 4 else None
         (version, vercode, tag) = check_tags(app, pattern)
-        msg = vercode
     elif mode == 'RepoManifest':
         (version, vercode) = check_repomanifest(app)
-        msg = vercode
     elif mode.startswith('RepoManifest/'):
         tag = mode[13:]
         (version, vercode) = check_repomanifest(app, tag)
-        msg = vercode
     elif mode == 'RepoTrunk':
         (version, vercode) = check_repotrunk(app)
-        msg = vercode
     elif mode == 'HTTP':
         (version, vercode) = check_http(app)
-        msg = vercode
     elif mode in ('None', 'Static'):
-        version = None
+        raise MetaDataException(_('Checking disabled'))
-        msg = 'Checking disabled'
-        noverok = True
     else:
-        version = None
+        raise MetaDataException(_('Invalid UpdateCheckMode: {mode}').format(mode=mode))
-        msg = 'Invalid update check method'
 
     if version and vercode and app.VercodeOperation:
         if not common.VERCODE_OPERATION_RE.match(app.VercodeOperation):
@@ -507,13 +455,9 @@ def checkupdates_app(app):
 
     updating = False
     if version is None:
-        logmsg = "...{0} : {1}".format(app.id, msg)
+        logging.warning('no version information found for {appid}'.format(appid=app.id))
-        if noverok:
-            logging.info(logmsg)
-        else:
-            logging.warning(logmsg)
     elif vercode == app.CurrentVersionCode:
-        logging.info("...up to date")
+        logging.debug("...up to date")
     elif int(vercode) > int(app.CurrentVersionCode):
         logging.debug("...updating - old vercode={0}, new vercode={1}".format(
             app.CurrentVersionCode, vercode))
@@ -521,9 +465,11 @@ def checkupdates_app(app):
         app.CurrentVersionCode = str(int(vercode))
         updating = True
     else:
-        logging.info("Refusing to auto update, since the current version is newer")
+        raise FDroidException(
-        logging.debug("...old vercode={0}, new vercode={1}".format(
+            _('current version is newer: old vercode={old}, new vercode={new}').format(
-            app.CurrentVersionCode, vercode))
+                old=app.CurrentVersionCode, new=vercode
+            )
+        )
 
     commitmsg = fetch_autoname(app, tag)
 
@@ -536,7 +482,9 @@ def checkupdates_app(app):
     if options.auto:
         mode = app.AutoUpdateMode
         if not app.CurrentVersionCode:
-            logging.warning("Can't auto-update app with no CurrentVersionCode: " + app.id)
+            raise MetaDataException(
+                _("Can't auto-update app with no CurrentVersionCode")
+            )
         elif mode in ('None', 'Static'):
             pass
         elif mode.startswith('Version'):
@@ -557,7 +505,11 @@ def checkupdates_app(app):
                     latest = build
 
             if int(latest.versionCode) > int(app.CurrentVersionCode):
-                logging.info("Refusing to auto update, since the latest build is newer")
+                raise FDroidException(
+                    _(
+                        'latest build recipe is newer: old vercode={old}, new vercode={new}'
+                    ).format(old=latest.versionCode, new=app.CurrentVersionCode)
+                )
 
             if not gotcur:
                 newbuild = copy.deepcopy(latest)
@@ -577,7 +529,9 @@ def checkupdates_app(app):
                 ver = _getcvname(app)
                 commitmsg = "Update %s to %s" % (name, ver)
         else:
-            logging.warning('Invalid auto update mode "' + mode + '" on ' + app.id)
+            raise MetaDataException(
+                _('Invalid AutoUpdateMode: {mode}').format(mode=mode)
+            )
 
     if commitmsg:
         metadata.write_metadata(app.metadatapath, app)
@@ -705,6 +659,7 @@ def main():
     locallog = ''
     processed = []
     failed = dict()
+    exit_code = 0
     for appid, app in apps.items():
 
         if options.autoonly and app.AutoUpdateMode in ('None', 'Static'):
@@ -721,12 +676,15 @@ def main():
         except Exception as e:
             msg = _("...checkupdate failed for {appid} : {error}").format(appid=appid, error=e)
             logging.error(msg)
+            logging.debug(traceback.format_exc())
             locallog += msg + '\n'
             failed[appid] = str(e)
+            exit_code = 1
 
     update_wiki(None, locallog)
     status_update_json(processed, failed)
     logging.info(_("Finished"))
+    sys.exit(exit_code)
 
 
 if __name__ == "__main__":

tests/checkupdates.TestCase

@@ -65,6 +65,7 @@ class CheckupdatesTest(unittest.TestCase):
         ):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
+                    with self.assertRaises(FDroidException):
                         fdroidserver.checkupdates.checkupdates_app(app)
                 build = app['Builds'][-1]
                 self.assertEqual(build.versionName, '1.1.9')
@@ -185,9 +186,8 @@ class CheckupdatesTest(unittest.TestCase):
             faked = scheme + '://fake.url/for/testing/scheme'
             app.UpdateCheckData = faked + '|ignored|' + faked + '|ignored'
             app.metadatapath = 'metadata/' + app.id + '.yml'
-            vername, vercode = fdroidserver.checkupdates.check_http(app)
+            with self.assertRaises(FDroidException):
-            self.assertIsNone(vername)
+                fdroidserver.checkupdates.check_http(app)
-            self.assertTrue(FDroidException.__name__ in vercode)
 
     def test_check_http_ignore(self):
         fdroidserver.checkupdates.options = mock.Mock()
@@ -205,7 +205,6 @@ class CheckupdatesTest(unittest.TestCase):
         with mock.patch('urllib.request.urlopen', lambda a, b, c: respmock):
             vername, vercode = fdroidserver.checkupdates.check_http(app)
             self.assertEqual(vername, None)
-            self.assertEqual(vercode, 'Version 1.1.9-beta is ignored')
 
     def test_check_tags_data(self):
         fdroidserver.checkupdates.options = mock.Mock()

tests/run-tests

@@ -688,7 +688,7 @@ LOCAL_COPY_DIR=`create_test_dir`/fdroid
 mkdir -p $LOCAL_COPY_DIR/repo
 echo "local_copy_dir: $LOCAL_COPY_DIR" >> config.yml
 
-$fdroid checkupdates --allow-dirty
+$fdroid checkupdates --allow-dirty || true
 which gpg && $fdroid gpgsign
 $fdroid lint
 $fdroid readmeta