Merge branch 'big-icon-extraction-overhaul' into 'master'

implement all paths for extracting PNG icons; purge related ancient cruft

Closes #885

See merge request fdroid/fdroidserver!1788

fdroidserver/__init__.py

@@ -12,16 +12,16 @@ rootpaths = [
     os.path.join(sys.prefix, 'share'),
 ]
 
-localedir = None
+LOCALEDIR = None
 for rootpath in rootpaths:
     found_mo = glob.glob(
         os.path.join(rootpath, 'locale', '*', 'LC_MESSAGES', 'fdroidserver.mo')
     )
     if len(found_mo) > 0:
-        localedir = os.path.join(rootpath, 'locale')
+        LOCALEDIR = os.path.join(rootpath, 'locale')
         break
 
-gettext.bindtextdomain('fdroidserver', localedir)
+gettext.bindtextdomain('fdroidserver', LOCALEDIR)
 gettext.textdomain('fdroidserver')
 _ = gettext.gettext
 

fdroidserver/common.py

@@ -79,7 +79,7 @@ import zipfile
 from argparse import BooleanOptionalAction
 from base64 import urlsafe_b64encode
 from binascii import hexlify
-from datetime import datetime, timedelta, timezone
+from datetime import datetime, timezone
 from pathlib import Path
 from queue import Queue
 from typing import List
@@ -2894,23 +2894,6 @@ def natural_key(s):
     return [int(sp) if sp.isdigit() else sp for sp in re.split(r'(\d+)', s)]
 
 
-def check_system_clock(dt_obj, path):
-    """Check if system clock is updated based on provided date.
-
-    If an APK has files newer than the system time, suggest updating
-    the system clock.  This is useful for offline systems, used for
-    signing, which do not have another source of clock sync info. It
-    has to be more than 24 hours newer because ZIP/APK files do not
-    store timezone info
-
-    """
-    checkdt = dt_obj - timedelta(1)
-    if datetime.today() < checkdt:
-        logging.warning(_('System clock is older than date in {path}!').format(path=path)
-                        + '\n' + _('Set clock to that time using:') + '\n'
-                        + 'sudo date -s "' + str(dt_obj) + '"')
-
-
 def get_file_extension(filename):
     """Get the normalized file extension, can be blank string but never None."""
     if isinstance(filename, bytes):

fdroidserver/mirror.py

@@ -168,10 +168,6 @@ def main():
             )
         )
 
-    icondirs = ['icons']
-    for density in update.screen_densities:
-        icondirs.append('icons-' + density)
-
     if options.output_dir:
         basedir = options.output_dir
     else:
@@ -193,7 +189,7 @@ def main():
 
         os.makedirs(sectiondir, exist_ok=True)
         os.chdir(sectiondir)
-        for icondir in icondirs:
+        for icondir in update.get_icon_dirs(section):
             os.makedirs(os.path.join(sectiondir, icondir), exist_ok=True)
 
         for packageName, packageList in data['packages'].items():
@@ -259,16 +255,17 @@ def main():
                 )
                 continue
             icon = app['icon']
-            for icondir in icondirs:
-                url = _append_to_url_path(section, icondir, icon)
+            for icondir in update.get_icon_dirs(section):
+                url = _append_to_url_path(icondir, icon)
                 if icondir not in urls:
                     urls[icondir] = []
                 urls[icondir].append(url)
 
-        for icondir in icondirs:
+        for icondir in update.get_icon_dirs(section):
+            print(icondir, 'icondir in urls', icondir in urls, sep='\t')
             if icondir in urls:
                 _run_wget(
-                    os.path.join(basedir, section, icondir),
+                    os.path.join(basedir, icondir),
                     urls[icondir],
                     options.verbose,
                 )

fdroidserver/update.py

@@ -21,6 +21,7 @@
 
 import argparse
 import copy
+import enum
 import filecmp
 import gettext
 import glob
@@ -36,7 +37,6 @@ import time
 import warnings
 import zipfile
 from argparse import ArgumentParser
-from datetime import datetime
 from pathlib import Path
 
 import asn1crypto.cms
@@ -53,6 +53,9 @@ from binascii import hexlify
 
 from PIL import Image, PngImagePlugin
 
+if not hasattr(Image, 'Resampling'):  # Pillow<9.0
+    Image.Resampling = Image
+
 import fdroidserver.index
 
 from . import _, common, metadata
@@ -81,22 +84,28 @@ APK_SDK_VERSION_PAT = re.compile(".*'([0-9]*)'.*")
 APK_PERMISSION_PAT = re.compile(r".*name='([^']*)'(?:.*maxSdkVersion='([^']*)')?.*")
 APK_FEATURE_PAT = re.compile(".*name='([^']*)'.*")
 
-screen_densities = ['65534', '640', '480', '320', '240', '160', '120']
-# resolutions must end with 'dpi'
-screen_resolutions = {
-    "xxxhdpi": '640',
-    "xxhdpi": '480',
-    "xhdpi": '320',
-    "hdpi": '240',
-    "mdpi": '160',
-    "ldpi": '120',
-    "tvdpi": '213',
-    "undefineddpi": '-1',
-    "anydpi": '65534',
-    "nodpi": '65535',
-}
+SCREEN_DENSITIES = [65534, 640, 480, 320, 240, 160, 120]
+
+
+# resolutions must end with 'dpi'
+# https://android.googlesource.com/platform/tools/base/+/refs/tags/studio-2025.3.4/build-system/aaptcompiler/src/main/java/com/android/aaptcompiler/android/ResTableConfig.kt#372
+@enum.unique
+class SCREEN_RESOLUTIONS(enum.IntEnum):
+    xxxhdpi = 640
+    xxhdpi = 480
+    xhdpi = 320
+    hdpi = 240
+    mdpi = 160
+    ldpi = 120
+    tvdpi = 213
+    anydpi = 65534
+    nodpi = 65535
+    default = 0
+
+    def get(key):
+        """Look up int values using string keys; don't throw KeyError on bad key."""
+        return SCREEN_RESOLUTIONS.__dict__.get(key)
 
-all_screen_densities = ['0'] + screen_densities
 
 ALLOWED_EXTENSIONS = ('png', 'jpg', 'jpeg')
 GRAPHIC_NAMES = ('featureGraphic', 'icon', 'promoGraphic', 'tvBanner')
