diff --git a/fdroidserver/update.py b/fdroidserver/update.py index e25a5209..ba3b5506 100644 --- a/fdroidserver/update.py +++ b/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 - actually modifies the resulting file to strip out the EXIF. + This only uses ctime/mtime to check for a new file since this + 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): - in_stat = os.stat(in_file) - out_stat = os.stat(out_file) - if in_stat.st_size == out_stat.st_size \ - and in_stat.st_mtime == out_stat.st_mtime: - return + if os.path.exists(out_file) and ( + os.path.getmtime(in_file) <= os.path.getmtime(out_file) + and os.path.getctime(in_file) <= os.path.getctime(out_file) + ): + return extension = common.get_extension(in_file)[1] if extension == 'png': diff --git a/tests/test_update.py b/tests/test_update.py index 3575b546..3d047d75 100755 --- a/tests/test_update.py +++ b/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):