Fix regression with distutils MANIFEST handing (#11104, #8688).

The changed behavior of sdist in 2.7 broke packaging for projects that
wanted to use a manually-maintained MANIFEST file (instead of having a
MANIFEST.in template and letting distutils generate the MANIFEST).

The fixes that were committed for #8688 (d29399100973 by Tarek and
f7639dcdffc3 by me) did not fix all issues exposed in the bug report,
and also added one problem: the MANIFEST file format gained comments,
but the read_manifest method was not updated to handle (i.e. ignore)
them.  This changeset should fix everything; the tests have been
expanded and I successfully tested with Mercurial, which suffered from
this regression.

I have grouped the versionchanged directives for these bugs in one place
and added micro version numbers to help users know the quirks of the
exact version they’re using.  I also removed a stanza in the docs that
was forgotten in Tarek’s first changeset.

Initial report, thorough diagnosis and patch by John Dennis, further
work on the patch by Stephen Thorne, and a few edits and additions by
me.

2 files changed, 63 insertions, 28 deletions

diff --git a/Lib/distutils/command/sdist.py b/Lib/distutils/command/sdist.py
index cf8982b..75950f3 100644
--- a/Lib/distutils/command/sdist.py
+++ b/Lib/distutils/command/sdist.py
@@ -182,14 +182,20 @@ class sdist(Command):
         reading the manifest, or just using the default file set -- it all
         depends on the user's options.
         """
-        # new behavior:
+        # new behavior when using a template:
         # the file list is recalculated everytime because
         # even if MANIFEST.in or setup.py are not changed
         # the user might have added some files in the tree that
         # need to be included.
         #
-        #  This makes --force the default and only behavior.
+        #  This makes --force the default and only behavior with templates.
         template_exists = os.path.isfile(self.template)
+        if not template_exists and self._manifest_is_not_generated():
+            self.read_manifest()
+            self.filelist.sort()
+            self.filelist.remove_duplicates()
+            return
+
         if not template_exists:
             self.warn(("manifest template '%s' does not exist " +
                         "(using default file list)") %
@@ -352,23 +358,28 @@ class sdist(Command):
         by 'add_defaults()' and 'read_template()') to the manifest file
         named by 'self.manifest'.
         """
-        if os.path.isfile(self.manifest):
-            fp = open(self.manifest)
-            try:
-                first_line = fp.readline()
-            finally:
-                fp.close()
-
-            if first_line != '# file GENERATED by distutils, do NOT edit\n':
-                log.info("not writing to manually maintained "
-                         "manifest file '%s'" % self.manifest)
-                return
+        if self._manifest_is_not_generated():
+            log.info("not writing to manually maintained "
+                     "manifest file '%s'" % self.manifest)
+            return
 
         content = self.filelist.files[:]
         content.insert(0, '# file GENERATED by distutils, do NOT edit')
         self.execute(file_util.write_file, (self.manifest, content),
                      "writing manifest file '%s'" % self.manifest)
 
+    def _manifest_is_not_generated(self):
+        # check for special comment used in 2.7.1 and higher
+        if not os.path.isfile(self.manifest):
+            return False
+
+        fp = open(self.manifest, 'rU')
+        try:
+            first_line = fp.readline()
+        finally:
+            fp.close()
+        return first_line != '# file GENERATED by distutils, do NOT edit\n'
+
     def read_manifest(self):
         """Read the manifest file (named by 'self.manifest') and use it to
         fill in 'self.filelist', the list of files to include in the source
@@ -376,12 +387,11 @@ class sdist(Command):
         """
         log.info("reading manifest file '%s'", self.manifest)
         manifest = open(self.manifest)
-        while 1:
-            line = manifest.readline()
-            if line == '':              # end of file
-                break
-            if line[-1] == '\n':
-                line = line[0:-1]
+        for line in manifest:
+            # ignore comments and blank lines
+            line = line.strip()
+            if line.startswith('#') or not line:
+                continue
             self.filelist.append(line)
         manifest.close()
 
diff --git a/Lib/distutils/tests/test_sdist.py b/Lib/distutils/tests/test_sdist.py
index 61f9c1f..8da6fe4 100644
--- a/Lib/distutils/tests/test_sdist.py
+++ b/Lib/distutils/tests/test_sdist.py
@@ -1,9 +1,11 @@
 """Tests for distutils.command.sdist."""
 import os
+import tarfile
 import unittest
-import shutil
+import warnings
 import zipfile
-import tarfile
+from os.path import join
+from textwrap import dedent
 
 # zlib is not used here, but if it's not available
 # the tests that use zipfile may fail
@@ -19,19 +21,13 @@ try:
 except ImportError:
     UID_GID_SUPPORT = False
 
-from os.path import join
-import sys
-import tempfile
-import warnings
-
 from test.test_support import captured_stdout, check_warnings, run_unittest
 
 from distutils.command.sdist import sdist, show_formats
 from distutils.core import Distribution
 from distutils.tests.test_config import PyPIRCCommandTestCase
-from distutils.errors import DistutilsExecError, DistutilsOptionError
+from distutils.errors import DistutilsOptionError
 from distutils.spawn import find_executable
-from distutils.tests import support
 from distutils.log import WARN
 from distutils.archive_util import ARCHIVE_FORMATS
 
@@ -405,13 +401,33 @@ class SDistTestCase(PyPIRCCommandTestCase):
         self.assertEqual(manifest[0],
                          '# file GENERATED by distutils, do NOT edit')
 
+    @unittest.skipUnless(zlib, 'requires zlib')
+    def test_manifest_comments(self):
+        # make sure comments don't cause exceptions or wrong includes
+        contents = dedent("""\
+            # bad.py
+            #bad.py
+            good.py
+            """)
+        dist, cmd = self.get_cmd()
+        cmd.ensure_finalized()
+        self.write_file((self.tmp_dir, cmd.manifest), contents)
+        self.write_file((self.tmp_dir, 'good.py'), '# pick me!')
+        self.write_file((self.tmp_dir, 'bad.py'), "# don't pick me!")
+        self.write_file((self.tmp_dir, '#bad.py'), "# don't pick me!")
+        cmd.run()
+        self.assertEqual(cmd.filelist.files, ['good.py'])
+
     @unittest.skipUnless(zlib, "requires zlib")
     def test_manual_manifest(self):
         # check that a MANIFEST without a marker is left alone
         dist, cmd = self.get_cmd()
         cmd.ensure_finalized()
         self.write_file((self.tmp_dir, cmd.manifest), 'README.manual')
+        self.write_file((self.tmp_dir, 'README.manual'),
+                         'This project maintains its MANIFEST file itself.')
         cmd.run()
+        self.assertEqual(cmd.filelist.files, ['README.manual'])
 
         f = open(cmd.manifest)
         try:
@@ -422,6 +438,15 @@ class SDistTestCase(PyPIRCCommandTestCase):
 
         self.assertEqual(manifest, ['README.manual'])
 
+        archive_name = join(self.tmp_dir, 'dist', 'fake-1.0.tar.gz')
+        archive = tarfile.open(archive_name)
+        try:
+            filenames = [tarinfo.name for tarinfo in archive]
+        finally:
+            archive.close()
+        self.assertEqual(sorted(filenames), ['fake-1.0', 'fake-1.0/PKG-INFO',
+                                             'fake-1.0/README.manual'])
+
 def test_suite():
     return unittest.makeSuite(SDistTestCase)