egg_info: Search egg-base for files to add to the manifest

#85 Declined
Repository
wtking
Branch
egg-base-metadata-preservation
Repository
pypa
Branch
default
Author
  1. W. Trevor King
Reviewers
Description

Before these commits, this:

$ mkdir -p /tmp/xyz/{home,lib,scripts,data,egg}
$ cat >/tmp/xyz/home/.pydistutils.cfg <<EOF
> [egg_info]
> egg-base = /tmp/xyz/egg
> EOF
$ export PYTHONPATH=/tmp/xyz/lib
$ export HOME=/tmp/xyz/home
$ setup.py install --home=/tmp/xyz/home --install-lib=/tmp/xyz/lib \
>   --install-scripts=/tmp/xyz/scripts --install-data=/tmp/xyz/data

drops a lot of metadata, installing only SOURCES.txt and zip-safe under EGG-INFO. The problem is that the metadata files are written to egg-base, but egg-base is not searched when creating the manifest because it's outside of the current directory. Work around this by explicitly searching egg-base with distutils.filelist.findall (which is really the version monkeypatched in by setuptools/__init__.py).

Since findall records relative paths, prefix the returned paths with egg-base, so the include_pattern looking for the absolute ei_cmd.egg_info will match them.

This pull request opens with a currently failing patch based on the above shell command, and then fixes egg_info to make the test pass.