@@ -161,7 +170,7 @@ class PackageAddedCache:
 
 
 def dpi_to_px(density):
-    return (int(density) * 48) / 160
+    return (density * 48) / 160
 
 
 def px_to_dpi(px):
@@ -174,19 +183,18 @@ def get_old_icon_filename(appid, versionCode):
 
 
 def get_icon_dir(repodir, density):
-    if density in ('0', '65534'):
+    if density in (
+        SCREEN_RESOLUTIONS.default,
+        SCREEN_RESOLUTIONS.anydpi,
+        SCREEN_RESOLUTIONS.nodpi,
+    ):
         return os.path.join(repodir, "icons")
     else:
-        return os.path.join(repodir, "icons-%s" % density)
+        return os.path.join(repodir, "icons-%d" % density)
 
 
 def get_icon_dirs(repodir):
-    for density in screen_densities:
-        yield get_icon_dir(repodir, density)
-
-
-def get_all_icon_dirs(repodir):
-    for density in all_screen_densities:
+    for density in SCREEN_DENSITIES:
         yield get_icon_dir(repodir, density)
 
 
@@ -297,9 +305,8 @@ def delete_disabled_builds(apps, apkcache, repodirs):
                     os.path.join(repodir, apkfilename + '.asc'),
                     os.path.join(repodir, apkfilename[:-4] + "_src.tar.gz"),
                 ]
-                for density in all_screen_densities:
-                    repo_dir = get_icon_dir(repodir, density)
-                    files.append(os.path.join(repo_dir, iconfilename))
+                for icon_dir in get_icon_dirs(repodir):
+                    files.append(os.path.join(icon_dir, iconfilename))
 
                 for f in files:
                     if os.path.exists(f):
@@ -324,7 +331,7 @@ def resize_icon(iconpath, density):
 
         if any(length > size for length in im.size):
             oldsize = im.size
