launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-contents-header into lp:launchpad.

Commit message:
Remove headers from Contents files.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1638219 in Launchpad itself: "launchpad: remove headers from Contents files"
  https://bugs.launchpad.net/launchpad/+bug/1638219

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-contents-header/+merge/309750

Primarily machine-readable files are no place for documentation.  This follows a change to dak (https://anonscm.debian.org/cgit/mirror/dak.git/commit/?id=33c8477957525ae6b929a5b0e34eb6ea1ff85c89).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-contents-header into lp:launchpad.

=== removed file 'cronscripts/publishing/gen-contents/Contents.top'
--- cronscripts/publishing/gen-contents/Contents.top	2011-04-18 14:03:06 +0000
+++ cronscripts/publishing/gen-contents/Contents.top	1970-01-01 00:00:00 +0000
@@ -1,32 +0,0 @@
-This file maps each file available in the %(distrotitle)s
-system to the package from which it originates.  It includes packages
-from the DIST distribution for the ARCH architecture.
-
-You can use this list to determine which package contains a specific
-file, or whether or not a specific file is available.  The list is
-updated weekly, each architecture on a different day.
-
-When a file is contained in more than one package, all packages are
-listed.  When a directory is contained in more than one package, only
-the first is listed.
-
-The best way to search quickly for a file is with the Unix `grep'
-utility, as in `grep <regular expression> CONTENTS':
-
- $ grep nose Contents
- etc/nosendfile                                          net/sendfile
- usr/X11R6/bin/noseguy                                   x11/xscreensaver
- usr/X11R6/man/man1/noseguy.1x.gz                        x11/xscreensaver
- usr/doc/examples/ucbmpeg/mpeg_encode/nosearch.param     graphics/ucbmpeg
- usr/lib/cfengine/bin/noseyparker                        admin/cfengine
-
-This list contains files in all packages, even though not all of the
-packages are installed on an actual system at once.  If you want to
-find out which packages on an installed Debian system provide a
-particular file, you can use `dpkg --search <filename>':
-
- $ dpkg --search /usr/bin/dselect
- dpkg: /usr/bin/dselect
-
-
-FILE                                                    LOCATION

=== modified file 'cronscripts/publishing/gen-contents/apt_conf_header.template'
--- cronscripts/publishing/gen-contents/apt_conf_header.template	2014-01-17 02:03:41 +0000
+++ cronscripts/publishing/gen-contents/apt_conf_header.template	2016-11-01 12:26:30 +0000
@@ -15,8 +15,6 @@
 
 TreeDefault
 {
-   // Header for Contents file.
    Contents "$(DIST)/Contents-$(ARCH)";
-   Contents::Header "%(content_archive)s/%(distribution)s-misc/Contents.top";
 };
 

=== modified file 'lib/lp/archivepublisher/scripts/generate_contents_files.py'
--- lib/lp/archivepublisher/scripts/generate_contents_files.py	2016-02-05 16:51:12 +0000
+++ lib/lp/archivepublisher/scripts/generate_contents_files.py	2016-11-01 12:26:30 +0000
@@ -205,21 +205,6 @@
         else:
             self.logger.debug("Did not find overrides; not copying.")
 
-    def writeContentsTop(self, distro_name, distro_title):
-        """Write Contents.top file.
-
-        This method won't access the database.
-        """
-        output_filename = os.path.join(
-            self.content_archive, '%s-misc' % distro_name, "Contents.top")
-        parameters = {
-            'distrotitle': distro_title,
-        }
-        output_file = file(output_filename, 'w')
-        text = file(get_template("Contents.top")).read() % parameters
-        output_file.write(text)
-        output_file.close()
-
     def runAptFTPArchive(self, distro_name):
         """Run apt-ftparchive to produce the Contents files.
 
@@ -233,7 +218,7 @@
                 "apt-contents.conf"),
             ])
 
-    def generateContentsFiles(self, override_root, distro_name, distro_title):
+    def generateContentsFiles(self, override_root, distro_name):
         """Generate Contents files.
 
         This method may take a long time to run.
@@ -243,13 +228,10 @@
             evaluated without accessing the database.
         :param distro_name: Copy of `self.distribution.name` that can be
             evaluated without accessing the database.
-        :param distro_title: Copy of `self.distribution.title` that can be
-            evaluated without accessing the database.
         """
         self.logger.debug(
             "Running apt in private tree to generate new contents.")
         self.copyOverrides(override_root)
-        self.writeContentsTop(distro_name, distro_title)
         self.runAptFTPArchive(distro_name)
 
     def updateContentsFile(self, suite, arch):
@@ -309,14 +291,12 @@
 
         overrideroot = self.config.overrideroot
         distro_name = self.distribution.name
-        distro_title = self.distribution.title
 
         # This takes a while.  Ensure that we do it without keeping a
         # database transaction open.
         self.txn.commit()
         with DatabaseBlockedPolicy():
-            self.generateContentsFiles(
-                overrideroot, distro_name, distro_title)
+            self.generateContentsFiles(overrideroot, distro_name)
 
         self.updateContentsFiles(suites)
 

=== modified file 'lib/lp/archivepublisher/tests/test_generate_contents_files.py'
--- lib/lp/archivepublisher/tests/test_generate_contents_files.py	2016-01-13 17:09:34 +0000
+++ lib/lp/archivepublisher/tests/test_generate_contents_files.py	2016-11-01 12:26:30 +0000
@@ -257,20 +257,6 @@
             'Architectures "%s source";' % das.architecturetag,
             apt_contents_conf)
 
-    def test_writeContentsTop(self):
-        # writeContentsTop writes a Contents.top file based on a
-        # standard template, with the distribution's title interpolated.
-        distro = self.makeDistro()
-        script = self.makeScript(distro)
-        content_archive = script.content_archive
-        script.writeContentsTop(distro.name, distro.title)
-
-        contents_top = file(
-            "%s/%s-misc/Contents.top" % (content_archive, distro.name)).read()
-
-        self.assertIn("This file maps", contents_top)
-        self.assertIn(distro.title, contents_top)
-
     def test_setUp_places_content_archive_in_distroroot(self):
         # The contents files are kept in subdirectories of distroroot.
         script = self.makeScript()