Stop using a spritesheet for site-icons

The site-icons spritesheet has already become unwieldy - it's almost
1MB, is mostly rarely-needed icons, and needs to be fully replaced and
re-downloaded whenever a new icon is added. With HTTP/2 now being widely
supported, spritesheets seem to be mostly obsolete, and I probably never
should have done it that way in the first place.

This commit changes over to simply using individual icon images, and
rebuilds the CSS file whenever new icons are downloaded. This new CSS
file will probably be somewhat large, but should gzip extremely well.
This probably still needs some work to support cache-busting on the CSS
file.

.gitignore

@@ -19,5 +19,5 @@ tildes/static/css/*
 tildes/static/js/third_party.js
 tildes/static/js/tildes.js
 
-# don't track the site-icons spritesheet(s)
-tildes/static/images/site-icons*
+# don't track site icon files
+tildes/static/images/site-icons/*.png

salt/salt/cronjobs.sls

@@ -6,8 +6,8 @@ data-cleanup-cronjob:
     - hour: 4
     - minute: 10
 
-generate-site-icons-cronjob:
+generate-site-icons-css-cronjob:
   cron.present:
-    - name: /usr/local/bin/generate-site-icons
+    - name: {{ bin_dir }}/python -c "from scripts.generate_site_icons_css import generate_css; generate_css()"
     - user: {{ app_username }}
     - minute: '*/5'

salt/salt/scripts/generate-site-icons.sh.jinja2