-            im.thumbnail((size, size), Image.LANCZOS)
+            im.thumbnail((size, size), Image.Resampling.LANCZOS)
             logging.debug("%s was too large at %s - new size is %s" % (
                 iconpath, oldsize, im.size))
             im.save(iconpath, "PNG", optimize=True,
@@ -347,7 +354,7 @@ def resize_all_icons(repodirs):
       the repo directories to process
     """
     for repodir in repodirs:
-        for density in screen_densities:
+        for density in SCREEN_DENSITIES:
             icon_dir = get_icon_dir(repodir, density)
             icon_glob = os.path.join(icon_dir, '*.png')
             for iconpath in glob.glob(icon_glob):
@@ -1742,29 +1749,53 @@ def scan_apk(apk_file):
 
 # fmt: off
 
-def _get_apk_icons_src(apkfile, icon_name):
+def _get_apk_icons_src(apkfile, apkobject, arsc):
     """Extract the paths to the app icon in all available densities.
 
-    The folder name is normally generated by the Android Tools, but
-    there is nothing that prevents people from using whatever DPI
-    names they make up.  Android will just ignore them, so we should
-    too.
+    Parse the manifest and the resources to find all available app
+    icons, in all available densities and file types (e.g. .png, .webp,
+    .xml).  "ic_launcher" was the semi-official default icon name back
+    in the day.
 
     """
+    icon_id_str = apkobject.get_attribute_value("application", "icon")
+    if not icon_id_str:
+        icon_id_str = apkobject.get_attribute_value("activity", "icon")
     icons_src = dict()
-    density_re = re.compile(r'^res/(.*)/{}\.png$'.format(icon_name))
-    with zipfile.ZipFile(apkfile) as zf:
-        for filename in zf.namelist():
-            m = density_re.match(filename)
-            if m:
-                folder = m.group(1).split('-')
-                try:
-                    density = screen_resolutions[folder[1]]
-                except Exception:
-                    density = '160'
-                icons_src[density] = m.group(0)
-    if icons_src.get('-1') is None and '160' in icons_src:
-        icons_src['-1'] = icons_src['160']
+    if not icon_id_str:
+        return icons_src
+    try:
+        with zipfile.ZipFile(apkfile) as zf:
+            names_in_zip = zf.namelist()
+
+        icon_id = int(icon_id_str.replace("@", "0x"), 16)
+
+        candidates = arsc.get_resolved_res_configs(icon_id)
+        for candidate in candidates:
+            density = candidate[0].get_density()
+            path = candidate[1]
+            if path.endswith('.xml') or path not in names_in_zip:
+                # check it actually exists in the ZIP, some
+                # toolkits do strange things, like Godot Engine.
+                continue
+            icons_src[density] = path
+        if not icons_src:
+            # no PNGs found, use the XML icon name
+            app_icon = apkobject.get_app_icon()
+            if app_icon:
+                png = os.path.basename(app_icon.replace('.xml', '.png'))
+                res_name_re = re.compile(
+                    rf'res/(drawable|mipmap)-?(x*[hlm]dpi|anydpi|nodpi).*/{png}'
+                )
+                for name in names_in_zip:
+                    m = res_name_re.match(name)
+                    if m:
+                        density = SCREEN_RESOLUTIONS.get(m.group(2))
+                        if density is not None:
+                            icons_src[density] = m.group()
+
+    except Exception as e:
+        logging.error("Cannot fetch icon from %s: %s" % (apkfile, str(e)))
     return icons_src
 
 
@@ -1955,19 +1986,9 @@ def scan_apk_androguard(apk, apkfile):
         # mistakenly put in 'manifest' in index-v2, TODO move to useSdk for index-v3
         manifest['maxSdkVersion'] = maxSdkVersion
 
-    icon_id_str = apkobject.get_attribute_value("application", "icon")
-    if icon_id_str:
-        try:
-            icon_id = int(icon_id_str.replace("@", "0x"), 16)
-            resource_id = arsc.get_id(apk['packageName'], icon_id)
-            if resource_id:
-                icon_name = arsc.get_id(apk['packageName'], icon_id)[1]
-            else:
-                # don't use 'anydpi' aka 0xFFFE aka 65534 since it is XML
-                icon_name = os.path.splitext(os.path.basename(apkobject.get_app_icon(max_dpi=65534 - 1)))[0]
-            apk['icons_src'] = _get_apk_icons_src(apkfile, icon_name)
-        except Exception as e:
-            logging.error("Cannot fetch icon from %s: %s" % (apkfile, str(e)))
+    icons_src = _get_apk_icons_src(apkfile, apkobject, arsc)
+    if icons_src:
+        apk['icons_src'] = icons_src
 
     arch_re = re.compile("^lib/(.*)/.*$")
     arch = set([arch_re.match(file).group(1) for file in apkobject.get_files() if arch_re.match(file)])
@@ -2197,26 +2218,13 @@ def process_apk(apkcache, apkfilename, repodir, package_added_cache, use_date_fr
                                 .format(apkfilename=apkfilename))
             return True, None, False
 
-        apkzip = zipfile.ZipFile(apkfile, 'r')
-
-        manifest = apkzip.getinfo('AndroidManifest.xml')
-        # 1980-0-0 means zeroed out, any other invalid date should trigger a warning
-        if (1980, 0, 0) != manifest.date_time[0:3]:
-            try:
-                common.check_system_clock(datetime(*manifest.date_time), apkfilename)
-            except ValueError as e:
-                logging.warning(_("{apkfilename}'s AndroidManifest.xml has a bad date: ")
-                                .format(apkfilename=apkfile) + str(e))
-
-        # extract icons from APK zip file
-        iconfilename = get_old_icon_filename(apk['packageName'], apk['versionCode'])
-        try:
-            empty_densities = extract_apk_icons(iconfilename, apk, apkzip, repodir)
-        finally:
-            apkzip.close()  # ensure that APK zip file gets closed
-
-        # resize existing icons for densities missing in the APK
-        fill_missing_icon_densities(empty_densities, iconfilename, apk, repodir)
+        # Do not extract icons in archive, they have not been used there
+        # in a very long time, if ever. And if so, only in specific cases.
+        if repodir == 'repo':
+            iconfilename = get_old_icon_filename(apk['packageName'], apk['versionCode'])
+            with zipfile.ZipFile(apkfile, 'r') as apkzip:
+                empty_densities = extract_apk_icons(iconfilename, apk, apkzip, repodir)
+                fill_missing_icon_densities(empty_densities, iconfilename, apk, repodir)
 
         apk['added'] = package_added_cache.get(apkfile, use_date_from_apk)
 
@@ -2251,7 +2259,7 @@ def process_apks(apkcache, repodir, package_added_cache, use_date_from_apk=False
     """
     cachechanged = False
 
-    for icon_dir in get_all_icon_dirs(repodir):
+    for icon_dir in get_icon_dirs(repodir):
         if os.path.exists(icon_dir):
             if options is not None and options.clean:
                 shutil.rmtree(icon_dir)
@@ -2281,6 +2289,13 @@ def extract_apk_icons(icon_filename, apk, apkzip, repo_dir):
     metadata dictionary.  If the icon is an XML icon, then this tries
     to find PNG icon that can replace it.
 
+    There are some odd special cases for DPI values, including:
+    * 0 means the old default when no DPI specified.
+    * 65535 means special case 'nodpi', which is the final fallback case.
+
+    For more, see the docstring on Androguard's get_app_icon():
+    https://github.com/androguard/androguard/blob/dd458bead6165975c3ef0b1b78eaf2450e4889d9/androguard/core/apk/__init__.py#L681
+
     Parameters
     ----------
     icon_filename
@@ -2298,32 +2313,19 @@ def extract_apk_icons(icon_filename, apk, apkzip, repo_dir):
     A list of icon densities that are missing
 
     """
-    res_name_re = re.compile(r'res/(drawable|mipmap)-(x*[hlm]dpi|anydpi).*/(.*)_[0-9]+dp.(png|xml)')
-    pngs = dict()
-    for f in apkzip.namelist():
-        m = res_name_re.match(f)
-        if m and m.group(4) == 'png':
-            density = screen_resolutions[m.group(2)]
-            pngs[m.group(3) + '/' + density] = m.group(0)
     empty_densities = []
-    for density in screen_densities:
+    for density in SCREEN_DENSITIES:
         if density not in apk['icons_src']:
             empty_densities.append(density)
             continue
         icon_src = apk['icons_src'][density]
+        if icon_src.endswith('.xml'):
+            empty_densities.append(density)
+            continue
+
         icon_dir = get_icon_dir(repo_dir, density)
         icon_dest = os.path.join(icon_dir, icon_filename)
 
-        # Extract the icon files per density
-        if icon_src.endswith('.xml'):
-            m = res_name_re.match(icon_src)
-            if m:
-                name = pngs.get(m.group(3) + '/' + str(density))
-                if name:
-                    icon_src = name
-            if icon_src.endswith('.xml'):
-                empty_densities.append(density)
-                continue
         try:
             with open(icon_dest, 'wb') as f:
                 f.write(get_icon_bytes(apkzip, icon_src))
@@ -2333,20 +2335,26 @@ def extract_apk_icons(icon_filename, apk, apkzip, repo_dir):
             del apk['icons_src'][density]
             empty_densities.append(density)
 
-    # '-1' here is a remnant of the parsing of aapt output, meaning "no DPI specified"
-    if '-1' in apk['icons_src'] and not apk['icons_src']['-1'].endswith('.xml'):
-        icon_src = apk['icons_src']['-1']
-        icon_path = os.path.join(get_icon_dir(repo_dir, '0'), icon_filename)
+    non_dpi_case = None
+    if SCREEN_RESOLUTIONS.default in apk['icons_src']:
+        non_dpi_case = SCREEN_RESOLUTIONS.default
+    elif SCREEN_RESOLUTIONS.nodpi in apk['icons_src']:
+        non_dpi_case = SCREEN_RESOLUTIONS.nodpi
+
+    # move image based on DPI from measuring the image size
+    if non_dpi_case is not None:
+        icon_src = apk['icons_src'][non_dpi_case]
+        icon_path = os.path.join(get_icon_dir(repo_dir, non_dpi_case), icon_filename)
         with open(icon_path, 'wb') as f:
             f.write(get_icon_bytes(apkzip, icon_src))
         im = None
         try:
             im = Image.open(icon_path)
             dpi = px_to_dpi(im.size[0])
-            for density in screen_densities:
+            for density in SCREEN_DENSITIES:
                 if density in apk['icons']:
                     break
-                if density == screen_densities[-1] or dpi >= int(density):
+                if density == SCREEN_DENSITIES[-1] or dpi >= density:
                     apk['icons'][density] = icon_filename
                     shutil.move(icon_path,
                                 os.path.join(get_icon_dir(repo_dir, density), icon_filename))
@@ -2378,8 +2386,8 @@ def fill_missing_icon_densities(empty_densities, icon_filename, apk, repo_dir):
     """
     # First try resizing down to not lose quality
     last_density = None
-    for density in screen_densities:
-        if density == '65534':  # not possible to generate 'anydpi' from other densities
+    for density in SCREEN_DENSITIES:
+        if density == SCREEN_RESOLUTIONS.anydpi:  # cannot generate from other density
             continue
         if density not in empty_densities:
             last_density = density
@@ -2397,7 +2405,7 @@ def fill_missing_icon_densities(empty_densities, icon_filename, apk, repo_dir):
 
             size = dpi_to_px(density)
 
-            im.thumbnail((size, size), Image.LANCZOS)
+            im.thumbnail((size, size), Image.Resampling.LANCZOS)
             im.save(icon_path, "PNG", optimize=True,
                     pnginfo=BLANK_PNG_INFO, icc_profile=None)
             empty_densities.remove(density)
@@ -2409,7 +2417,7 @@ def fill_missing_icon_densities(empty_densities, icon_filename, apk, repo_dir):
 
     # Then just copy from the highest resolution available
     last_density = None
-    for density in reversed(screen_densities):
+    for density in reversed(SCREEN_DENSITIES):
         if density not in empty_densities:
             last_density = density
             continue
@@ -2423,16 +2431,17 @@ def fill_missing_icon_densities(empty_densities, icon_filename, apk, repo_dir):
         )
         empty_densities.remove(density)
 
