update: use ctime/mtime to control _strip_and_copy_image runs

Oftentimes, the file that is copied is stripped, in which case, the file
size is different.  Using a file size check here means it will rerun the
strip and copy every time `fdroid update` is run for any image that needs
to be stripped.  If the source's ctime is newer than the destination, then
the process should run since it is a newly created file.  Even more so with
mtime, since the destination's mtime is reset based on the source's.

fdroidserver/update.py

@@ -800,14 +800,18 @@ def _strip_and_copy_image(in_file, outpath):
     It is not used at all in the F-Droid ecosystem, so its much safer
     just to remove it entirely.
 
-    This uses size+mtime to check for a new file since this process
+    This only uses ctime/mtime to check for a new file since this
-    actually modifies the resulting file to strip out the EXIF.
+    process actually modifies the resulting file to strip out the EXIF.
+    Therefore, whenever the file needs to be stripped, it will have a
+    newer ctime and most likely a different size.  The mtime is copied
+    from the source to the destination, so it can be the same.
 
     outpath can be path to either a file or dir.  The dir that outpath
     refers to must exist before calling this.
 
     Potential source of Python code to strip JPEGs without dependencies:
     http://www.fetidcascade.com/public/minimal_exif_writer.py
+
     """
     logging.debug('copying %s %s', in_file, outpath)
 
@@ -823,12 +827,11 @@ def _strip_and_copy_image(in_file, outpath):
     else:
         out_file = outpath
 
-    if os.path.exists(out_file):
+    if os.path.exists(out_file) and (
-        in_stat = os.stat(in_file)
+        os.path.getmtime(in_file) <= os.path.getmtime(out_file)
-        out_stat = os.stat(out_file)
+        and os.path.getctime(in_file) <= os.path.getctime(out_file)
-        if in_stat.st_size == out_stat.st_size \
+    ):
-           and in_stat.st_mtime == out_stat.st_mtime:
+        return
-            return
 
     extension = common.get_extension(in_file)[1]
     if extension == 'png':

tests/test_update.py

@@ -12,6 +12,7 @@ import shutil
 import string
 import subprocess
 import sys
+import time
 import unittest
 import yaml
 import zipfile
@@ -1369,15 +1370,66 @@ class UpdateTest(unittest.TestCase):
     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')
-        fdroidserver.update._strip_and_copy_image(in_file, out_file)
+        with self.assertLogs(level=logging.DEBUG):
+            fdroidserver.update._strip_and_copy_image(in_file, out_file)
         self.assertTrue(os.path.exists(out_file))
 
     def test_strip_and_copy_image_bad_filename(self):
         in_file = basedir / 'corrupt-featureGraphic.png'
         out_file = os.path.join(self.testdir, 'corrupt-featureGraphic.png')
-        fdroidserver.update._strip_and_copy_image(in_file, out_file)
+        with self.assertLogs(level=logging.DEBUG):
+            fdroidserver.update._strip_and_copy_image(in_file, out_file)
         self.assertFalse(os.path.exists(out_file))
 
+    def test_strip_and_copy_image_unchanged(self):
+        in_file = basedir / 'metadata/info.guardianproject.urzip/en-US/images/icon.png'
+        out_file = os.path.join(self.testdir, 'icon.png')
+        shutil.copy2(in_file, out_file)
+        ctime = os.path.getctime(out_file)
+        delta = 0.01
+        time.sleep(delta)  # ensure reliable failure if file isn't preserved
+        with self.assertLogs(level=logging.DEBUG):  # suppress log output
+            fdroidserver.update._strip_and_copy_image(in_file, out_file)
+        self.assertAlmostEqual(ctime, os.path.getctime(out_file), delta=delta)
+
+    def test_strip_and_copy_image_in_file_ctime_changed(self):
+        out_file = os.path.join(self.testdir, 'icon.png')
+        with open(out_file, 'w') as fp:
+            fp.write('to be replaced')
+        size = os.path.getsize(out_file)
+        delta = 0.01
+        time.sleep(delta)  # ensure reliable failure when testing ctime
+        src_file = basedir / 'metadata/info.guardianproject.urzip/en-US/images/icon.png'
+        in_file = os.path.join(self.testdir, 'in-icon.png')
+        shutil.copy(src_file, in_file)
+        time.sleep(delta)  # ensure reliable failure when testing ctime
+        with self.assertLogs(level=logging.DEBUG):  # suppress log output
+            fdroidserver.update._strip_and_copy_image(in_file, out_file)
+        self.assertNotEqual(size, os.path.getsize(out_file))
+        self.assertNotAlmostEqual(
+            os.path.getctime(in_file), os.path.getctime(out_file), delta=delta
+        )
+        # _strip_and_copy_image syncs mtime from in_file to out_file
+        self.assertAlmostEqual(
+            os.path.getmtime(in_file), os.path.getmtime(out_file), delta=delta
+        )
+
+    def test_strip_and_copy_image_in_file_mtime_changed(self):
+        in_file = basedir / 'metadata/info.guardianproject.urzip/en-US/images/icon.png'
+        out_file = os.path.join(self.testdir, 'icon.png')
+        shutil.copy(in_file, out_file)
+        os.utime(out_file, (12345, 12345))  # set atime/mtime to something old
+        with self.assertLogs(level=logging.DEBUG):  # suppress log output
+            fdroidserver.update._strip_and_copy_image(in_file, out_file)
+        delta = 0.01
+        self.assertNotAlmostEqual(
+            os.path.getctime(in_file), os.path.getctime(out_file), delta=delta
+        )
+        # _strip_and_copy_image syncs mtime from in_file to out_file
+        self.assertAlmostEqual(
+            os.path.getmtime(in_file), os.path.getmtime(out_file), delta=delta
+        )
+
     def test_create_metadata_from_template_empty_keys(self):
         apk = {'packageName': 'rocks.janicerand'}
         with mkdtemp() as tmpdir, TmpCwd(tmpdir):