@@ -1,5 +0,0 @@
-{% from 'common.jinja2' import app_dir -%}
-{% from 'site-icons-spriter.sls' import site_icons_venv_dir, site_icons_data_dir -%}
-#!/bin/bash
-{{ site_icons_venv_dir }}/bin/glue --sprite-namespace= --namespace= --retina --css-template={{ app_dir }}/scripts/site-icons-spriter/css_template.jinja2 {{ site_icons_data_dir }}/site-icons {{ site_icons_data_dir }}/output
-rsync --checksum {{ site_icons_data_dir }}/output/*.png {{ app_dir }}/static/images

salt/salt/site-icons-spriter.sls

@@ -1,41 +0,0 @@
-{% from 'common.jinja2' import app_dir, app_username, python_version %}
-
-{% set site_icons_venv_dir = '/opt/venvs/site-icons-spriter' %}
-{% set site_icons_data_dir = '/var/lib/site-icons-spriter' %}
-
-# Salt seems to use the deprecated pyvenv script, manual for now
-site-icons-venv-setup:
-  cmd.run:
-    - name: python{{ python_version }} -m venv {{ site_icons_venv_dir }}
-    - creates: {{ site_icons_venv_dir }}
-    - require:
-      - pkg: python{{ python_version }}-venv
-
-site-icons-pip-installs:
-  cmd.run:
-    - name: {{ site_icons_venv_dir }}/bin/pip install glue
-    - unless: ls {{ site_icons_venv_dir }}/lib/python{{ python_version }}/site-packages/glue
-
-site-icons-output-placeholder:
-  file.managed:
-    - name: {{ site_icons_data_dir }}/output/site-icons.css
-    - contents: ''
-    - allow_empty: True
-    - makedirs: True
-    - user: {{ app_username }}
-    - group: {{ app_username }}
-    - unless: ls {{ site_icons_data_dir }}/output/site-icons.css
-
-site-icons-input-folder:
-  file.directory:
-    - name: {{ site_icons_data_dir }}/site-icons
-    - user: {{ app_username }}
-    - group: {{ app_username }}
-
-/usr/local/bin/generate-site-icons:
-  file.managed:
-    - source: salt://scripts/generate-site-icons.sh.jinja2
-    - template: jinja
-    - user: root
-    - group: root
-    - mode: 755

salt/salt/top.sls

@@ -19,7 +19,6 @@ base:
     - prometheus.exporters.rabbitmq_exporter
     - prometheus.exporters.redis_exporter
     - consumers
-    - site-icons-spriter
     - boussole
     - webassets
     - cronjobs

tildes/consumers/site_icon_downloader.py

@@ -21,7 +21,7 @@ from tildes.models.scraper import ScraperResult
 class SiteIconDownloader(PgsqlQueueConsumer):
     """Consumer that generates content_metadata for topics."""
 
-    ICON_FOLDER = "/var/lib/site-icons-spriter/site-icons"
+    ICON_FOLDER = "/opt/tildes/static/images/site-icons"
 
     def __init__(self, queue_name: str, routing_keys: Sequence[str]):
         """Initialize the consumer, including the public suffix list."""

tildes/scripts/generate_site_icons_css.py

@@ -0,0 +1,56 @@
+# Copyright (c) 2019 Tildes contributors <code@tildes.net>
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+"""Script to generate CSS related to site icons based on which have been downloaded."""
+
+import os
+import shutil
+import stat
+from tempfile import NamedTemporaryFile
+
+ICON_FOLDER = "/opt/tildes/static/images/site-icons"
+OUTPUT_FILE = "/opt/tildes/static/css/site-icons.css"
+
+CSS_RULE = """
+.topic-icon-{domain} {{
+  background-image: url('/images/site-icons/{filename}');
+  border: 0;
+}}
+"""
+
+
+def _is_output_file_outdated() -> bool:
+    """Return whether the output file needs an update yet."""
+    # check if any icon files have a modified time higher than the output file's
+    try:
+        output_file_modified = os.stat(OUTPUT_FILE).st_mtime
+    except FileNotFoundError:
+        return True
+
+    for entry in os.scandir(ICON_FOLDER):
+        if entry.stat().st_mtime > output_file_modified:
+            return True
+
+    return False
+
+
+def generate_css() -> None:
+    """Generate the CSS file for site icons and replace the old one."""
+    if not _is_output_file_outdated():
+        return
+
+    with NamedTemporaryFile(mode="w") as temp_file:
+        for filename in os.listdir(ICON_FOLDER):
+            split_filename = filename.split(".")
+            if len(split_filename) < 2 or split_filename[1] != "png":
+                continue
+
+            temp_file.write(
+                CSS_RULE.format(domain=split_filename[0], filename=filename)
+            )
+
+        temp_file.flush()
+        shutil.copy(temp_file.name, OUTPUT_FILE)
+
+    # set file permissions to 644 (rw-r--r--)
+    os.chmod(OUTPUT_FILE, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)

tildes/scripts/site-icons-spriter/css_template.jinja2

@@ -1,25 +0,0 @@
-{# Copyright (c) 2018 Tildes contributors <code@tildes.net> #}
-{# SPDX-License-Identifier: AGPL-3.0-or-later #}
-
-{% for r, ratio in ratios.items() %}
-{% if ratio.ratio == 1.0 %}
-.topic-icon {
-  background-image: url('/images/{{ ratio.sprite_path }}?{{ hash }}');
-  background-size: 0 0;
-}
-{% else %}
-@media screen and (min-device-pixel-ratio: {{ ratio.ratio }}), screen and (min-resolution: {{ ratio.ratio }}dppx) {
-  .topic-icon {
-    background-image: url('/images/{{ ratio.sprite_path }}?{{ hash }}');
-  }
-}
-{% endif %}
-{% endfor %}
-
-{% for image in images %}
-.topic-icon-{{ image.label }} {
-  background-position: {{ image.x ~ ('px' if image.x) }} {{ image.y ~ ('px' if image.y) }};
-  background-size: {{ width }}px {{ height }}px;
-  border: 0;
-}
-{% endfor %}

tildes/scss/modules/_topic.scss

@@ -85,6 +85,7 @@
   margin-top: 2px;
   margin-right: 0.2rem;
   border: 1px dashed;
+  background-size: 16px 16px;
 }
 
 .topic-log {

tildes/static/images/site-icons/README

@@ -0,0 +1 @@
+This folder holds the site-icons (favicons) downloaded by the "site_icon_downloader" consumer.

tildes/tildes/__init__.py

@@ -40,7 +40,6 @@ def main(global_config: Dict[str, str], **settings: str) -> PrefixMiddleware:
     config.add_webasset("javascript", Bundle(output="js/tildes.js"))
     config.add_webasset("javascript-third-party", Bundle(output="js/third_party.js"))
     config.add_webasset("css", Bundle(output="css/tildes.css"))
-    config.add_webasset("site-icons-css", Bundle(output="css/site-icons.css"))
 
     config.scan("tildes.views")
 

tildes/tildes/templates/base.jinja2

@@ -33,9 +33,7 @@
   {% assets "css" %}
   <link rel="stylesheet" href="{{ ASSET_URL }}">
   {% endassets %}
-  {% assets "site-icons-css" %}
-  <link rel="stylesheet" href="{{ ASSET_URL }}">
-  {% endassets %}
+  <link rel="stylesheet" href="/css/site-icons.css">
 
   {# Favicons and other data for "pinning" the site on various platforms #}
   <link rel="icon" type="image/png" href="/favicon-16x16.png" sizes="16x16">

tildes/webassets.yaml

@@ -21,8 +21,3 @@ bundles:
       - css/spectre-0.5.1/spectre.css
       - css/styles.css
     output: css/tildes.css
-  site-icons-css:
-    merge: false
-    contents:
-      - /var/lib/site-icons-spriter/output/site-icons.css
-    output: css/site-icons.css