-    for density in screen_densities:
+    # If any of the icons are too big, then size them down.
+    for density in SCREEN_DENSITIES:
         icon_dir = get_icon_dir(repo_dir, density)
         icon_dest = os.path.join(icon_dir, icon_filename)
         resize_icon(icon_dest, density)
 
     # Copy from icons-mdpi to icons since mdpi is the baseline density
-    baseline = os.path.join(get_icon_dir(repo_dir, '160'), icon_filename)
+    baseline = os.path.join(get_icon_dir(repo_dir, 160), icon_filename)
     if os.path.isfile(baseline):
-        apk['icons']['0'] = icon_filename
-        shutil.copyfile(baseline, os.path.join(get_icon_dir(repo_dir, '0'), icon_filename))
+        apk['icons'][0] = icon_filename
+        shutil.copyfile(baseline, os.path.join(get_icon_dir(repo_dir, 0), icon_filename))
 
 
 def apply_info_from_latest_apk(apps, apks):
@@ -2537,12 +2546,11 @@ def move_apk_between_sections(from_dir, to_dir, apk):
     _move_file(from_dir, to_dir, filename + '.asc', True)
     _move_file(from_dir, to_dir, filename + '.idsig', True)
     _move_file(from_dir, to_dir, filename[:-4] + '.log.gz', True)
-    for density in all_screen_densities:
+    for density in SCREEN_DENSITIES:
         from_icon_dir = get_icon_dir(from_dir, density)
         to_icon_dir = get_icon_dir(to_dir, density)
-        if density not in apk.get('icons', []):
-            continue
-        _move_file(from_icon_dir, to_icon_dir, apk['icons'][density], True)
+        default = get_old_icon_filename(apk['packageName'], apk['versionCode'])
+        _move_file(from_icon_dir, to_icon_dir, apk['icons'].get(density, default), True)
     if 'srcname' in apk:
         _move_file(from_dir, to_dir, apk['srcname'], False)
         _move_file(from_dir, to_dir, apk['srcname'] + '.asc', True)

tests/metadata/apk/info.guardianproject.urzip.yaml

@@ -6,11 +6,10 @@ file:
   ipfsCIDv1: bafybeigmtgrwyvj77jaflje2rf533haeqtpu2wtwsctryjusjnsawacsam
 icon: info.guardianproject.urzip.100.png
 icons:
-  '0': info.guardianproject.urzip.100.png
-  '160': info.guardianproject.urzip.100.png
+  0: info.guardianproject.urzip.100.png
+  120: info.guardianproject.urzip.100.png
 icons_src:
-  '-1': res/drawable/ic_launcher.png
-  '160': res/drawable/ic_launcher.png
+  0: res/drawable/ic_launcher.png
 manifest:
   signer:
     sha256:

tests/metadata/apk/org.dyndns.fules.ck.yaml

@@ -6,15 +6,14 @@ file:
   ipfsCIDv1: bafybeifijmr5ygvfvig4vzbmdc3ysj6m46ddohaol4vgp4qoyooqpc27zu
 icon: org.dyndns.fules.ck.20.png
 icons:
-  '0': org.dyndns.fules.ck.20.png
-  '120': org.dyndns.fules.ck.20.png
-  '160': org.dyndns.fules.ck.20.png
-  '240': org.dyndns.fules.ck.20.png
+  0: org.dyndns.fules.ck.20.png
+  120: org.dyndns.fules.ck.20.png
+  160: org.dyndns.fules.ck.20.png
+  240: org.dyndns.fules.ck.20.png
 icons_src:
-  '-1': res/drawable-mdpi-v4/icon_launcher.png
-  '120': res/drawable-ldpi-v4/icon_launcher.png
-  '160': res/drawable-mdpi-v4/icon_launcher.png
-  '240': res/drawable-hdpi-v4/icon_launcher.png
+  120: res/drawable-ldpi-v4/icon_launcher.png
+  160: res/drawable-mdpi-v4/icon_launcher.png
+  240: res/drawable-hdpi-v4/icon_launcher.png
 manifest:
   nativecode:
     - arm64-v8a

tests/metadata/apk/org.maxsdkversion.yaml

