From f4c4973dc04af32ed6a49b73cd0f5f3482ef35a5 Mon Sep 17 00:00:00 2001 From: Deimos Date: Tue, 12 Mar 2019 13:06:58 -0600 Subject: [PATCH] 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 | 4 +- salt/salt/cronjobs.sls | 4 +- .../scripts/generate-site-icons.sh.jinja2 | 5 -- salt/salt/site-icons-spriter.sls | 41 -------------- salt/salt/top.sls | 1 - tildes/consumers/site_icon_downloader.py | 2 +- tildes/scripts/generate_site_icons_css.py | 56 +++++++++++++++++++ .../site-icons-spriter/css_template.jinja2 | 25 --------- tildes/scss/modules/_topic.scss | 1 + tildes/static/images/site-icons/README | 1 + tildes/tildes/__init__.py | 1 - tildes/tildes/templates/base.jinja2 | 4 +- tildes/webassets.yaml | 5 -- 13 files changed, 64 insertions(+), 86 deletions(-) delete mode 100644 salt/salt/scripts/generate-site-icons.sh.jinja2 delete mode 100644 salt/salt/site-icons-spriter.sls create mode 100644 tildes/scripts/generate_site_icons_css.py delete mode 100644 tildes/scripts/site-icons-spriter/css_template.jinja2 create mode 100644 tildes/static/images/site-icons/README diff --git a/.gitignore b/.gitignore index 906dbbe..877913c 100644 --- a/.gitignore +++ b/.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 diff --git a/salt/salt/cronjobs.sls b/salt/salt/cronjobs.sls index 86c9a8a..05e4300 100644 --- a/salt/salt/cronjobs.sls +++ b/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' diff --git a/salt/salt/scripts/generate-site-icons.sh.jinja2 b/salt/salt/scripts/generate-site-icons.sh.jinja2 deleted file mode 100644 index b8db625..0000000 --- a/salt/salt/scripts/generate-site-icons.sh.jinja2 +++ /dev/null @@ -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 diff --git a/salt/salt/site-icons-spriter.sls b/salt/salt/site-icons-spriter.sls deleted file mode 100644 index ddd013c..0000000 --- a/salt/salt/site-icons-spriter.sls +++ /dev/null @@ -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 diff --git a/salt/salt/top.sls b/salt/salt/top.sls index ccfb711..ba77e1c 100644 --- a/salt/salt/top.sls +++ b/salt/salt/top.sls @@ -19,7 +19,6 @@ base: - prometheus.exporters.rabbitmq_exporter - prometheus.exporters.redis_exporter - consumers - - site-icons-spriter - boussole - webassets - cronjobs diff --git a/tildes/consumers/site_icon_downloader.py b/tildes/consumers/site_icon_downloader.py index 127f30e..d20a331 100644 --- a/tildes/consumers/site_icon_downloader.py +++ b/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.""" diff --git a/tildes/scripts/generate_site_icons_css.py b/tildes/scripts/generate_site_icons_css.py new file mode 100644 index 0000000..e84333e --- /dev/null +++ b/tildes/scripts/generate_site_icons_css.py @@ -0,0 +1,56 @@ +# Copyright (c) 2019 Tildes contributors +# 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) diff --git a/tildes/scripts/site-icons-spriter/css_template.jinja2 b/tildes/scripts/site-icons-spriter/css_template.jinja2 deleted file mode 100644 index aafbb3f..0000000 --- a/tildes/scripts/site-icons-spriter/css_template.jinja2 +++ /dev/null @@ -1,25 +0,0 @@ -{# Copyright (c) 2018 Tildes contributors #} -{# 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 %} diff --git a/tildes/scss/modules/_topic.scss b/tildes/scss/modules/_topic.scss index 3c1647d..f6ac541 100644 --- a/tildes/scss/modules/_topic.scss +++ b/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 { diff --git a/tildes/static/images/site-icons/README b/tildes/static/images/site-icons/README new file mode 100644 index 0000000..2f6691a --- /dev/null +++ b/tildes/static/images/site-icons/README @@ -0,0 +1 @@ +This folder holds the site-icons (favicons) downloaded by the "site_icon_downloader" consumer. diff --git a/tildes/tildes/__init__.py b/tildes/tildes/__init__.py index a64341b..6023928 100644 --- a/tildes/tildes/__init__.py +++ b/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") diff --git a/tildes/tildes/templates/base.jinja2 b/tildes/tildes/templates/base.jinja2 index 6ef8f4c..99d3ded 100644 --- a/tildes/tildes/templates/base.jinja2 +++ b/tildes/tildes/templates/base.jinja2 @@ -33,9 +33,7 @@ {% assets "css" %} {% endassets %} - {% assets "site-icons-css" %} - - {% endassets %} + {# Favicons and other data for "pinning" the site on various platforms #} diff --git a/tildes/webassets.yaml b/tildes/webassets.yaml index e25f76b..aa5fcda 100644 --- a/tildes/webassets.yaml +++ b/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