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.
diff --git a/Doc/distutils/sourcedist.rst b/Doc/distutils/sourcedist.rst
index fc860de..a9858d0 100644
--- a/Doc/distutils/sourcedist.rst
+++ b/Doc/distutils/sourcedist.rst
@@ -111,12 +111,22 @@
:file:`MANIFEST`, you must specify everything: the default set of files
described above does not apply in this case.
-.. versionadded:: 2.7
+.. versionchanged:: 2.7
+ 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:: 2.7.1
:file:`MANIFEST` files start with a comment indicating they are generated.
Files without this comment are not overwritten or removed.
+.. versionchanged:: 2.7.3
+ :command:`sdist` will read a :file:`MANIFEST` file if no :file:`MANIFEST.in`
+ exists, like it did before 2.7.
+
See :ref:`manifest_template` section for a syntax reference.
+
.. _manifest-options:
Manifest-related options
@@ -124,16 +134,16 @@
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
-* if either :file:`MANIFEST.in` or the setup script (:file:`setup.py`) are more
- recent than :file:`MANIFEST`, recreate :file:`MANIFEST` by reading
- :file:`MANIFEST.in`
-
* use the list of files now in :file:`MANIFEST` (either just generated or read
in) to create the source distribution archive(s)
@@ -271,8 +281,3 @@
``a-z``, ``a-zA-Z``, ``a-f0-9_.``). The definition of "regular filename
character" is platform-specific: on Unix it is anything except slash; on Windows
anything except backslash or colon.
-
-.. versionchanged:: 2.7
- 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`.
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 @@
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 @@
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 @@
"""
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 unittest
-import shutil
-import zipfile
import tarfile
+import unittest
+import warnings
+import zipfile
+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 @@
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 @@
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 @@
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)
diff --git a/Misc/ACKS b/Misc/ACKS
index bfbbf69..ff0a29a 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -194,6 +194,7 @@
Vincent Delft
Arnaud Delobelle
Erik Demaine
+John Dennis
Roger Dev
Raghuram Devarakonda
Catherine Devlin
@@ -813,6 +814,7 @@
Tobias Thelen
James Thomas
Robin Thomas
+Stephen Thorne
Eric Tiedemann
Tracy Tims
Oren Tirosh
diff --git a/Misc/NEWS b/Misc/NEWS
index fc4dfe9..0aeddf4 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -37,6 +37,9 @@
Library
-------
+- Issues #11104, #8688: Fix the behavior of distutils' sdist command with
+ manually-maintained MANIFEST files.
+
- Issue #8887: "pydoc somebuiltin.somemethod" (or help('somebuiltin.somemethod')
in Python code) now finds the doc of the method.