@@ -6,11 +6,10 @@ file:
   ipfsCIDv1: bafybeibdls2h4mpfw5gks3iirsne2qaez6uefwb5xmqkhahqbakvdszk6y
 icon: org.maxsdkversion.4.png
 icons:
-  '0': org.maxsdkversion.4.png
-  '160': org.maxsdkversion.4.png
+  0: org.maxsdkversion.4.png
+  160: org.maxsdkversion.4.png
 icons_src:
-  '-1': res/drawable-mdpi-v4/mirror.png
-  '160': res/drawable-mdpi-v4/mirror.png
+  160: res/drawable-mdpi-v4/mirror.png
 manifest:
   features:
     - name: android.hardware.camera.front

tests/test_update.py

@@ -61,9 +61,8 @@ class Options:
     verbose = False
 
 
-@unittest.skipIf(sys.byteorder == 'big', 'androguard is not ported to big-endian')
-class UpdateTest(unittest.TestCase):
-    '''fdroid update'''
+class SetUpTearDownMixin:
+    """A mixin with no tests in it for shared setUp and tearDown."""
 
     def setUp(self):
         os.chdir(basedir)
@@ -77,6 +76,9 @@ class UpdateTest(unittest.TestCase):
         os.chdir(basedir)
         self._td.cleanup()
 
+
+@unittest.skipIf(sys.byteorder == 'big', 'androguard is not ported to big-endian')
+class UpdateTest(SetUpTearDownMixin, unittest.TestCase):
     def test_insert_store_metadata(self):
         os.chdir(self.testdir)
 
@@ -909,8 +911,8 @@ class UpdateTest(unittest.TestCase):
     def test_scan_apk_features(self):
         apk_info = fdroidserver.update.scan_apk('repo/duplicate.permisssions_9999999.apk')
         self.assertEqual(apk_info['manifest']['versionName'], '')