Comments (19)

  1. Jason R. Coombs

    Thanks for the contrib.

    This kind of change needs more information. A read without the commit message tells me very little about what this code is intended to do. I suggest implementing the functionality as a separate method with a docstring explaining the purpose. This type of functionality should probably also include a unit test or regression test to capture the expected behavior.

    Also, although the commit message does provide a lot of detail, it's difficult to reference the commit message from the CHANGES.txt file, so it will be difficult for a user to assess the impact of this change. It would be preferable to have a ticket which explains the motivation behind the issue and provides hyperlink target and forum for discussion.

    1. W. Trevor King author

      On Sat, Oct 11, 2014 at 04:23:37PM -0000, Jason R. Coombs wrote:

      I'll add some tests and pull this out as a separate method (although personally I use 'blame' for that sort of thing, otherwise the code just ends up filled with distracting comments ;). I wonder how well git-remote-hg handles force-pushing after rebases…

      Sorry, I usually just assume the patch-submission channel is ephemeral, and that the best places for persistent comments are the commit message and the CHANGES.txt file itself. I'll try and recalibrate my approach for Setuptools ;).

      Cross-posted from the commit message for folks reading the issue:

      Before this commit, this:

      $ mkdir -p /tmp/xyz/{home,lib,scripts,data,egg} $ cat >/tmp/xyx/home/.pydistutils.cfg <<EOF

      [egg_info] egg-base = /tmp/xyz/egg EOF $ export PYTHONPATH=/tmp/xyz/lib $ export HOME=/tmp/xyz/home $ setup.py install --home=/tmp/xyz/home --install-lib=/tmp/xyz/lib \ --install-scripts=/tmp/xyz/scripts --install-data=/tmp/xyz/data

      drops a lot of metadata, installing only SOURCES.txt and zip-safe under EGG-INFO. The problem is that the metadata files are written to egg-base, but egg-base is not searched when creating the manifest because it's outside of the current directory. Work around this by explicitly searching egg-base with distutils.filelist.findall (which is really the version monkeypatched in by setuptools/init.py).

      Since findall records relative paths, prefix the returned paths with egg-base, so the include_pattern looking for the absolute ei_cmd.egg_info will match them.

      1. Jason R. Coombs

        I usually just assume the patch-submission channel is ephemeral

        I agree, and so does Setuptools. What I was suggesting is creating a ticket, which describes the problem being addressed and creates a forum for discussion. Any number of pull requests might be made against a ticket, and any number of commits could also reference a ticket. When a release is made, the CHANGES.txt could reference the ticket. The ticket is the best canonical location to document the motivation, considerations, and relevant changes.

  2. W. Trevor King author

    Bitbucket doesn't seem to be posting additional follow-up emails so:

    On Sat, Oct 11, 2014 at 09:42:11AM -0700, W. Trevor King wrote:

    On Sat, Oct 11, 2014 at 04:23:37PM -0000, Jason R. Coombs wrote:

    I suggest implementing the functionality as a separate method with a
    docstring explaining the purpose. This type of functionality should
    probably also include a unit test or regression test to capture the
    expected behavior.

    I'll add some tests…

    Tests added with the 94aff38 head of the
    egg-base-metadata-preservation branch. I've updated the pull request
    to use the new head. It doesn't look like there's a way to delete the
    old 66b5f9e head on Bitbucket (strip works locally), so I'll just
    leave it dangling. If anyone does know a way to delete the old head,
    let me know.

    and pull this out as a separate method (although personally I use
    'blame' for that sort of thing, otherwise the code just ends up
    filled with distracting comments ;).

    I plan on getting to this part tomorrow.

  3. W. Trevor King author

    Also posted because the email didn't show up:

    On Thu, Oct 16, 2014 at 04:51:01PM -0700, W. Trevor King wrote:

    On Sat, Oct 11, 2014 at 09:42:11AM -0700, W. Trevor King wrote:

    I'll add some tests…

    Tests added with the 94aff38 head of the
    egg-base-metadata-preservation branch. I've updated the pull
    request to use the new head. It doesn't look like there's a way to
    delete the old 66b5f9e head on Bitbucket (strip works locally), so
    I'll just leave it dangling. If anyone does know a way to delete
    the old head, let me know.

    Git's git-remote-hg doesn't seem to handle rebases well (it shifted
    some default-branch commits into my egg-base-metadata-preservation
    branch. I've dropped the git-remote-hg business and stumbled through
    cherry-picking my fixed up pull reqeuest in native Mercurial. I think
    I've irrevocably borked my fork's default branch, but the new PR tip
    (cbd4f6038604) seems to be right. I'll go through this all again
    tomorrow when I pull the new code out into a separate function (if the
    ‘blame’ approach is insufficient).

  4. W. Trevor King author

    Also posted because the email didn't show up:

    On Sat, Oct 11, 2014 at 04:23:37PM -0000, Jason R. Coombs wrote:

    I suggest implementing the functionality as a separate method with a docstring explaining the purpose.

    Done with 6abc59dd (egg_info: Split manifest_maker._add_egg_info into its own method, 2014-10-14). I think that covers everything, but let me know if you want any more changes.

  5. Jason R. Coombs

    I'm getting a test failure when testing on Windows:

    C:\Users\jaraco\projects\setuptools [default pr-85 tip]> ./setup ptr --addopts='-k egg_base'
    running ptr
    running egg_info
    writing top-level names to setuptools.egg-info\top_level.txt
    writing requirements to setuptools.egg-info\requires.txt
    writing setuptools.egg-info\PKG-INFO
    writing dependency_links to setuptools.egg-info\dependency_links.txt
    writing entry points to setuptools.egg-info\entry_points.txt
    reading manifest file 'setuptools.egg-info\SOURCES.txt'
    reading manifest template 'MANIFEST.in'
    writing manifest file 'setuptools.egg-info\SOURCES.txt'
    running build_ext
    ============================= test session starts =============================
    platform win32 -- Python 3.4.2 -- py-1.4.26 -- pytest-2.6.4
    collected 131 items
    
    setuptools/tests/test_egg_info.py F
    
    ================================== FAILURES ===================================
    ________________ TestEggInfo.test_egg_base_installed_egg_info _________________
    
    self = <setuptools.tests.test_egg_info.TestEggInfo testMethod=test_egg_base_installed_egg_info>
    
        def test_egg_base_installed_egg_info(self):
            self._create_project()
            temp_dir = tempfile.mkdtemp(prefix='setuptools-test.')
            os.chmod(temp_dir, stat.S_IRWXU)
            try:
                paths = {}
                for dirname in ['home', 'lib', 'scripts', 'data', 'egg-base']:
                    paths[dirname] = os.path.join(temp_dir, dirname)
                    os.mkdir(paths[dirname])
                config = os.path.join(paths['home'], '.pydistutils.cfg')
                with open(config, 'w') as f:
                    f.write('[egg_info]\n')
                    f.write('egg-base = %s\n' % paths['egg-base'])
                environ = os.environ.copy()
                environ['HOME'] = paths['home']
                code, data = environment.run_setup_py(
                    cmd=[
                        'install', '--home', paths['home'],
                        '--install-lib', paths['lib'],
                        '--install-scripts', paths['scripts'],
                        '--install-data', paths['data']],
                    pypath=':'.join([paths['lib'], self.old_cwd]),
                    data_stream=1,
                    env=environ)
                if code:
    >               raise AssertionError(data)
    E               AssertionError: TEST FAILED: C:\Users\jaraco\AppData\Local\Temp\setuptools-test.6umwig2m\lib\ does NOT support .pth files
    E               error: bad install directory or PYTHONPATH
    E
    E               You are attempting to install a package to a directory that is not
    E               on PYTHONPATH and which Python does not read ".pth" files from.  The
    E               installation directory you specified (via --install-dir, --prefix, or
    E               the distutils default setting) was:
    E
    E                   C:\Users\jaraco\AppData\Local\Temp\setuptools-test.6umwig2m\lib\
    E
    E               and your PYTHONPATH environment variable currently contains:
    E
    E                   'C:\\Users\\jaraco\\AppData\\Local\\Temp\\setuptools-test.6umwig2m\\lib:C:\\Users\\jaraco\\projects\\setuptools'
    
    E
    E               Here are some of your options for correcting the problem:
    E
    E               * You can choose a different installation directory, i.e., one that is
    E                 on PYTHONPATH or supports .pth files
    E
    E               * You can add the installation directory to the PYTHONPATH environment
    E                 variable.  (It must then also be on PYTHONPATH whenever you run
    E                 Python and want to use the package(s) you are installing.)
    E
    E               * You can set up the installation directory to support ".pth" files by
    E                 using one of the approaches described here:
    E
    E                 https://pythonhosted.org/setuptools/easy_install.html#custom-installation-locations
    E
    E               Please make the appropriate changes for your system and try again.
    
    setuptools\tests\test_egg_info.py:125: AssertionError
    ==================== 130 tests deselected by '-kegg_base' =====================
    ================== 1 failed, 130 deselected in 17.88 seconds ==================
    
    1. W. Trevor King author

      On Sat, Dec 13, 2014 at 08:31:23PM -0000, Jason R. Coombs wrote:

      Hmm. It certainly looks like ‘C:\Users\jaraco\AppData\Local\Temp\setuptools-test.6umwig2m\lib\’ is in that PYTHONPATH. Maybe Windows uses another PYTHONPATH separator besides the colon, to avoid confusion with C:\?

    1. W. Trevor King author

      On Sat, Dec 13, 2014 at 08:51:08PM -0000, Jason R. Coombs wrote:

      For future reference, the rebased commits are:

      • f22f6793 (tests.egg_info: Test absolute egg-base install, 2014-10-16)
      • 6cb242a0 (egg_info: Search egg-base for files to add to the manifest, 2014-10-16)
      • c0eae4f6 (egg_info: Split manifest_maker._add_egg_info into its own method, 2014-10-16)

      And the path-sep fix is:

      • 1af3a5f2 (Use os.pathsep. Fixes failure on Windows, 2014-12-13)
  6. Dirk Thomas

    I ran into the same problem and created a minimal package as well as a command sequence to reproduce the problem: https://github.com/dirk-thomas/setuptools_issue

    When I found this ticket I thought installing the latest version of setuptools would solve the problem since it should be fixed since version 8.2. Therefore I installed version 14.0 using pip3 (and confirmed the version with python3 -c "import setuptools; print(setuptools.__version__)") but the problem is still the same.

    Could there be anything else I am doing wrong? Or is this still a problem with setuptools?

  7. Jason R. Coombs

    @Dirk Thomas I believe this functionality is present in all setuptools versions 8.2 and later. I think the issue might be that you're invoking egg_info after invoking install, so egg_info is probably being invoked without the parameters (implicit in install). Try invoking the egg_info command first. If that doesn't work, please file a new ticket. Thanks for the github repo to describe the problem.

  8. Dirk Thomas

    With your comment I was able to fix it.

    I now call the commands in the following order: egg_info, build, install, bdist_egg.

    Beside that I had to change the paths for --egg-base and --build-base to not be the same.

    Thanks!