classification
Title: packaging.tests.test_manifest and distutils.tests.test_filelist failures
Type: behavior Stage: resolved
Components: Distutils, Distutils2, Library (Lib), Tests Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: alexis, benjamin.peterson, eric.araujo, flox, francismb, georg.brandl, higery, jlove, michael.mulich, nadeem.vawda, paul.moore, pitrou, python-dev, rpetrov, tarek, vinay.sajip, westley.martinez
Priority: release blocker Keywords: patch

Created on 2011-10-16 21:24 by pitrou, last changed 2012-03-18 19:40 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
fix-packaging-manifest.diff eric.araujo, 2012-02-11 06:19 review
revert-filelist-fix-2.7.diff eric.araujo, 2012-02-12 05:00
fix-test_process_template_lines.diff nadeem.vawda, 2012-02-12 20:36
Messages (57)
msg145643 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-10-16 21:24
http://www.python.org/dev/buildbot/all/builders/x86%20XP-5%203.x/builds/3593/steps/test/logs/stdio
msg145685 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-17 12:57
The first failure looks like a bug in the manifest recursive-include code:

FAIL: test_process_template (distutils.tests.test_filelist.FileListTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "distutils\tests\test_filelist.py", line 181, in test_process_template
    self.assertEqual(file_list.files, ['d/b.py', 'd/d/e.py'])
AssertionError: Lists differ: [] != ['d/b.py', 'd/d/e.py']


The second one may be a sys.path issue:

ERROR: test_resources (packaging.tests.test_command_install_data.InstallDataTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "packaging\tests\test_command_install_data.py", line 129, in test_resources
    with packaging.database.get_file('Spamlib', 'spamd') as fp:
  File "packaging\database.py", line 649, in get_file
    return open(get_file_path(distribution_name, relative_path),
  File "packaging\database.py", line 644, in get_file_path
    raise LookupError('no distribution named %r found' % distribution_name)
LookupError: no distribution named 'Spamlib' found

The test creates a temporary directory which is inserted at the head of sys.path.  packaging.database.get_distribution should thus find Spamlib-0.1.dist-info.  Can someone with a Windows install help me with this?  Printing sys.path and os.listdir(sys.path[0]) would be a good start.
msg145756 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-17 20:07
First one - the problem is in packaging.manifest._translate_pattern, which uses os.path.join on regex parts. That won't work on Windows where os.sep is a backslash, as the backslash is a RE metacharacter.

Actually, the file list seems to only use '/' as a path separator, so you can just use

    pattern_re = "^" + prefix_re + '/' + ".*" + pattern_re

Whether this is right or not depends on whether the test is correctly supplying the expected file list as always /-separated. I don't know enough to be certain on that. An alternative would be to use '[/\\]' in the above, to catch both / and \ separators. Technically wrong, but robust enough to do for now if needed.
msg145757 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-17 20:10
I'm not getting the second error on my home PC. As the failing buildbot is mine, I'll have a look on there to see if I can reproduce when I get the chance.
msg145834 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-10-18 16:19
Similar test failure under x86 OpenIndiana:

======================================================================
ERROR: test_resources (packaging.tests.test_command_install_data.InstallDataTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/export/home/buildbot/32bits/3.x.cea-indiana-x86/build/Lib/packaging/tests/test_command_install_data.py", line 129, in test_resources
    with packaging.database.get_file('Spamlib', 'spamd') as fp:
  File "/export/home/buildbot/32bits/3.x.cea-indiana-x86/build/Lib/packaging/database.py", line 649, in get_file
    return open(get_file_path(distribution_name, relative_path),
  File "/export/home/buildbot/32bits/3.x.cea-indiana-x86/build/Lib/packaging/database.py", line 644, in get_file_path
    raise LookupError('no distribution named %r found' % distribution_name)
LookupError: no distribution named 'Spamlib' found

http://www.python.org/dev/buildbot/all/builders/x86%20OpenIndiana%203.x/builds/2153/steps/test/logs/stdio
msg145923 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2011-10-19 17:21
> The test creates a temporary directory which is inserted at the head of sys.path.
> packaging.database.get_distribution should thus find Spamlib-0.1.dist-info.
> Can someone with a Windows install help me with this?  Printing sys.path and
> os.listdir(sys.path[0]) would be a good start.

    test_resources (packaging.tests.test_command_install_data.InstallDataTestCase) ...
    sys.path: ['c:\\users\\nadeem\\appdata\\local\\temp\\tmpi7pf1f\\tmp4gdwnp',
               '',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\PCbuild\\python33_d.zip',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\DLLs',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\lib',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\PCbuild',
               'C:\\Users\\Nadeem\\AppData\\Roaming\\Python\\Python33\\site-packages',
               'C:\\Users\\Nadeem\\Code\\python3\\python',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\lib\\site-packages']
    os.listdir(sys.path[0]): ['Spamlib-0.1.dist-info']
    ERROR

I noticed that the test succeeds if you run only the InstallDataTestCase (with
"... -v -m InstallDataTestCase test_packaging"), but the sys.path info for this
case is the same as before (modulo the name of the temp directory):

    test_resources (packaging.tests.test_command_install_data.InstallDataTestCase) ...
    sys.path: ['c:\\users\\nadeem\\appdata\\local\\temp\\tmpddi17y\\tmp1mnv9z',
               '',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\PCbuild\\python33_d.zip',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\DLLs',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\lib',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\PCbuild',
               'C:\\Users\\Nadeem\\AppData\\Roaming\\Python\\Python33\\site-packages',
               'C:\\Users\\Nadeem\\Code\\python3\\python',
               'C:\\Users\\Nadeem\\Code\\python3\\python\\lib\\site-packages']
    os.listdir(sys.path[0]): ['Spamlib-0.1.dist-info']
    ok

Additionally, I've been getting a similar failure on my Ubuntu machine:

    ======================================================================
    ERROR: test_resources (packaging.tests.test_command_install_data.InstallDataTestCase)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/home/nadeem/code/src/cpython/def/Lib/packaging/tests/test_command_install_data.py", line 129, in test_resources
        with packaging.database.get_file('Spamlib', 'spamd') as fp:
      File "/home/nadeem/code/src/cpython/def/Lib/packaging/database.py", line 649, in get_file
        return open(get_file_path(distribution_name, relative_path),
      File "/home/nadeem/code/src/cpython/def/Lib/packaging/database.py", line 644, in get_file_path
        raise LookupError('no distribution named %r found' % distribution_name)
    LookupError: no distribution named 'Spamlib' found

Output from Ubuntu machine:

    test_resources (packaging.tests.test_command_install_data.InstallDataTestCase) ...
    sys.path: ['/tmp/tmpubkdye/tmpdmdybx',
               '',
               '/usr/local/lib/python33.zip',
               '/home/nadeem/code/src/cpython/def/Lib',
               '/home/nadeem/code/src/cpython/def/Lib/plat-linux',
               '/home/nadeem/code/src/cpython/def/build/lib.linux-x86_64-3.3',
               '/home/nadeem/.local/lib/python3.3/site-packages',
               '/usr/local/lib/python3.3/site-packages']
    os.listdir(sys.path[0]): ['Spamlib-0.1.dist-info']
    ERROR
msg146011 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2011-10-20 08:34
Issue 13230 (closed as duplicate) reports another Linux failure in test_resources.

Changing the title, since it isn't Windows-specific.
msg146305 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-24 16:19
Spamlib-0.1.dist-info is in a directory on sys.path, so the error makes no sense to me.  Can you inspect the content of that directory?  (i.e os.listdir and file contents)  Also, if you can interrupt the test to get a Python or pdb shell, could you try this:

  >>> from packaging.database import Distribution
  >>> d = Distribution('path/to/Spamlib-0.1.dist-info')
  >>> d.metadata
  >>> d.list_distinfo_files()
  >>> d.list_installed_files()
msg146318 - (view) Author: Francis MB (francismb) * Date: 2011-10-24 19:33
Hi Eli,
I cannot find the file/Distro:

find / -name *'dist-info' 2> /dev/null

/home/ci/cpython/Lib/packaging/tests/fake_dists/grammar-1.0a4.dist-info

/home/ci/cpython/Lib/packaging/tests/fake_dists/towel_stuff-0.1.dist-info

/home/ci/cpython/Lib/packaging/tests/fake_dists/babar-0.1.dist-info

/home/ci/cpython/Lib/packaging/tests/fake_dists/choxie-2.0.0.9.dist-info

/home/ci/cpython/.hg/store/data/_lib/packaging/tests/fake__dists/grammar-1.0a4.dist-info

/home/ci/cpython/.hg/store/data/_lib/packaging/tests/fake__dists/babar-0.1.dist-info

/home/ci/cpython/.hg/store/data/_lib/packaging/tests/fake__dists/towel__stuff-0.1.dist-info

/home/ci/cpython/.hg/store/data/_lib/packaging/tests/fake__dists/choxie-2.0.0.9.dist-info

Where should be the distro?

Cheers,

francis
msg146319 - (view) Author: Francis MB (francismb) * Date: 2011-10-24 19:38
The changeset was:
$ hg tip
changeset:   73075:d4839fea4a5a
[...]
msg146333 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2011-10-24 22:27
> Spamlib-0.1.dist-info is in a directory on sys.path, so the error makes no sense to me.  Can you inspect the content of that directory?  (i.e os.listdir and file contents)  Also, if you can interrupt the test to get a Python or pdb shell, could you try this:
> 
>   >>> from packaging.database import Distribution
>   >>> d = Distribution('path/to/Spamlib-0.1.dist-info')
>   >>> d.metadata
>   >>> d.list_distinfo_files()
>   >>> d.list_installed_files()

(Pdb) p install_dir
'c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b'

(Pdb) from packaging.database import Distribution
(Pdb) _d = Distribution(os.path.join(install_dir, "Spamlib-0.1.dist-info"))
(Pdb) p _d
<Distribution 'Spamlib' 0.1 at 'c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info'>

(Pdb) p _d.metadata
<packaging.metadata.Metadata object at 0x0351C118>

(Pdb) p dict(_d.metadata.items())
{'Name': 'Spamlib', 'License': 'UNKNOWN', 'Author': 'UNKNOWN', 'Metadata-Version': '1.0',
 'Home-page': 'UNKNOWN', 'Summary': 'UNKNOWN', 'Platform': [], 'Version': '0.1',
 'Keywords': [''], 'Author-email': 'UNKNOWN', 'Description': 'UNKNOWN'}

(Pdb) p list(_d.list_distinfo_files())
['c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\METADATA',
 'c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\INSTALLER',
 'c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\REQUESTED',
 'c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\RESOURCES',
 'c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\RECORD']

(Pdb) p list(_d.list_installed_files())
[('c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmpo8yw3_\\spamd', 'd13e6156ce78919a981e424b2fdcd974', '15'),
 ('c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\METADATA', '8c9f474663f774563ded42983eb22447', '182'),
 ('c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\INSTALLER', '44e3fde05f3f537ed85831969acf396d', '9'),
 ('c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\REQUESTED', 'd41d8cd98f00b204e9800998ecf8427e', '0'),
 ('c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\RESOURCES', '1c5aec043865966e17b1044e20087b9d', '68'),
 ('c:\\users\\nadeem\\appdata\\local\\temp\\tmpe96yhv\\tmp9ga04b\\Spamlib-0.1.dist-info\\RECORD', '', '')]

==== File contents:

spamd [no EOL]:
    # Python script

METADATA:
    Metadata-Version: 1.0
    Name: Spamlib
    Version: 0.1
    Summary: UNKNOWN
    Description: UNKNOWN
    Keywords: 
    Home-page: UNKNOWN
    Author: UNKNOWN
    Author-email: UNKNOWN
    License: UNKNOWN

INSTALLER [no EOL]:
    distutils

REQUESTED is empty

RESOURCES:
    spamd,c:\users\nadeem\appdata\local\temp\tmpe96yhv\tmpo8yw3_\spamd

RECORD:
    c:\users\nadeem\appdata\local\temp\tmpe96yhv\tmpo8yw3_\spamd,d13e6156ce78919a981e424b2fdcd974,15
    c:\users\nadeem\appdata\local\temp\tmpe96yhv\tmp9ga04b\Spamlib-0.1.dist-info\METADATA,8c9f474663f774563ded42983eb22447,182
    c:\users\nadeem\appdata\local\temp\tmpe96yhv\tmp9ga04b\Spamlib-0.1.dist-info\INSTALLER,44e3fde05f3f537ed85831969acf396d,9
    c:\users\nadeem\appdata\local\temp\tmpe96yhv\tmp9ga04b\Spamlib-0.1.dist-info\REQUESTED,d41d8cd98f00b204e9800998ecf8427e,0
    c:\users\nadeem\appdata\local\temp\tmpe96yhv\tmp9ga04b\Spamlib-0.1.dist-info\RESOURCES,1c5aec043865966e17b1044e20087b9d,68
    c:\users\nadeem\appdata\local\temp\tmpe96yhv\tmp9ga04b\Spamlib-0.1.dist-info\RECORD,,

====

> Where should be the distro?

You won't find it in the hg repository - it gets created by the test, and
then deleted afterwards.

> The changeset was:
> $ hg tip
> changeset:   73075:d4839fea4a5a
> [...]

That's not possible - these failures have been around for over a week now,
and that changeset was only committed yesterday.

From running the tests locally, it looks like test_resources was failing on
Windows since it was first introduced in 1f3459b08298. This didn't show up so
clearly on the buildbots because the build was broken at the time.
msg146335 - (view) Author: Francis MB (francismb) * Date: 2011-10-24 22:56
>
>> Where should be the distro?
>
> You won't find it in the hg repository - it gets created by the test, and
> then deleted afterwards.

Thanks for the info

>
>> The changeset was:
>> $ hg tip
>> changeset:   73075:d4839fea4a5a
>> [...]
>
> That's not possible - these failures have been around for over a week now,
> and that changeset was only committed yesterday.
>

Thanks again, next time I'll search the exact start point of failure in 
the repo. I've just put the ref to MY actual tip ;)
msg146442 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-26 16:14
anikom15, can you give me a URI to download the OS you used?  I don’t know when I’ll be able to download Windows, but surely a linux-using OS will take less space and time to download and install in a VM.
msg146443 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2011-10-26 16:36
I've been able to consistently reproduce the test_resources failure on Ubuntu 11.10 64-bit, FWIW.
msg146456 - (view) Author: Westley Martínez (westley.martinez) * Date: 2011-10-26 21:17
http://www.archlinux.org/download/

It's a minimalist distribution but if you read through the install guide or beginner's guide you'll be fine.
msg147082 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-05 12:39
I get the opposite failure to Nadeem as far as InstallDataTestCase.test_resources: it works on Ubuntu 64-bit, but fails on 32-bit. Digging into it a bit further, I find that _generate_cache in Lib/packaging/database.py returns prematurely in the failing case, because _cache_generated_egg is True in the failing case but not in the test run which succeeds. My guess is that this could be due to a missing call to clear_cache() in the same module, as _cache_generated_egg is set to True only in the last part of _generate_cache, and my guess is that the value is a holdover from an earlier test.

If I add the line "packaging.database.clear_cache()" just above "with packaging.database.get_file('Spamlib', 'spamd') as fp:" then the test succeeds.
msg147083 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-05 12:47
I added the packaging.database.clear_cache() at the end of setUp(); that works, too.

Gotta love global variables - not! ;-)
msg147085 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-05 13:53
I looked in packaging/test, and noticed that database.get_distribution is called from test_depgraph, test_xmlrpc and test_database. These may also need looking over to see if cache invalidation needs to happen.
msg147086 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2011-11-05 14:07
> I added the packaging.database.clear_cache() at the end of setUp(); that works, too.

That fixes the InstallDataTestCase failure for me as well
(on both Ubuntu 11.10 64-bit and Windows 7 64-bit).

> I get the opposite failure to Nadeem as far as InstallDataTestCase.test_resources: it works on Ubuntu 64-bit, but fails on 32-bit.

FTR, I haven't checked this on Ubuntu 32-bit. It's rather odd for the
failure to be architecture-dependent. Are these two machines running
different versions of Ubuntu, perhaps?
msg147088 - (view) Author: Roumen Petrov (rpetrov) * Date: 2011-11-05 15:13
Test test_install and test_command_install_data interference cache . Adding clear_cache as clean up routine  will resolve reported issue with Spamlib. The patch is trivial. Also one of the test set clear_cache as cleanup.

Most important is why packaging use global variables . To me good solution will not use globals , but this  require authors to rewrite packaging itself.
msg147098 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-05 16:37
Thanks, I’ll add the disable_cache call.

Westley: I’ve installed Arch.  Is there a command I can run to get zlib, openssl, gcc, make and all that, similar to “aptitude build-dep python3.2”?
msg147099 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-05 16:48
I’ll open another reports for the possible bug in _generate_cache and for review of the other tests calling get_distribution.

> Test test_install and test_command_install_data interference cache
I’ve added checks in regrtest to make sure that each tests clears the caches some time ago.

> Most important is why packaging use global variables.
[citation needed].  First, mutable objects with a global name (let’s not use “variable” for Python) are not unconditionally evil; see sys.path and sys.modules for example.  Second, how concretely would you change that implementation?  We need shared caches for functions in packaging.database.  They should be built on demand, and should be cleared by higher-level libraries or applications when the filesystem has changed (after an installation or removal for example).  With these constraints, do you have implementation ideas?
msg147101 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-05 16:52
New changeset 4eee9dd61147 by Éric Araujo in branch 'default':
Try to fix buildbot failures from #13193
http://hg.python.org/cpython/rev/4eee9dd61147
msg147115 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-05 21:35
>[citation needed].  First, mutable objects with a global name (let’s not use
> "variable" for Python) are not unconditionally evil; see sys.path and
> sys.modules for example.  Second, how concretely would you change that
> implementation?  We need shared caches for functions in packaging.database.
> They should be built on demand, and should be cleared by higher-level
> libraries or applications when the filesystem has changed (after an
> installation or removal for example).  With these constraints, do you have
> implementation ideas?

I agree, we shouldn't have a knee-jerk reaction to global bindings - they definitely
have their uses, and I'm sometimes driven to defend their use in logging. Since
you ask, though - one possible approach might be: Have those bindings be
instance variables in a Database class in database.py, and have a module-level
binding to an instance of it. Then, tests can have their own instance which is
thrown away aftereach test.

This problem was not trivial to find, because it appears that test execution order
may not be entirely deterministic: I couldn't see any other reason why the flag
would have different values on different machines. I believe that you (Éric) had
difficulty reproducing it. Perhaps we don't need to re-implement, but instead
add more tests around cache invalidation and cache contents.
msg147118 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-05 21:47
> FTR, I haven't checked this on Ubuntu 32-bit. It's rather odd for the

> failure to be architecture-dependent. Are these two machines running
> different versions of Ubuntu, perhaps?

They are (Ubuntu Natty 32-bit failing, Ubuntu Oneiric 64-bit passing).
However, I don't believe the failure is directly architecture-related, but
indirectly so. For example, test execution order might not be identical
on all machines, depending for example on hash values and dict bucket
sizes, which I would expect to be different on different machines for any
number of reasons. In my limited testing, I found the following:

Ubuntu Natty 32-bit - failed.
Windows 7 32-bit - failed.
Ubuntu Oneiric 64-bit - passed.
Linux Mint "Katya" 11 64-bit (based on Ubuntu Natty) - passed.

With non-deterministic test execution order combined with module-level
data updated by tests, it can be hard to reproduce these failures - a bit
like uninitialised data errors in C :-(
msg147119 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2011-11-05 21:59
How strange. I've had no problem reproducing the failures; they've
occurred every time I've run test_packaging since the bug was opened
(on both Ubuntu and Windows). And the Windows buildbots seem to be
failing consistently as well...
msg147125 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-05 23:49
> How strange. I've had no problem reproducing the failures; they've

> occurred every time I've run test_packaging since the bug was opened
> (on both Ubuntu and Windows). And the Windows buildbots seem to be
> failing consistently as well...

On a given machine, if the failure occurs, it fails every time. I got failures on
Ubuntu 32-bit and Windows 32-bit every time, too. One more data
point: on OS X Leopard (32-bit), there were no failures at all in my testing.

I only saw the tests pass when I tested on 64-bit Ubuntu variants (and OS X
32-bit).

N.B. All my testing was on the pythonv branch - though I don't believe that
had any bearing on the tests, as they also failed on the default branch (on
Ubuntu Jaunty 32-bit, where I do maintenance for logging etc.)
msg147228 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-07 15:22
Re. Paul Moore's comment - IMO he's right about the problem, but changing only packaging.manifest._translate_pattern doesn't do it. The equivalent fix has to be made in distutils.filelist.translate_pattern. I've made the change in the pythonv branch, and the test no longer fails.
msg147232 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-07 15:59
[global variables]
> one possible approach might be: Have those bindings be instance variables in a Database class in
> database.py, and have a module-level binding to an instance of it. Then, tests can have their
> own instance which is thrown away aftereach test.
I’m not sure we can do that, or that I understand the suggestion.  If you’re talking about how pprint/textwrap/reprlib use a module-level instance to offer module-level functions with some defaults, I think the database module can’t work that way.  We have module-level classes (Distribution and EggInfoDistribution, no inheritance) and module-level functions (get_distribution, the one in the failing test, for example) which may use any of the four caches and return instances of either class.  If I understand your suggestion correctly, you’d move database._cache_egg to database.EggInfoDistribution._cache, and maybe change the code to move the cache logic to the *Distribution classes instead of in the various functions (thus implementing a singleton registry like logging loggers.  I like this idea.

Writing this made me think of another possible solution: dependency injection!  Just like the functions have a paths argument that defaults to sys.path if None is passed, I could change the internal cache generation function to take arguments for the four caches, so that the tests could pass fresh dictionaries that would not be shared between tests, unlike database-module-level global objects.

> This problem was not trivial to find, because it appears that test execution order may not be
> entirely deterministic: I couldn't see any other reason why the flag would have different values
> on different machines.
Sorry, what flag?

> I believe that you (Éric) had difficulty reproducing it.
More than difficulty: I have not yet reproduced it.  The tests pass on my OS, Debian x86_64 with linux3.  I’ve installed Arch but not cloned/built Python yet.

> Perhaps we don't need to re-implement, but instead add more tests around cache invalidation
> and cache contents.
The packaging database cache API is not fantastic.  Libraries or applications can turn it off entirely, or clear it so that sys.path gets scanned again.  I’m not even sure that our tests do the right thing: They disable the cache in setUp and re-enable it in cleanup, but maybe they should just clear it in cleanup.  (BTW I have added a regrtest check to make sure the cache is re-enabled and clean after tests run.)  In any case, we don’t have tests that check the behavior of the database module with respect to caching.

“There are only two hard problems in Computer Science: cache invalidation and naming things” (Phil Karlton), and I’m less bad at the latter.  The student who implemented most of the database module is not active in our group anymore, but Michael Mulich, who started the module but did not write the cache code, still is.  So there’s hope that we can fix this together (and thanks for all the reports, diagnosis and suggestions so far!).

> Re. Paul Moore's comment - IMO he's right about the problem, but changing only
> packaging.manifest._translate_pattern doesn't do it. The equivalent fix has to be made in
> distutils.filelist.translate_pattern. I've made the change in the pythonv branch, and the test no
> longer fails.
Patches for upstream cpython would be most helpful.  I also think that fixing bugs in the pythonv branch makes it harder to review.
msg147234 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-07 16:22
> 
>>  entirely deterministic: I couldn't see any other reason why the flag 
> would have different values
>>  on different machines.
> Sorry, what flag?

By "flag" I mean _cache_generated_egg ("flag" as in Boolean value)

> Patches for upstream cpython would be most helpful.  I also think that fixing 
> bugs in the pythonv branch makes it harder to review.

Ordinarily I'd submit a cpython patch, but in this case it's a one liner as Paul has
suggested, so there's not much to review / comment on.IMO BitBucket makes it
reasonably easy to review short patches like this. Here's the change to
packaging.manifest:

--- a/Lib/packaging/manifest.py    Sun Nov 06 22:27:53 2011 +0000
+++ b/Lib/packaging/manifest.py    Mon Nov 07 14:58:23 2011 +0000
@@ -366,7 +366,8 @@
         # ditch end of pattern character
         empty_pattern = _glob_to_re('')
         prefix_re = _glob_to_re(prefix)[:-len(empty_pattern)]
-        pattern_re = "^" + os.path.join(prefix_re, ".*" + pattern_re)
+        # See issue 13193: Don't use os.path.join
+        pattern_re = "^%s/.*%s" % (prefix_re, pattern_re)
     else:                               # no prefix -- respect anchor flag
         if anchor:
             pattern_re = "^" + pattern_re

and the change to distutils.filelist:

--- a/Lib/distutils/filelist.py    Mon Nov 07 14:58:23 2011 +0000
+++ b/Lib/distutils/filelist.py    Mon Nov 07 15:06:18 2011 +0000
@@ -313,7 +313,8 @@
         # ditch end of pattern character
         empty_pattern = glob_to_re('')
         prefix_re = (glob_to_re(prefix))[:-len(empty_pattern)]
-        pattern_re = "^" + os.path.join(prefix_re, ".*" + pattern_re)
+        # See issue 13193: Don't use os.path.join
+        pattern_re = "^%s/.*%s" % (prefix_re, pattern_re)
     else:                               # no prefix -- respect anchor flag
         if anchor:
             pattern_re = "^" + pattern_re

You'll see I used a different idiom to Paul in my fix :-)

Can the distutils/packaging duplication not be avoided? IMO the correct
cpython fix would address this.
msg147241 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-07 17:14
> By "flag" I mean _cache_generated_egg ("flag" as in Boolean value)
Ah, I had forgotten this earlier message:

> I get the opposite failure to Nadeem as far as InstallDataTestCase.test_resources: it works on
> Ubuntu 64-bit, but fails on 32-bit. Digging into it a bit further, I find that _generate_cache in
> Lib/packaging/database.py returns prematurely in the failing case, because _cache_generated_egg is
> True in the failing case but not in the test run which succeeds.

That it depends on the architecture currently baffles me.

> Ordinarily I'd submit a cpython patch, but in this case it's a one liner as Paul has suggested,
> so there's not much to review / comment on.
A patch in the list of files is much easier to find that a one-liner in a message.  Anyway, it’s not your fault.  Antoine was kind enough to get my attention on the buildbot failures; to make this manageable, I will open separate reports with specific names for each different bug.

> Here's the change to packaging.manifest: [snip]
> and the change to distutils.filelist: [snip]
Thanks.  Luckily, these modules recently gained full test coverage, so I will be able to commit the fixes and feel safe.

> You'll see I used a different idiom to Paul in my fix :-)
I’m a big fan of format strings other string concatenation, too.

> Can the distutils/packaging duplication not be avoided?
No.  They are independent modules.  distutils will die; packaging will be improved and cleaned up.  However, contributors can work on packaging only and leave the gruesome backporting work to me.

> IMO the correct cpython fix would address this.
I don’t understand.
msg147243 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-07 18:10
>
> That it depends on the architecture currently baffles me.

>

The only explanation I can come up with is that on different machines, the order of the tests might be slightly different. That would allow the flag to be set differently on different machines, based on which other tests have run earlier.

Different ordering could be explained by hash implementations and/or dict bucket sizes being different on different architectures.

>>  Can the distutils/packaging duplication not be avoided?
> No.  They are independent modules.  distutils will die; packaging will be 
> improved and cleaned up.  However, contributors can work on packaging only and 
> leave the gruesome backporting work to me.
> 
>>  IMO the correct cpython fix would address this.
> I don’t understand.

I meant removing the duplication - but from your explanation, I agree that there is no point, since distutils has a limited lifetime.
msg147454 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-12 00:38
New changeset 0a94e2f807c7 by Antoine Pitrou in branch '3.2':
Issue #13193: fix distutils.filelist.FileList under Windows
http://hg.python.org/cpython/rev/0a94e2f807c7

New changeset 80d5040f2a78 by Antoine Pitrou in branch '3.2':
Add NEWS entry for #13193
http://hg.python.org/cpython/rev/80d5040f2a78

New changeset 64485e0700ba by Antoine Pitrou in branch 'default':
Issue #13193: Fix distutils.filelist.FileList and
http://hg.python.org/cpython/rev/64485e0700ba

New changeset 557a973709de by Antoine Pitrou in branch '2.7':
Issue #13193: Fix distutils.filelist.FileList under Windows.  The
http://hg.python.org/cpython/rev/557a973709de
msg147485 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2011-11-12 14:03
I'm no longer getting the failures on either Ubuntu or Windows (and the
Windows buildbots are now green), so tentatively marking this as fixed.
Feel free to reopen if something is still broken.
msg147489 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-12 14:35
> This problem was not trivial to find, because it appears that test execution order may not be
> entirely deterministic: I couldn't see any other reason why the flag would have different values
> on different machines.
On my machine, it looks like unittest runs them in the order they’re found.  I have only one core, but maybe tests are run in parallel on your machine, so with the missing call to enable_cache, that would explain the test failures.

Antoine, I appreciate that you took time to fix this bug while I was without Internet and without Windows, but unfortunately I will have to backout your commit.  Postel’s Law doesn’t win here: It is documented that the MANIFEST template only accepts /-delimited paths, so I have to find a fix for the tests without changing the code to avoid breaking the feature freeze.  I’ll get a Windows VM before I do that, to avoid making the bots red again.

In the future, please feel free to add unittest.expectedFailure decorators to problematic tests when I’m too long to come up with a fix, so that other people can see when their commits add problems.
msg147491 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-11-12 14:42
> Antoine, I appreciate that you took time to fix this bug while I was
> without Internet and without Windows, but unfortunately I will have to
> backout your commit.  Postel’s Law doesn’t win here: It is documented
> that the MANIFEST template only accepts /-delimited paths,

“Just like in the setup script, file and directory names in the manifest
template should always be slash-separated”

That's "should", not "must".
Also, I thought people did undocumented things with distutils, and we
had to support these undocumented uses?

> In the future, please feel free to add unittest.expectedFailure
> decorators to problematic tests when I’m too long to come up with a
> fix, so that other people can see when their commits add problems.

I don't see how adding "expected failures" solves anything. It's not
clear why this failure should have been expected, rather than a bug.
A "commit adding problem" should be fixed or reverted; marking some
failures expected is just dodging the issue.
msg147497 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-12 15:11
> That's "should", not "must".
I read that “should” as a polite “must”.

> Also, I thought people did undocumented things with distutils, and we
> had to support these undocumented uses?
People rely on undocumented features and sometimes on bugs.  Thus, we cannot refactor or otherwise clean up internals.  Here, you did change internals.

> I don't see how adding "expected failures" solves anything. [...].
If a buildbot is red for a week because of me and another developer commits something that creates another test failure, they won’t see that a buildbot has turned red because it already was.  It’s just a temporary edition to avoid polluting the buildbots output.

> A "commit adding problem" should be fixed or reverted
The point is that fixing it may take tome.  Reverting is fine by me.
msg147499 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-11-12 15:21
> > A "commit adding problem" should be fixed or reverted
> The point is that fixing it may take tome.  Reverting is fine by me.

But we have no way of knowing you will be taking tome to do it.
Ideally, you should have reverted it yourself (or applied whatever
test-skipping solution you see fit).
msg153020 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-10 04:25
Bumping up to release blocker, as I don’t want the next bugfixes to go out without reverting the last changeset (but won’t right know as I need to fix the bug there was on Windows).
msg153108 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-11 06:19
Attached is Vinay’s fix as a diff.  Can someone test this patch on Windows or any of the other OSes that showed failures?
msg153124 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-11 13:29
All tests pass with the patch applied, on both Windows 7 and Ubuntu 11.10.

I notice that the patch only changes Lib/packaging/manifest.py; does
Lib/distutils/filelist.py perhaps also need to be updated? Changeset
64485e0700ba modified both of these files.
msg153153 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-12 03:44
New changeset c566a3447ba1 by Éric Araujo in branch '2.7':
Fix distutils.filelist.FileList under Windows (#13193).
http://hg.python.org/cpython/rev/c566a3447ba1
msg153161 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-12 04:26
New changeset 90b30d62caf2 by Éric Araujo in branch '3.2':
Fix distutils.filelist.FileList under Windows (#13193).
http://hg.python.org/cpython/rev/90b30d62caf2

New changeset 68347f8430ec by Éric Araujo in branch 'default':
Merge fixes for #13193 and FAQ from 3.2
http://hg.python.org/cpython/rev/68347f8430ec

New changeset f06effb61cde by Éric Araujo in branch 'default':
Port the fix for #13193 to packaging
http://hg.python.org/cpython/rev/f06effb61cde
msg153162 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-12 04:29
Reverted the liberal fix and replaced it with Vinay’s in time for the next bugfix releases.  Will watch buildbot and close the report.  Thanks to all who helped.
msg153173 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-12 05:00
Buildbots exist to crush joy: http://www.python.org/dev/buildbot/all/builders/x86%20XP-4%202.7/builds/1285/steps/test/logs/stdio/text

Possible ways forward:
- One of you gentlemen with a Windows box and a clear head can try to find a correct fix for the bug
- If Benjamin wants to release this week-end, the attached patch reverts to the buggy 2.7.2 code and skips the failing test on Windows (more skips may be needed)
- I’ll be on the Internet on Monday afternoon (Europe).
msg153174 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-02-12 05:04
> Possible ways forward:
> - One of you gentlemen with a Windows box and a clear head can try to
> find a correct fix for the bug

This error shows that distutils's own test suite relies on being able to
put both kind of path separators in a manifest, so my fix was probably
the correct one ;)
msg153177 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-12 05:22
I’d say there is a bug in the tests then.  Remember that they were added recently and only tested on posix, so they’re not authoritative.
msg153178 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-02-12 05:27
> I’d say there is a bug in the tests then.  Remember that they were
> added recently and only tested on posix, so they’re not authoritative.

How about people who have similar "bugs" in their code then?
I think you're trying to be pure instead of practical.
msg153228 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-12 20:36
It appears that while test_process_template() uses "/"-delimited paths
consistently across all OSes, 2.7 also has a test_process_template_line()
that uses os.path.join() to construct its paths. Changing this test to
use hardcoded "/"-delimited paths fixes the failure (see attached patch).
msg153289 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-02-13 19:27
Apply!
msg153290 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-13 19:34
New changeset 3925081a7ca0 by Nadeem Vawda in branch '2.7':
Issue #13193: Fix distutils.filelist tests to always use / as path separator.
http://hg.python.org/cpython/rev/3925081a7ca0
msg153315 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-14 02:35
Sorry I was unavailable.  I read and approved of the patch but could not push, so thanks for doing it :)
msg153622 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-18 01:45
FYI, the last commit was actually incorrect.  I must confess that I wanted to fix the buildbots as quick as possible, especially in light of the upcoming-though-undated 2.7 release, and did not read the code as carefully as I should have, but now I have done it and am much more confident about what is the correct behavior.  Interested parties can follow #6884.
msg154257 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-25 15:14
New changeset 47788c90f80b by Éric Araujo in branch '2.7':
Fix long-standing bugs with MANIFEST.in parsing on Windows (#6884).
http://hg.python.org/cpython/rev/47788c90f80b
msg154261 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-25 15:32
New changeset 73aa4c9305b3 by Éric Araujo in branch '3.2':
Fix long-standing bugs with MANIFEST.in parsing on Windows (#6884).
http://hg.python.org/cpython/rev/73aa4c9305b3
msg154455 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-27 11:29
New changeset 4d6a9f287543 by Éric Araujo in branch 'default':
Fix bugs with MANIFEST.in parsing on Windows (#6884).
http://hg.python.org/distutils2/rev/4d6a9f287543
msg156270 - (view) Author: Roundup Robot (python-dev) Date: 2012-03-18 19:40
New changeset edcdef70c44e by Éric Araujo in branch '3.2':
Fix long-standing bugs with MANIFEST.in parsing on Windows (#6884).
http://hg.python.org/cpython/rev/edcdef70c44e
History
Date User Action Args
2012-03-18 19:40:13python-devsetmessages: + msg156270
2012-02-27 11:29:45python-devsetmessages: + msg154455
2012-02-25 15:32:25python-devsetmessages: + msg154261
2012-02-25 15:14:00python-devsetmessages: + msg154257
2012-02-18 01:45:21eric.araujosetmessages: + msg153622
2012-02-14 02:35:45eric.araujosetstatus: open -> closed
resolution: fixed
messages: + msg153315

stage: commit review -> resolved
2012-02-13 19:34:20python-devsetmessages: + msg153290
2012-02-13 19:27:23benjamin.petersonsetmessages: + msg153289
2012-02-12 20:36:29nadeem.vawdasetfiles: + fix-test_process_template_lines.diff

messages: + msg153228
stage: needs patch -> commit review
2012-02-12 05:27:45pitrousetmessages: + msg153178
2012-02-12 05:22:38eric.araujosetmessages: + msg153177
2012-02-12 05:04:16pitrousetmessages: + msg153174
2012-02-12 05:00:39eric.araujosetfiles: + revert-filelist-fix-2.7.diff
resolution: fixed -> (no value)
messages: + msg153173

stage: resolved -> needs patch
2012-02-12 04:29:11eric.araujosetresolution: fixed
messages: + msg153162
stage: resolved
2012-02-12 04:26:26python-devsetmessages: + msg153161
2012-02-12 03:44:55python-devsetmessages: + msg153153
2012-02-11 13:29:53nadeem.vawdasetmessages: + msg153124
2012-02-11 06:19:59eric.araujosetfiles: + fix-packaging-manifest.diff
keywords: + patch
messages: + msg153108

title: test_packaging and test_distutils failures -> packaging.tests.test_manifest and distutils.tests.test_filelist failures
2012-02-10 04:25:31eric.araujosetpriority: high -> release blocker
versions: + Python 2.7
nosy: + benjamin.peterson, georg.brandl

messages: + msg153020
2011-11-12 15:21:28pitrousetmessages: + msg147499
2011-11-12 15:11:19eric.araujosetmessages: + msg147497
2011-11-12 14:42:46pitrousetmessages: + msg147491
2011-11-12 14:35:07eric.araujosetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg147489

stage: resolved -> (no value)
2011-11-12 14:03:10nadeem.vawdasetstatus: open -> closed
resolution: fixed
messages: + msg147485

stage: needs patch -> resolved
2011-11-12 00:38:49python-devsetmessages: + msg147454
2011-11-07 18:10:13vinay.sajipsetmessages: + msg147243
2011-11-07 17:14:02eric.araujosetmessages: + msg147241
2011-11-07 16:22:42vinay.sajipsetmessages: + msg147234
2011-11-07 15:59:17eric.araujosetnosy: + michael.mulich
messages: + msg147232
2011-11-07 15:22:07vinay.sajipsetmessages: + msg147228
2011-11-05 23:49:25vinay.sajipsetmessages: + msg147125
2011-11-05 21:59:35nadeem.vawdasetmessages: + msg147119
2011-11-05 21:47:28vinay.sajipsetmessages: + msg147118
2011-11-05 21:35:37vinay.sajipsetmessages: + msg147115
2011-11-05 16:52:11python-devsetnosy: + python-dev
messages: + msg147101
2011-11-05 16:48:02eric.araujosetmessages: + msg147099
2011-11-05 16:37:22eric.araujosetmessages: + msg147098
2011-11-05 15:13:18rpetrovsetnosy: + rpetrov
messages: + msg147088
2011-11-05 14:07:52nadeem.vawdasetmessages: + msg147086
2011-11-05 13:53:09vinay.sajipsetmessages: + msg147085
2011-11-05 12:47:38vinay.sajipsetmessages: + msg147083
2011-11-05 12:39:54vinay.sajipsetnosy: + vinay.sajip
messages: + msg147082
2011-10-26 21:17:40westley.martinezsetmessages: + msg146456
2011-10-26 16:36:59nadeem.vawdasetmessages: + msg146443
2011-10-26 16:14:07eric.araujosetmessages: + msg146442
2011-10-24 22:56:45francismbsetmessages: + msg146335
2011-10-24 22:27:40nadeem.vawdasetmessages: + msg146333
2011-10-24 19:38:12francismbsetmessages: + msg146319
2011-10-24 19:33:54francismbsetnosy: + francismb
messages: + msg146318
2011-10-24 16:19:33eric.araujosetassignee: tarek -> eric.araujo
messages: + msg146305
2011-10-20 08:34:38nadeem.vawdasettitle: test_packaging and test_distutils failures under Windows -> test_packaging and test_distutils failures
2011-10-20 08:34:05nadeem.vawdasetnosy: + westley.martinez

messages: + msg146011
stage: needs patch
2011-10-19 17:36:40floxsetnosy: + flox
2011-10-19 17:21:46nadeem.vawdasetnosy: + nadeem.vawda
messages: + msg145923
2011-10-18 16:19:45pitrousetmessages: + msg145834
2011-10-17 20:10:20paul.mooresetmessages: + msg145757
2011-10-17 20:07:52paul.mooresetmessages: + msg145756
2011-10-17 12:57:07eric.araujosetnosy: + paul.moore, jlove, higery
messages: + msg145685
2011-10-16 21:24:52pitroucreate