-        self.assertEqual(apk_info['icons_src'], {'160': 'res/drawable/ic_launcher.png',
-                                                 '-1': 'res/drawable/ic_launcher.png'})
+        self.assertEqual(apk_info['icons_src'], {0: 'res/drawable/ic_launcher.png'})
+
         self.assertEqual(
             apk_info['manifest']['features'],
             [{'name': 'android.hardware.telephony'}],
@@ -918,10 +920,9 @@ class UpdateTest(unittest.TestCase):
 
     def test_scan_apk_lots_of_data(self):
         apk_info = fdroidserver.update.scan_apk('org.dyndns.fules.ck_20.apk')
-        self.assertEqual(apk_info['icons_src'], {'240': 'res/drawable-hdpi-v4/icon_launcher.png',
-                                                 '120': 'res/drawable-ldpi-v4/icon_launcher.png',
-                                                 '160': 'res/drawable-mdpi-v4/icon_launcher.png',
-                                                 '-1': 'res/drawable-mdpi-v4/icon_launcher.png'})
+        self.assertEqual(apk_info['icons_src'], {240: 'res/drawable-hdpi-v4/icon_launcher.png',
+                                                 120: 'res/drawable-ldpi-v4/icon_launcher.png',
+                                                 160: 'res/drawable-mdpi-v4/icon_launcher.png'})
         self.assertEqual(apk_info['icons'], {})
         self.assertEqual(apk_info['antiFeatures'], dict())
         self.assertEqual(apk_info['manifest']['versionName'], 'v1.6pre2')
@@ -946,8 +947,7 @@ class UpdateTest(unittest.TestCase):
     def test_scan_apk_two_icons(self):
         apk_info = fdroidserver.update.scan_apk('org.bitbucket.tickytacky.mirrormirror_4.apk')
         self.assertEqual(apk_info['manifest']['versionName'], '1.0.3')
-        self.assertEqual(apk_info['icons_src'], {'160': 'res/drawable-mdpi/mirror.png',
-                                                 '-1': 'res/drawable-mdpi/mirror.png'})
+        self.assertEqual(apk_info['icons_src'], {160: 'res/drawable-mdpi/mirror.png'})
 
     def test_scan_apk_xml_icon(self):
         apk_info = fdroidserver.update.scan_apk('repo/info.zwanenburg.caffeinetile_4.apk')
@@ -957,11 +957,10 @@ class UpdateTest(unittest.TestCase):
     def test_scan_apk_old_icons(self):
         apk_info = fdroidserver.update.scan_apk('repo/com.politedroid_6.apk')
         self.assertEqual(apk_info['manifest']['versionName'], '1.5')
-        self.assertEqual(apk_info['icons_src'], {'120': 'res/drawable-ldpi-v4/icon.png',
-                                                 '160': 'res/drawable-mdpi-v4/icon.png',
-                                                 '240': 'res/drawable-hdpi-v4/icon.png',
-                                                 '320': 'res/drawable-xhdpi-v4/icon.png',
-                                                 '-1': 'res/drawable-mdpi-v4/icon.png'})
+        self.assertEqual(apk_info['icons_src'], {120: 'res/drawable-ldpi-v4/icon.png',
+                                                 160: 'res/drawable-mdpi-v4/icon.png',
+                                                 240: 'res/drawable-hdpi-v4/icon.png',
+                                                 320: 'res/drawable-xhdpi-v4/icon.png'})
 
     def test_scan_apk_no_icons(self):
         apk_info = fdroidserver.update.scan_apk('SpeedoMeterApp.main_1.apk')
@@ -994,10 +993,7 @@ class UpdateTest(unittest.TestCase):
         self.maxDiff = None
         expected = {
             'icons': {},
-            'icons_src': {
-                '-1': 'res/drawable/ic_launcher.png',
-                '160': 'res/drawable/ic_launcher.png',
-            },
+            'icons_src': {0: 'res/drawable/ic_launcher.png'},
             'file': {
                 'name': 'no.min.target.sdk_987.apk',
                 'sha256': 'e2e1dc1d550df2b5bc383860139207258645b5540abeccd305ed8b2cb6459d2c',
@@ -1113,7 +1109,7 @@ class UpdateTest(unittest.TestCase):
         fdroidserver.update.options.clean = True
         fdroidserver.update.options.delete_unknown = True
 
-        for icon_dir in fdroidserver.update.get_all_icon_dirs('repo'):
+        for icon_dir in fdroidserver.update.get_icon_dirs('repo'):
             if not os.path.exists(icon_dir):
                 os.makedirs(icon_dir)
 
@@ -1132,7 +1128,7 @@ class UpdateTest(unittest.TestCase):
                 self.assertEqual(apk['icon'], 'info.guardianproject.urzip.100.png')
             if apkName == '../org.dyndns.fules.ck_20.apk':
                 self.assertEqual(apk['icon'], 'org.dyndns.fules.ck.20.png')
-            for density in fdroidserver.update.screen_densities:
+            for density in fdroidserver.update.SCREEN_DENSITIES:
                 icon_path = os.path.join(
                     fdroidserver.update.get_icon_dir('repo', density), apk['icon']
                 )
@@ -1225,7 +1221,7 @@ class UpdateTest(unittest.TestCase):
                 self.assertFalse(os.path.exists(os.path.join('repo', apkName)))
 
                 # ensure that icons have been moved to the archive as well
-                for density in fdroidserver.update.screen_densities:
+                for density in fdroidserver.update.SCREEN_DENSITIES:
                     icon_path = os.path.join(fdroidserver.update.get_icon_dir('archive', density),
                                              apk['icon'])
                     self.assertTrue(os.path.isfile(icon_path))
@@ -1570,16 +1566,6 @@ class UpdateTest(unittest.TestCase):
         with self.assertRaises(fdroidserver.exception.FDroidException):
             fdroidserver.update.has_known_vulnerability('janus.apk')
 
-    def test_get_apk_icon_when_src_is_none(self):
-        config = dict()
-        fdroidserver.common.fill_config_defaults(config)
-        fdroidserver.common.config = config
-        fdroidserver.update.config = config
-
-        # pylint: disable=protected-access
-        icons_src = fdroidserver.update._get_apk_icons_src('urzip-release.apk', None)
-        self.assertFalse(icons_src)
-
     def test_strip_and_copy_image(self):
         in_file = basedir / 'metadata/info.guardianproject.urzip/en-US/images/icon.png'
         out_file = os.path.join(self.testdir, 'icon.png')
@@ -1974,10 +1960,9 @@ class UpdateTest(unittest.TestCase):
                         'name': 'org.dyndns.fules.ck_20.apk',
                     },
                     'icons_src': {
-                        '240': 'res/drawable-hdpi-v4/icon_launcher.png',
-                        '120': 'res/drawable-ldpi-v4/icon_launcher.png',
-                        '160': 'res/drawable-mdpi-v4/icon_launcher.png',
-                        '-1': 'res/drawable-mdpi-v4/icon_launcher.png',
+                        240: 'res/drawable-hdpi-v4/icon_launcher.png',
+                        120: 'res/drawable-ldpi-v4/icon_launcher.png',
+                        160: 'res/drawable-mdpi-v4/icon_launcher.png',
                     },
                     'manifest': {
                         'nativecode': [
@@ -2251,6 +2236,324 @@ class UpdateTest(unittest.TestCase):
             index['repo'][CATEGORIES_CONFIG_NAME],
         )
 
+    def test_get_icon_dir_hdpi_density(self):
+        repodir = 'repo'
+        density = fdroidserver.update.SCREEN_RESOLUTIONS.hdpi
+        self.assertEqual(
+            f'{repodir}/icons-{density}',
+            fdroidserver.update.get_icon_dir(repodir, density),
+        )
+
+    def test_get_icon_dir_anydpi_density(self):
+        repodir = 'repo'
+        density = fdroidserver.update.SCREEN_RESOLUTIONS.anydpi
+        self.assertEqual(
+            f'{repodir}/icons',
+            fdroidserver.update.get_icon_dir(repodir, density),
+        )
+
+    def test_get_icon_dir_nodpi(self):
+        repodir = 'repo'
+        density = fdroidserver.update.SCREEN_RESOLUTIONS.nodpi
+        self.assertEqual(
+            f'{repodir}/icons',
+            fdroidserver.update.get_icon_dir(repodir, density),
+        )
+
+    def test_get_icon_dir_0(self):
+        """Test the very old "default" case."""
+        density = fdroidserver.update.SCREEN_RESOLUTIONS.default
+        repodir = 'repo'
+        self.assertEqual(
+            f'{repodir}/icons',
+            fdroidserver.update.get_icon_dir(repodir, density),
+        )
+
+
+class TestExtractApkIcons(SetUpTearDownMixin, unittest.TestCase):
+    def setUp(self):
+        super().setUp()
+        os.chdir(self.testdir)
+        repodir = 'repo'
+        os.mkdir(repodir)
+        for icon_dir in fdroidserver.update.get_icon_dirs(repodir):
+            if not os.path.exists(icon_dir):
+                os.mkdir(icon_dir)
+
+    def extract_apk_icons(self, apkfile, appid, versionCode=1):
+        apkobject = fdroidserver.common.get_androguard_APK(apkfile)
+        arsc = apkobject.get_android_resources()
+        icons_src = fdroidserver.update._get_apk_icons_src(apkfile, apkobject, arsc)
+        apk = {
+            'icons_src': icons_src,
+            'icons': {},
+            'packageName': appid,
+            'versionCode': versionCode,
+        }
+        self.filename = fdroidserver.update.get_old_icon_filename(
+            apk['packageName'], apk['versionCode']
+        )
+        with zipfile.ZipFile(apkfile) as apkzip:
+            empty_densities = fdroidserver.update.extract_apk_icons(
+                self.filename, apk, apkzip, 'repo'
+            )
+        self.apk = apk
+        return empty_densities
+
+    def test_extract_apk_icons_urzip(self):
+        apkfile = basedir / 'urzip.apk'
+        appid = 'info.guardianproject.urzip'
+        empty_densities = self.extract_apk_icons(apkfile, appid)
+        self.assertEqual([65534, 640, 480, 320, 240, 160], empty_densities)
+        self.assertEqual(1413, os.path.getsize(f'repo/icons-120/{self.filename}'))
+        for density in empty_densities:
+            self.assertFalse(os.path.exists(f'repo/icons-{density}/{self.filename}'))
+
+    def test_extract_apk_icons_mirrormirror(self):
+        appid = 'org.bitbucket.tickytacky.mirrormirror'
+        apkfile = basedir / f'{appid}_4.apk'
+        empty_densities = self.extract_apk_icons(apkfile, appid)
+        self.assertEqual([65534, 640, 480, 320, 240, 120], empty_densities)
+        self.assertEqual(91, os.path.getsize(f'repo/icons-160/{self.filename}'))
+        for density in empty_densities:
+            self.assertFalse(os.path.exists(f'repo/icons-{density}/{self.filename}'))
+
+    def test_extract_apk_icons_fules_ck(self):
+        appid = 'org.dyndns.fules.ck'
+        apkfile = basedir / f'{appid}_20.apk'
+        empty_densities = self.extract_apk_icons(apkfile, appid)
+        self.assertEqual([65534, 640, 480, 320], empty_densities)
+        self.assertEqual(1430, os.path.getsize(f'repo/icons-120/{self.filename}'))
+        self.assertEqual(2120, os.path.getsize(f'repo/icons-160/{self.filename}'))
+        self.assertEqual(3942, os.path.getsize(f'repo/icons-240/{self.filename}'))
+        for density in empty_densities:
+            self.assertFalse(os.path.exists(f'repo/icons-{density}/{self.filename}'))
+
+    def test_extract_apk_icons_fallingblocks(self):
+        """Test example made with Godot Engine."""
+        appid = 'org.sajeg.fallingblocks'
+        apkfile = basedir / f'{appid}_3.apk'
+        empty_densities = self.extract_apk_icons(apkfile, appid)
+        self.assertEqual([65534, 480, 320, 240, 160, 120], empty_densities)
+        self.assertEqual(6793, os.path.getsize(f'repo/icons-640/{self.filename}'))
+        for density in empty_densities:
+            self.assertFalse(os.path.exists(f'repo/icons-{density}/{self.filename}'))
+
+    def test_extract_apk_icons_org_maxsdkversion(self):
+        appid = 'org.maxsdkversion'
+        apkfile = basedir / f'repo/{appid}_4.apk'
+        empty_densities = self.extract_apk_icons(apkfile, appid)
+        self.assertEqual([65534, 640, 480, 320, 240, 120], empty_densities)
+        self.assertEqual(91, os.path.getsize(f'repo/icons-160/{self.filename}'))
+        for density in empty_densities:
+            self.assertFalse(os.path.exists(f'repo/icons-{density}/{self.filename}'))
+
+    def test_extract_apk_icons_souch_smsbypass(self):
+        appid = 'souch.smsbypass'
+        apkfile = basedir / f'repo/{appid}_9.apk'
+        empty_densities = self.extract_apk_icons(apkfile, appid)
+        self.assertEqual([65534, 640, 120], empty_densities)
+        self.assertEqual(1558, os.path.getsize(f'repo/icons-160/{self.filename}'))
+        self.assertEqual(3615, os.path.getsize(f'repo/icons-320/{self.filename}'))
+        self.assertEqual(5874, os.path.getsize(f'repo/icons-480/{self.filename}'))
+        for density in empty_densities:
+            self.assertFalse(os.path.exists(f'repo/icons-{density}/{self.filename}'))
+
+    def test_extract_apk_icons_SpeedoMeterApp_main(self):
+        """Test an APK with no icons."""
+        appid = 'SpeedoMeterApp.main'
+        apkfile = basedir / f'{appid}_1.apk'
+        empty_densities = self.extract_apk_icons(apkfile, appid)
+        self.assertEqual(fdroidserver.update.SCREEN_DENSITIES, empty_densities)
+        self.assertFalse(os.path.exists(f'repo/icons/{self.filename}'))
+        for density in empty_densities:
+            self.assertFalse(os.path.exists(f'repo/icons-{density}/{self.filename}'))
+
+    def test_extract_apk_icons_info_zwanenburg_caffeinetile(self):
+        """Test an APK with no PNG or WebP, only XML."""
+        appid = 'info.zwanenburg.caffeinetile'
+        apkfile = basedir / f'repo/{appid}_4.apk'
+        empty_densities = self.extract_apk_icons(apkfile, appid)
+        self.assertEqual(fdroidserver.update.SCREEN_DENSITIES, empty_densities)
+        self.assertFalse(os.path.exists(f'repo/icons/{self.filename}'))
+        for density in empty_densities:
+            self.assertFalse(os.path.exists(f'repo/icons-{density}/{self.filename}'))
+
+    def test_extract_apk_icons_rsynced(self):
+        """Test based on current production by rsyncing the key files.
+
+        To get the APKs:
+        rsync -axv --max-size=125k "ftp.fau.de::fdroid/repo/{appid}*.apk" ../f-droid.org/fdroid/repo/
+
+        Then just get all of the extracted icons, they are small:
+        rsync -axv "ftp.fau.de::fdroid/repo/icons*" ../f-droid.org/fdroid/repo/
+
+        """
+        production = basedir / '../../f-droid.org/fdroid/repo'
+        if not production.exists():
+            self.skipTest(f'No files rsynced to {production}')
+        for apkfile in sorted(production.glob('*.apk')):
+            appid, versionCode = apkfile.stem.rsplit('_', 1)
+            iconname = fdroidserver.update.get_old_icon_filename(appid, versionCode)
+            icon_glob = f'icons*/{iconname}'
+
+            prod_icons = dict()
+            for icon in sorted(production.glob(icon_glob)):
+                prod_icons[str(icon.relative_to(production))] = icon.stat().st_size
+
+            empty_densities = self.extract_apk_icons(apkfile, appid, versionCode)
+            fdroidserver.update.fill_missing_icon_densities(
+                empty_densities, self.filename, self.apk, 'repo'
+            )
+            repo_dir = Path(self.testdir) / 'repo'
+            test_icons = dict()
+            for icon in repo_dir.glob(icon_glob):
+                test_icons[str(icon.relative_to(repo_dir))] = icon.stat().st_size
+            # skip if the production algorithm has worse results, like
+            # it didn't extract, or has fewer icons
+            if prod_icons and len(test_icons) <= len(prod_icons):
+                self.assertEqual(prod_icons.keys(), test_icons.keys(), self.filename)
+
+    def test_move_apk_between_sections(self):
+        """Test when moving an APK that the extracted icons follow."""
+        appid = 'com.politedroid'
+        versionCode = 3
+        apkfile = f'repo/{appid}_{versionCode}.apk'
+        shutil.copy(basedir / apkfile, apkfile)
+        empty_densities = self.extract_apk_icons(apkfile, appid, versionCode)
+        fdroidserver.update.fill_missing_icon_densities(
+            empty_densities, self.filename, self.apk, 'repo'
+        )
+        self.apk['file'] = {'name': os.path.basename(apkfile)}
+        fdroidserver.update.move_apk_between_sections('repo', 'archive', self.apk)
+        self.assertEqual([], sorted(glob.glob('repo/icons*/*.png')))
+        self.assertEqual(
+            [
+                'archive/icons-120/com.politedroid.3.png',
+                'archive/icons-160/com.politedroid.3.png',
+                'archive/icons-240/com.politedroid.3.png',
+                'archive/icons-320/com.politedroid.3.png',
+                'archive/icons-480/com.politedroid.3.png',
+                'archive/icons-640/com.politedroid.3.png',
+                'archive/icons/com.politedroid.3.png',
+            ],
+            sorted(glob.glob('archive/icons*/*.png')),
+        )
+
+
+class TestGetApkIconsSrc(unittest.TestCase):
+    def get_apk_icons_src(self, apkfile):
+        apkobject = fdroidserver.common.get_androguard_APK(apkfile)
+        arsc = apkobject.get_android_resources()
+        return fdroidserver.update._get_apk_icons_src(apkfile, apkobject, arsc)
+
+    def test_get_apk_icons_src_urzip(self):
+        self.assertEqual(
+            {0: 'res/drawable/ic_launcher.png'},
+            self.get_apk_icons_src(basedir / 'urzip.apk'),
+        )
+
+    def test_get_apk_icons_src_mirrormirror(self):
+        appid = 'org.bitbucket.tickytacky.mirrormirror'
+        self.assertEqual(
+            {160: 'res/drawable-mdpi/mirror.png'},
+            self.get_apk_icons_src(basedir / f'{appid}_4.apk'),
+        )
+
+    def test_get_apk_icons_src_fules_ck(self):
+        appid = 'org.dyndns.fules.ck'
+        self.assertEqual(
+            {
+                120: 'res/drawable-ldpi-v4/icon_launcher.png',
+                160: 'res/drawable-mdpi-v4/icon_launcher.png',
+                240: 'res/drawable-hdpi-v4/icon_launcher.png',
+            },
+            self.get_apk_icons_src(basedir / f'{appid}_20.apk'),
+        )
+
+    def test_get_apk_icons_src_SpeedoMeterApp_main(self):
+        """Test handling APK with no icon set."""
+        appid = 'SpeedoMeterApp.main'
+        self.assertEqual(
+            {},
+            self.get_apk_icons_src(basedir / f'{appid}_1.apk'),
+        )
+
+    def test_get_apk_icons_src_fallingblocks(self):
+        """Test example made with Godot Engine."""
+        appid = 'org.sajeg.fallingblocks'
+        self.assertEqual(
+            {0: 'res/mipmap/icon.png'},
+            self.get_apk_icons_src(basedir / f'{appid}_3.apk'),
+        )
+
+    def test_get_apk_icons_src_com_example_test_helloworld(self):
+        """Test APK with no apparent icon, but some similarly named files."""
+        appid = 'com.example.test.helloworld'
+        self.assertEqual(
+            {},
+            self.get_apk_icons_src(basedir / f'repo/{appid}_1.apk'),
+        )
+
+    def test_get_apk_icons_src_com_politedroid_3(self):
+        appid = 'com.politedroid'
+        self.assertEqual(
+            {
+                120: 'res/drawable-ldpi/icon.png',
+                160: 'res/drawable-mdpi/icon.png',
+                240: 'res/drawable-hdpi/icon.png',
+                320: 'res/drawable-xhdpi/icon.png',
+            },
+            self.get_apk_icons_src(basedir / f'repo/{appid}_3.apk'),
+        )
+
+    def test_get_apk_icons_src_com_politedroid_6(self):
+        appid = 'com.politedroid'
+        self.assertEqual(
+            {
+                120: 'res/drawable-ldpi-v4/icon.png',
+                160: 'res/drawable-mdpi-v4/icon.png',
+                240: 'res/drawable-hdpi-v4/icon.png',
+                320: 'res/drawable-xhdpi-v4/icon.png',
+            },
+            self.get_apk_icons_src(basedir / f'repo/{appid}_6.apk'),
+        )
+
+    def test_get_apk_icons_src_duplicate_permisssions(self):
+        appid = 'duplicate.permisssions'
+        self.assertEqual(
+            {0: 'res/drawable/ic_launcher.png'},
+            self.get_apk_icons_src(basedir / f'repo/{appid}_9999999.apk'),
+        )
+
+    def test_get_apk_icons_src_info_zwanenburg_caffeinetile(self):
+        """Test an APK with no PNG or WebP, only XML."""
+        appid = 'info.zwanenburg.caffeinetile'
+        self.assertEqual(
+            dict(),
+            self.get_apk_icons_src(basedir / f'repo/{appid}_4.apk'),
+        )
+
+    def test_get_apk_icons_src_org_maxsdkversion(self):
+        appid = 'org.maxsdkversion'
+        self.assertEqual(
+            {160: 'res/drawable-mdpi-v4/mirror.png'},
+            self.get_apk_icons_src(basedir / f'repo/{appid}_4.apk'),
+        )
+
+    def test_get_apk_icons_src_souch_smsbypass(self):
+        appid = 'souch.smsbypass'
+        self.assertEqual(
+            {
+                160: 'res/drawable-mdpi-v4/ic_launcher.png',
+                213: 'res/drawable-tvdpi-v4/ic_launcher.png',
+                240: 'res/drawable-hdpi-v4/ic_launcher.png',
+                320: 'res/drawable-xhdpi-v4/ic_launcher.png',
+                480: 'res/drawable-xxhdpi-v4/ic_launcher.png',
+            },
+            self.get_apk_icons_src(basedir / f'repo/{appid}_9.apk'),
+        )
+
 
 class TestParseIpa(unittest.TestCase):
     def test_parse_ipa(self):