launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/publishing-timestamps-2 into lp:launchpad with lp:~cjwatson/launchpad/inline-release-timestamp as a prerequisite.

Commit message:
Synchronise timestamps of Release and other index files even if not signing the repository.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1033581 in Launchpad itself: "Publisher should set modification times on Releases et al"
  https://bugs.launchpad.net/launchpad/+bug/1033581

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/publishing-timestamps-2/+merge/272508

This is a ridiculously belated follow-up to https://code.launchpad.net/~cjwatson/launchpad/publishing-timestamps/+merge/120148.  We need to synchronise timestamps of Release and other index files (Packages, Sources, etc.) even if not signing the repository, for the same reasons as before.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/publishing-timestamps-2 into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2015-09-26 01:22:34 +0000
+++ lib/lp/archivepublisher/publishing.py	2015-09-26 01:22:34 +0000
@@ -876,16 +876,14 @@
         self._writeReleaseFile(suite, release_file)
         all_files.add("Release")
 
-        # Skip signature if the archive signing key is undefined.
-        if self.archive.signing_key is None:
+        if self.archive.signing_key is not None:
+            # Sign the repository.
+            IArchiveSigningKey(self.archive).signRepository(suite)
+            all_files.add("Release.gpg")
+            all_files.add("InRelease")
+        else:
+            # Skip signature if the archive signing key is undefined.
             self.log.debug("No signing key available, skipping signature.")
-            return
-
-        # Sign the repository.
-        archive_signer = IArchiveSigningKey(self.archive)
-        archive_signer.signRepository(suite)
-        all_files.add("Release.gpg")
-        all_files.add("InRelease")
 
         # Make sure all the timestamps match, to make it easier to insert
         # caching headers on mirrors.

=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2015-09-26 01:22:34 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2015-09-26 01:22:34 +0000
@@ -5,9 +5,9 @@
 
 __metaclass__ = type
 
-
 import bz2
 import crypt
+from functools import partial
 import gzip
 import hashlib
 import os
@@ -1848,6 +1848,30 @@
             self.assertReleaseContentsMatch(
                 release, 'Contents-i386.gz', contents_file.read())
 
+    def testReleaseFileTimestamps(self):
+        # The timestamps of Release and all its entries match.
+        publisher = Publisher(
+            self.logger, self.config, self.disk_pool,
+            self.ubuntutest.main_archive)
+
+        self.getPubSource(filecontent='Hello world')
+
+        publisher.A_publish(False)
+        publisher.C_doFTPArchive(False)
+
+        suite_path = partial(
+            os.path.join, self.config.distsroot, 'breezy-autotest')
+        sources = suite_path('main', 'source', 'Sources')
+        sources_timestamp = os.stat(sources).st_mtime - 60
+        os.utime(sources, (sources_timestamp, sources_timestamp))
+
+        publisher.D_writeReleaseFiles(False)
+
+        release = self.parseRelease(suite_path('Release'))
+        paths = ['Release'] + [entry['name'] for entry in release['md5sum']]
+        timestamps = set(os.stat(suite_path(path)).st_mtime for path in paths)
+        self.assertEqual(1, len(timestamps))
+
     def testCreateSeriesAliasesNoAlias(self):
         """createSeriesAliases has nothing to do by default."""
         publisher = Publisher(
@@ -2202,12 +2226,25 @@
         cprov = getUtility(IPersonSet).getByName('cprov')
         self.assertTrue(cprov.archive.signing_key is None)
 
+        self.setupPublisher(cprov.archive)
+        self.archive_publisher._syncTimestamps = FakeMethod()
+
         self._publishArchive(cprov.archive)
 
         # Release file exist but it doesn't have any signature.
         self.assertTrue(os.path.exists(self.release_file_path))
         self.assertFalse(os.path.exists(self.release_file_signature_path))
 
+        # The publisher synchronises the timestamp of the Release file with
+        # any other files, but does not do anything to Release.gpg or
+        # InRelease.
+        self.assertEqual(1, self.archive_publisher._syncTimestamps.call_count)
+        sync_args = self.archive_publisher._syncTimestamps.extract_args()[0]
+        self.assertEqual(self.distroseries.name, sync_args[0])
+        self.assertIn('Release', sync_args[1])
+        self.assertNotIn('Release.gpg', sync_args[1])
+        self.assertNotIn('InRelease', sync_args[1])
+
     def testRepositorySignatureWithSigningKey(self):
         """Check publisher behaviour when signing repositories.