]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
Fix regression with distutils MANIFEST handing (#11104, #8688).
authorÉric Araujo <merwok@netwok.org>
Sun, 31 Jul 2011 02:06:12 +0000 (04:06 +0200)
committerÉric Araujo <merwok@netwok.org>
Sun, 31 Jul 2011 02:06:12 +0000 (04:06 +0200)
The changed behavior of sdist in 3.1 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 (76643c286b9f by Tarek and
d54da9248ed9 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 the 2.7 version 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.

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.

Doc/distutils/sourcedist.rst
Lib/distutils/command/sdist.py
Lib/distutils/tests/test_sdist.py
Misc/ACKS
Misc/NEWS

index 15d0bafd71aa8ce9ec0378228958ef933028601b..1666436be0b1d15dd74d26d04f15c0a4f1dec2c3 100644 (file)
@@ -103,10 +103,20 @@ per line, regular files (or symlinks to them) only.  If you do supply your own
 :file:`MANIFEST`, you must specify everything: the default set of files
 described above does not apply in this case.
 
-.. versionadded:: 3.1
+.. versionchanged:: 3.1
+   An existing generated :file:`MANIFEST` will be regenerated without
+   :command:`sdist` comparing its modification time to the one of
+   :file:`MANIFEST.in` or :file:`setup.py`.
+
+.. versionchanged:: 3.1.3
    :file:`MANIFEST` files start with a comment indicating they are generated.
    Files without this comment are not overwritten or removed.
 
+.. versionchanged:: 3.2.2
+   :command:`sdist` will read a :file:`MANIFEST` file if no :file:`MANIFEST.in`
+   exists, like it used to do.
+
+
 The manifest template has one command per line, where each command specifies a
 set of files to include or exclude from the source distribution.  For an
 example, again we turn to the Distutils' own manifest template::
@@ -185,8 +195,12 @@ Manifest-related options
 
 The normal course of operations for the :command:`sdist` command is as follows:
 
-* if the manifest file, :file:`MANIFEST` doesn't exist, read :file:`MANIFEST.in`
-  and create the manifest
+* if the manifest file (:file:`MANIFEST` by default) exists and the first line
+  does not have a comment indicating it is generated from :file:`MANIFEST.in`,
+  then it is used as is, unaltered
+
+* if the manifest file doesn't exist or has been previously automatically
+  generated, read :file:`MANIFEST.in` and create the manifest
 
 * if neither :file:`MANIFEST` nor :file:`MANIFEST.in` exist, create a manifest
   with just the default file set
@@ -204,8 +218,3 @@ distribution::
    python setup.py sdist --manifest-only
 
 :option:`-o` is a shortcut for :option:`--manifest-only`.
-
-.. versionchanged:: 3.1
-   An existing generated :file:`MANIFEST` will be regenerated without
-   :command:`sdist` comparing its modification time to the one of
-   :file:`MANIFEST.in` or :file:`setup.py`.
index 48cb26b6ae4143a9a3707d48b629257af2d0540d..21ea61d96ac6cdf204f834d7ea1ba5be858abede 100644 (file)
@@ -174,14 +174,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)") %
@@ -336,23 +342,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 3.1.3 and higher
+        if not os.path.isfile(self.manifest):
+            return False
+
+        fp = open(self.manifest)
+        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
@@ -360,12 +371,11 @@ class sdist(Command):
         """
         log.info("reading manifest file '%s'", self.manifest)
         manifest = open(self.manifest)
-        while True:
-            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()
 
index c7dd47fb5c35de666aa543307a4c8ffd7872ac00..440af9886ccda16153c8a469854d6285388b5763 100644 (file)
@@ -1,21 +1,19 @@
 """Tests for distutils.command.sdist."""
 import os
+import tarfile
 import unittest
-import shutil
+import warnings
 import zipfile
 from os.path import join
-import sys
-import tempfile
-import warnings
+from textwrap import dedent
 
 from 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
 
@@ -346,13 +344,33 @@ class SDistTestCase(PyPIRCCommandTestCase):
         self.assertEqual(manifest[0],
                          '# file GENERATED by distutils, do NOT edit')
 
+    @unittest.skipUnless(ZLIB_SUPPORT, "Need zlib support to run")
+    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_SUPPORT, 'Need zlib support to run')
     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:
@@ -363,6 +381,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)
 
index db451d71904804a565706f9d2f70568e4cad85a8..d345e54ef9d16f3f06f286bcd8d5ce4b05c22f62 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -215,6 +215,7 @@ Ned Deily
 Vincent Delft
 Arnaud Delobelle
 Erik Demaine
+John Dennis
 Roger Dev
 Raghuram Devarakonda
 Caleb Deveraux
@@ -875,6 +876,7 @@ Mikhail Terekhov
 Tobias Thelen
 James Thomas
 Robin Thomas
+Stephen Thorne
 Jeremy Thurgood
 Eric Tiedemann
 July Tikhonov
index ef432e0d29687bb479fbd117d5977ba01c95a855..aab5e1ad7139fc72342be7287917c30087e50ce5 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -41,6 +41,9 @@ Core and Builtins
 Library
 -------
 
+- Issues #11104, #8688: Fix the behavior of distutils' sdist command with
+  manually-maintained MANIFEST files.
+
 - Issue #12464: tempfile.TemporaryDirectory.cleanup() should not follow
   symlinks: fix it. Patch by Petri Lehtinen.