From a3e072bb89b2ef34d49458c78c1c67ee65e5d2a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Araujo?= Date: Sat, 30 Jul 2011 21:34:04 +0200 Subject: =?UTF-8?q?Let=20=E2=80=9Cmake=20patchcheck=E2=80=9D=20work=20for?= =?UTF-8?q?=20out-of-dir=20builds=20(#9860)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Tools/scripts/patchcheck.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/Tools/scripts/patchcheck.py b/Tools/scripts/patchcheck.py index 13b8ba4..9eb367d 100755 --- a/Tools/scripts/patchcheck.py +++ b/Tools/scripts/patchcheck.py @@ -4,11 +4,15 @@ import sys import shutil import os.path import subprocess +import sysconfig import reindent import untabify +SRCDIR = sysconfig.get_config_var('srcdir') + + def n_files_str(count): """Return 'N file(s)' with the proper plurality on 'file'.""" return "{} file{}".format(count, "s" if count != 1 else "") @@ -36,7 +40,7 @@ def status(message, modal=False, info=None): info=lambda x: n_files_str(len(x))) def changed_files(): """Get the list of changed or added files from the VCS.""" - if os.path.isdir('.hg'): + if os.path.isdir(os.path.join(SRCDIR, '.hg')): vcs = 'hg' cmd = 'hg status --added --modified --no-status' elif os.path.isdir('.svn'): @@ -75,7 +79,7 @@ def normalize_whitespace(file_paths): reindent.makebackup = False # No need to create backups. fixed = [] for path in (x for x in file_paths if x.endswith('.py')): - if reindent.check(path): + if reindent.check(os.path.join(SRCDIR, path)): fixed.append(path) return fixed @@ -85,10 +89,11 @@ def normalize_c_whitespace(file_paths): """Report if any C files """ fixed = [] for path in file_paths: - with open(path, 'r') as f: + abspath = os.path.join(SRCDIR, path) + with open(abspath, 'r') as f: if '\t' not in f.read(): continue - untabify.process(path, 8, verbose=False) + untabify.process(abspath, 8, verbose=False) fixed.append(path) return fixed @@ -99,13 +104,14 @@ ws_re = re.compile(br'\s+(\r?\n)$') def normalize_docs_whitespace(file_paths): fixed = [] for path in file_paths: + abspath = os.path.join(SRCDIR, path) try: - with open(path, 'rb') as f: + with open(abspath, 'rb') as f: lines = f.readlines() new_lines = [ws_re.sub(br'\1', line) for line in lines] if new_lines != lines: - shutil.copyfile(path, path + '.bak') - with open(path, 'wb') as f: + shutil.copyfile(abspath, abspath + '.bak') + with open(abspath, 'wb') as f: f.writelines(new_lines) fixed.append(path) except Exception as err: -- cgit v0.12 From ab7c1b3f1137f7479aa3c1aee82a353ee214cfe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Araujo?= Date: Sun, 31 Jul 2011 04:06:12 +0200 Subject: Fix regression with distutils MANIFEST handing (#11104, #8688). MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 25 +++++++++++++------- Lib/distutils/command/sdist.py | 48 +++++++++++++++++++++++---------------- Lib/distutils/tests/test_sdist.py | 39 ++++++++++++++++++++++++++----- Misc/ACKS | 2 ++ Misc/NEWS | 3 +++ 5 files changed, 84 insertions(+), 33 deletions(-) diff --git a/Doc/distutils/sourcedist.rst b/Doc/distutils/sourcedist.rst index 15d0baf..1666436 100644 --- a/Doc/distutils/sourcedist.rst +++ b/Doc/distutils/sourcedist.rst @@ -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`. diff --git a/Lib/distutils/command/sdist.py b/Lib/distutils/command/sdist.py index 48cb26b..21ea61d 100644 --- a/Lib/distutils/command/sdist.py +++ b/Lib/distutils/command/sdist.py @@ -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() diff --git a/Lib/distutils/tests/test_sdist.py b/Lib/distutils/tests/test_sdist.py index c7dd47f..440af98 100644 --- a/Lib/distutils/tests/test_sdist.py +++ b/Lib/distutils/tests/test_sdist.py @@ -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) diff --git a/Misc/ACKS b/Misc/ACKS index db451d7..d345e54 100644 --- 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 diff --git a/Misc/NEWS b/Misc/NEWS index ef432e0..aab5e1a 100644 --- 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. -- cgit v0.12 From 548c054fb70c504150ec8bafa4503d6b4e74e535 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Araujo?= Date: Sun, 31 Jul 2011 17:58:46 +0200 Subject: Stop trying to write into the stdlib during lib2to3 tests (#12331). This prevents tests from failing when run from a Python installed in a read-only directory. --- Lib/lib2to3/tests/test_refactor.py | 18 +++++++++++------- Misc/NEWS | 3 +++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/Lib/lib2to3/tests/test_refactor.py b/Lib/lib2to3/tests/test_refactor.py index 73122d8..54edeb4 100644 --- a/Lib/lib2to3/tests/test_refactor.py +++ b/Lib/lib2to3/tests/test_refactor.py @@ -177,22 +177,26 @@ from __future__ import print_function""" self.assertEqual(results, expected) def check_file_refactoring(self, test_file, fixers=_2TO3_FIXERS): + tmpdir = tempfile.mkdtemp(prefix="2to3-test_refactor") + self.addCleanup(shutil.rmtree, tmpdir) + # make a copy of the tested file that we can write to + shutil.copy(test_file, tmpdir) + test_file = os.path.join(tmpdir, os.path.basename(test_file)) + os.chmod(test_file, 0o644) + def read_file(): with open(test_file, "rb") as fp: return fp.read() + old_contents = read_file() rt = self.rt(fixers=fixers) rt.refactor_file(test_file) self.assertEqual(old_contents, read_file()) - try: - rt.refactor_file(test_file, True) - new_contents = read_file() - self.assertNotEqual(old_contents, new_contents) - finally: - with open(test_file, "wb") as fp: - fp.write(old_contents) + rt.refactor_file(test_file, True) + new_contents = read_file() + self.assertNotEqual(old_contents, new_contents) return new_contents def test_refactor_file(self): diff --git a/Misc/NEWS b/Misc/NEWS index aab5e1a..e1060f6 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -140,6 +140,9 @@ Tools/Demos Tests ----- +- Issue #12331: The test suite for lib2to3 can now run from an installed + Python. + - Issue #12626: In regrtest, allow to filter tests using a glob filter with the ``-m`` (or ``--match``) option. This works with all test cases using the unittest module. This is useful with long test suites -- cgit v0.12 From 56ec5fe950da9903ec9fc614e8d0a9b4b7f6f95f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Araujo?= Date: Sun, 31 Jul 2011 18:41:25 +0200 Subject: Small cleanup --- Tools/scripts/patchcheck.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Tools/scripts/patchcheck.py b/Tools/scripts/patchcheck.py index b01f77c..204407e 100755 --- a/Tools/scripts/patchcheck.py +++ b/Tools/scripts/patchcheck.py @@ -39,18 +39,13 @@ def status(message, modal=False, info=None): @status("Getting the list of files that have been added/changed", info=lambda x: n_files_str(len(x))) def changed_files(): - """Get the list of changed or added files from the VCS.""" - if os.path.isdir(os.path.join(SRCDIR, '.hg')): - cmd = 'hg status --added --modified --no-status' - else: + """Get the list of changed or added files from Mercurial.""" + if not os.path.isdir(os.path.join(SRCDIR, '.hg')): sys.exit('need a checkout to get modified files') - st = subprocess.Popen(cmd.split(), stdout=subprocess.PIPE) - try: - st.wait() + cmd = 'hg status --added --modified --no-status' + with subprocess.Popen(cmd.split(), stdout=subprocess.PIPE) as st: return [x.decode().rstrip() for x in st.stdout] - finally: - st.stdout.close() def report_modified_files(file_paths): -- cgit v0.12 From b85b966de6aa1b5b5584c294656a7f574f492cc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Araujo?= Date: Sun, 31 Jul 2011 20:47:47 +0200 Subject: Stop trying to write into the stdlib during packaging tests (#12331). This prevents tests from failing when run from a Python installed in a read-only directory. The code is a bit uglier; shutil.copytree calls copystat on directories behind our back, so I had to add an os.walk with os.chmod (*and* os.path.join!) calls. shutil, I am disappoint. This changeset is dedicated to the hundreds of neurons that were lost while I was debugging this on an otherwise fine afternoon. --- Lib/packaging/tests/test_database.py | 54 +++++++++++++++++++++--------------- Misc/NEWS | 3 ++ 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/Lib/packaging/tests/test_database.py b/Lib/packaging/tests/test_database.py index 119fa23..9baf396 100644 --- a/Lib/packaging/tests/test_database.py +++ b/Lib/packaging/tests/test_database.py @@ -39,19 +39,39 @@ def record_pieces(file): return [path, digest, size] -class CommonDistributionTests: - """Mixin used to test the interface common to both Distribution classes. - - Derived classes define cls, sample_dist, dirs and records. These - attributes are used in test methods. See source code for details. - """ +class FakeDistsMixin: def setUp(self): - super(CommonDistributionTests, self).setUp() + super(FakeDistsMixin, self).setUp() self.addCleanup(enable_cache) disable_cache() - self.fake_dists_path = os.path.abspath( + + # make a copy that we can write into for our fake installed + # distributions + tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmpdir) + self.fake_dists_path = os.path.join(tmpdir, 'fake_dists') + fake_dists_src = os.path.abspath( os.path.join(os.path.dirname(__file__), 'fake_dists')) + shutil.copytree(fake_dists_src, self.fake_dists_path) + # XXX ugly workaround: revert copystat calls done by shutil behind our + # back (to avoid getting a read-only copy of a read-only file). we + # could pass a custom copy_function to change the mode of files, but + # shutil gives no control over the mode of directories :( + for root, dirs, files in os.walk(self.fake_dists_path): + os.chmod(root, 0o755) + for f in files: + os.chmod(os.path.join(root, f), 0o644) + for d in dirs: + os.chmod(os.path.join(root, d), 0o755) + + +class CommonDistributionTests(FakeDistsMixin): + """Mixin used to test the interface common to both Distribution classes. + + Derived classes define cls, sample_dist, dirs and records. These + attributes are used in test methods. See source code for details. + """ def test_instantiation(self): # check that useful attributes are here @@ -110,6 +130,7 @@ class TestDistribution(CommonDistributionTests, unittest.TestCase): self.records = {} for distinfo_dir in self.dirs: + record_file = os.path.join(distinfo_dir, 'RECORD') with open(record_file, 'w') as file: record_writer = csv.writer( @@ -138,12 +159,6 @@ class TestDistribution(CommonDistributionTests, unittest.TestCase): record_data[path] = md5_, size self.records[distinfo_dir] = record_data - def tearDown(self): - for distinfo_dir in self.dirs: - record_file = os.path.join(distinfo_dir, 'RECORD') - open(record_file, 'wb').close() - super(TestDistribution, self).tearDown() - def test_instantiation(self): super(TestDistribution, self).test_instantiation() self.assertIsInstance(self.dist.requested, bool) @@ -252,20 +267,13 @@ class TestEggInfoDistribution(CommonDistributionTests, class TestDatabase(support.LoggingCatcher, + FakeDistsMixin, unittest.TestCase): def setUp(self): super(TestDatabase, self).setUp() - disable_cache() - # Setup the path environment with our fake distributions - current_path = os.path.abspath(os.path.dirname(__file__)) - self.fake_dists_path = os.path.join(current_path, 'fake_dists') sys.path.insert(0, self.fake_dists_path) - - def tearDown(self): - sys.path.remove(self.fake_dists_path) - enable_cache() - super(TestDatabase, self).tearDown() + self.addCleanup(sys.path.remove, self.fake_dists_path) def test_distinfo_dirname(self): # Given a name and a version, we expect the distinfo_dirname function diff --git a/Misc/NEWS b/Misc/NEWS index 3f5aa29..afe4894 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -1147,6 +1147,9 @@ Extension Modules Tests ----- +- Issue #12331: The test suite for the packaging module can now run from an + installed Python. + - Issue #12331: The test suite for lib2to3 can now run from an installed Python. -- cgit v0.12