From d26d43014b9202437d276be298539efddbc914e3 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Sat, 25 May 2013 07:58:44 +0200 Subject: Replace setuptools_git with a smarter approach Implement a local egg_info command that only re-generates the SOURCES.txt file when we need to. That is: - If there is no SOURCES.txt, make one - If we have run the sdist command, make one Otherwise, leave well enough alone. Also, skip doing any git processing if SKIP_GIT_SDIST is specified. This should mean that consumers of our tarballs should not get screwed by the need to inject git processing into the sdist. Change-Id: I163b1c153d030e79b120600a2890edeb49e1fa90 --- pbr/extra_files.py | 35 ++++++++++++++ pbr/hooks/commands.py | 1 + pbr/packaging.py | 79 ++++++++++++++++++++++++++++++++ pbr/tests/test_core.py | 27 ++++++++++- pbr/tests/testpackage/git-extra-file.txt | 0 pbr/util.py | 21 ++------- requirements.txt | 1 - 7 files changed, 145 insertions(+), 19 deletions(-) create mode 100644 pbr/extra_files.py create mode 100644 pbr/tests/testpackage/git-extra-file.txt diff --git a/pbr/extra_files.py b/pbr/extra_files.py new file mode 100644 index 0000000..a72db0c --- /dev/null +++ b/pbr/extra_files.py @@ -0,0 +1,35 @@ +# Copyright (c) 2013 Hewlett-Packard Development Company, L.P. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from distutils import errors +import os + +_extra_files = [] + + +def get_extra_files(): + global _extra_files + return _extra_files + + +def set_extra_files(extra_files): + # Let's do a sanity check + for filename in extra_files: + if not os.path.exists(filename): + raise errors.DistutilsFileError( + '%s from the extra_files option in setup.cfg does not ' + 'exist' % filename) + global _extra_files + _extra_files[:] = extra_files[:] diff --git a/pbr/hooks/commands.py b/pbr/hooks/commands.py index 0171d0d..b4206ed 100644 --- a/pbr/hooks/commands.py +++ b/pbr/hooks/commands.py @@ -39,6 +39,7 @@ class CommandsConfig(base.BaseConfig): self.commands = "%s\n%s" % (self.commands, command) def hook(self): + self.add_command('pbr.packaging.LocalEggInfo') self.add_command('pbr.packaging.LocalSDist') self.add_command('pbr.packaging.LocalInstallScripts') if os.name != 'nt': diff --git a/pbr/packaging.py b/pbr/packaging.py index 763ba45..310ad39 100644 --- a/pbr/packaging.py +++ b/pbr/packaging.py @@ -31,6 +31,7 @@ import distutils.errors from distutils import log import pkg_resources from setuptools.command import easy_install +from setuptools.command import egg_info from setuptools.command import install from setuptools.command import install_scripts from setuptools.command import sdist @@ -40,6 +41,8 @@ try: except ImportError: import io +from pbr import extra_files + log.set_verbosity(log.INFO) TRUE_VALUES = ('true', '1', 'yes') REQUIREMENTS_FILES = ('requirements.txt', 'tools/pip-requires') @@ -305,6 +308,23 @@ def generate_authors(git_dir=None, dest_dir='.', option_dict=dict()): .encode('utf-8')) +def _find_git_files(dirname='', git_dir=None): + """Behave like a file finder entrypoint plugin. + + We don't actually use the entrypoints system for this because it runs + at absurd times. We only want to do this when we are building an sdist. + """ + file_list = [] + if git_dir is None: + git_dir = _get_git_directory() + if git_dir: + log.info("[pbr] In git context, generating filelist from git") + git_ls_cmd = "git --git-dir=%s ls-files -z" % git_dir + file_list = _run_shell_command(git_ls_cmd) + file_list = file_list.split(b'\x00'.decode('utf-8')) + return [f for f in file_list if f] + + _rst_template = """%(heading)s %(underline)s @@ -509,6 +529,65 @@ class LocalInstallScripts(install_scripts.install_scripts): self.write_script(*args) +class LocalManifestMaker(egg_info.manifest_maker): + """Add any files that are in git and some standard sensible files.""" + + def _add_pbr_defaults(self): + for template_line in [ + 'include AUTHORS', + 'include ChangeLog', + 'exclude .gitignore', + 'exclude .gitreview', + 'global-exclude *.pyc' + ]: + self.filelist.process_template_line(template_line) + + def add_defaults(self): + option_dict = self.distribution.get_option_dict('pbr') + + sdist.sdist.add_defaults(self) + self.filelist.append(self.template) + self.filelist.append(self.manifest) + self.filelist.extend(extra_files.get_extra_files()) + should_skip = get_boolean_option(option_dict, 'skip_git_sdist', + 'SKIP_GIT_SDIST') + if not should_skip: + rcfiles = _find_git_files() + if rcfiles: + self.filelist.extend(rcfiles) + elif os.path.exists(self.manifest): + self.read_manifest() + ei_cmd = self.get_finalized_command('egg_info') + self._add_pbr_defaults() + self.filelist.include_pattern("*", prefix=ei_cmd.egg_info) + + +class LocalEggInfo(egg_info.egg_info): + """Override the egg_info command to regenerate SOURCES.txt sensibly.""" + + command_name = 'egg_info' + + def find_sources(self): + """Generate SOURCES.txt only if there isn't one already. + + If we are in an sdist command, then we always want to update + SOURCES.txt. If we are not in an sdist command, then it doesn't + matter one flip, and is actually destructive. + """ + manifest_filename = os.path.join(self.egg_info, "SOURCES.txt") + if not os.path.exists(manifest_filename) or 'sdist' in sys.argv: + log.info("[pbr] Processing SOURCES.txt") + mm = LocalManifestMaker(self.distribution) + mm.manifest = manifest_filename + mm.run() + self.filelist = mm.filelist + else: + log.info("[pbr] Reusing existing SOURCES.txt") + self.filelist = egg_info.FileList() + for entry in open(manifest_filename, 'r').read().split('\n'): + self.filelist.append(entry) + + class LocalSDist(sdist.sdist): """Builds the ChangeLog and Authors files from VC first.""" diff --git a/pbr/tests/test_core.py b/pbr/tests/test_core.py index a7376eb..dd21e6f 100644 --- a/pbr/tests/test_core.py +++ b/pbr/tests/test_core.py @@ -90,7 +90,7 @@ class TestCore(tests.BaseTestCase): tf = tarfile.open(tf_path) names = ['/'.join(p.split('/')[1:]) for p in tf.getnames()] - assert 'extra-file.txt' in names + self.assertIn('extra-file.txt', names) def test_console_script_install(self): """Test that we install a non-pkg-resources console script.""" @@ -120,3 +120,28 @@ class TestCore(tests.BaseTestCase): 'develop', '--install-dir=%s' % self.temp_dir) self.check_script_install(stdout) + + +class TestGitSDist(tests.BaseTestCase): + + def setUp(self): + super(TestGitSDist, self).setUp() + + stdout, _, return_code = self._run_cmd('git', ('init',)) + if return_code: + self.skipTest("git not installed") + + stdout, _, return_code = self._run_cmd('git', ('add', '.')) + stdout, _, return_code = self._run_cmd( + 'git', ('commit', '-m', 'Turn this into a git repo')) + + stdout, _, return_code = self.run_setup('sdist', '--formats=gztar') + + def test_sdist_git_extra_files(self): + """Test that extra files found in git are correctly added.""" + # There can be only one + tf_path = glob.glob(os.path.join('dist', '*.tar.gz'))[0] + tf = tarfile.open(tf_path) + names = ['/'.join(p.split('/')[1:]) for p in tf.getnames()] + + self.assertIn('git-extra-file.txt', names) diff --git a/pbr/tests/testpackage/git-extra-file.txt b/pbr/tests/testpackage/git-extra-file.txt new file mode 100644 index 0000000..e69de29 diff --git a/pbr/util.py b/pbr/util.py index 214cacf..1682b66 100644 --- a/pbr/util.py +++ b/pbr/util.py @@ -77,6 +77,7 @@ try: except ImportError: import configparser +from pbr import extra_files import pbr.hooks # A simplified RE for this; just checks that the line ends with version @@ -258,23 +259,9 @@ def cfg_to_args(path='setup.cfg'): wrap_commands(kwargs) # Handle the [files]/extra_files option - extra_files = has_get_option(config, 'files', 'extra_files') - if extra_files: - extra_files = split_multiline(extra_files) - # Let's do a sanity check - for filename in extra_files: - if not os.path.exists(filename): - raise DistutilsFileError( - '%s from the extra_files option in setup.cfg does not ' - 'exist' % filename) - # Unfortunately the only really sensible way to do this is to - # monkey-patch the manifest_maker class - @monkeypatch_method(manifest_maker) - def add_defaults(self, extra_files=extra_files, log=log): - log.info('[pbr] running patched manifest_maker command ' - 'with extra_files support') - add_defaults._orig(self) - self.filelist.extend(extra_files) + files_extra_files = has_get_option(config, 'files', 'extra_files') + if files_extra_files: + extra_files.set_extra_files(split_multiline(files_extra_files)) finally: # Perform cleanup if any paths were added to sys.path diff --git a/requirements.txt b/requirements.txt index da917d5..4627de5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1 @@ -setuptools_git>=0.4 pip>=1.0 -- cgit v1.2.1