Issue13193
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2011-10-16 21:24 by pitrou, last changed 2022-04-11 14:57 by admin. 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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 |
2022-04-11 14:57:22 | admin | set | github: 57402 |
2012-03-18 19:40:13 | python-dev | set | messages: + msg156270 |
2012-02-27 11:29:45 | python-dev | set | messages: + msg154455 |
2012-02-25 15:32:25 | python-dev | set | messages: + msg154261 |
2012-02-25 15:14:00 | python-dev | set | messages: + msg154257 |
2012-02-18 01:45:21 | eric.araujo | set | messages: + msg153622 |
2012-02-14 02:35:45 | eric.araujo | set | status: open -> closed resolution: fixed messages: + msg153315 stage: commit review -> resolved |
2012-02-13 19:34:20 | python-dev | set | messages: + msg153290 |
2012-02-13 19:27:23 | benjamin.peterson | set | messages: + msg153289 |
2012-02-12 20:36:29 | nadeem.vawda | set | files:
+ fix-test_process_template_lines.diff messages: + msg153228 stage: needs patch -> commit review |
2012-02-12 05:27:45 | pitrou | set | messages: + msg153178 |
2012-02-12 05:22:38 | eric.araujo | set | messages: + msg153177 |
2012-02-12 05:04:16 | pitrou | set | messages: + msg153174 |
2012-02-12 05:00:39 | eric.araujo | set | files:
+ revert-filelist-fix-2.7.diff resolution: fixed -> (no value) messages: + msg153173 stage: resolved -> needs patch |
2012-02-12 04:29:11 | eric.araujo | set | resolution: fixed messages: + msg153162 stage: resolved |
2012-02-12 04:26:26 | python-dev | set | messages: + msg153161 |
2012-02-12 03:44:55 | python-dev | set | messages: + msg153153 |
2012-02-11 13:29:53 | nadeem.vawda | set | messages: + msg153124 |
2012-02-11 06:19:59 | eric.araujo | set | files:
+ 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:31 | eric.araujo | set | priority: high -> release blocker versions: + Python 2.7 nosy: + benjamin.peterson, georg.brandl messages: + msg153020 |
2011-11-12 15:21:28 | pitrou | set | messages: + msg147499 |
2011-11-12 15:11:19 | eric.araujo | set | messages: + msg147497 |
2011-11-12 14:42:46 | pitrou | set | messages: + msg147491 |
2011-11-12 14:35:07 | eric.araujo | set | status: closed -> open resolution: fixed -> (no value) messages: + msg147489 stage: resolved -> (no value) |
2011-11-12 14:03:10 | nadeem.vawda | set | status: open -> closed resolution: fixed messages: + msg147485 stage: needs patch -> resolved |
2011-11-12 00:38:49 | python-dev | set | messages: + msg147454 |
2011-11-07 18:10:13 | vinay.sajip | set | messages: + msg147243 |
2011-11-07 17:14:02 | eric.araujo | set | messages: + msg147241 |
2011-11-07 16:22:42 | vinay.sajip | set | messages: + msg147234 |
2011-11-07 15:59:17 | eric.araujo | set | nosy:
+ michael.mulich messages: + msg147232 |
2011-11-07 15:22:07 | vinay.sajip | set | messages: + msg147228 |
2011-11-05 23:49:25 | vinay.sajip | set | messages: + msg147125 |
2011-11-05 21:59:35 | nadeem.vawda | set | messages: + msg147119 |
2011-11-05 21:47:28 | vinay.sajip | set | messages: + msg147118 |
2011-11-05 21:35:37 | vinay.sajip | set | messages: + msg147115 |
2011-11-05 16:52:11 | python-dev | set | nosy:
+ python-dev messages: + msg147101 |
2011-11-05 16:48:02 | eric.araujo | set | messages: + msg147099 |
2011-11-05 16:37:22 | eric.araujo | set | messages: + msg147098 |
2011-11-05 15:13:18 | rpetrov | set | nosy:
+ rpetrov messages: + msg147088 |
2011-11-05 14:07:52 | nadeem.vawda | set | messages: + msg147086 |
2011-11-05 13:53:09 | vinay.sajip | set | messages: + msg147085 |
2011-11-05 12:47:38 | vinay.sajip | set | messages: + msg147083 |
2011-11-05 12:39:54 | vinay.sajip | set | nosy:
+ vinay.sajip messages: + msg147082 |
2011-10-26 21:17:40 | westley.martinez | set | messages: + msg146456 |
2011-10-26 16:36:59 | nadeem.vawda | set | messages: + msg146443 |
2011-10-26 16:14:07 | eric.araujo | set | messages: + msg146442 |
2011-10-24 22:56:45 | francismb | set | messages: + msg146335 |
2011-10-24 22:27:40 | nadeem.vawda | set | messages: + msg146333 |
2011-10-24 19:38:12 | francismb | set | messages: + msg146319 |
2011-10-24 19:33:54 | francismb | set | nosy:
+ francismb messages: + msg146318 |
2011-10-24 16:19:33 | eric.araujo | set | assignee: tarek -> eric.araujo messages: + msg146305 |
2011-10-20 08:34:38 | nadeem.vawda | set | title: test_packaging and test_distutils failures under Windows -> test_packaging and test_distutils failures |
2011-10-20 08:34:05 | nadeem.vawda | set | nosy:
+ westley.martinez messages: + msg146011 stage: needs patch |
2011-10-19 17:36:40 | flox | set | nosy:
+ flox |
2011-10-19 17:21:46 | nadeem.vawda | set | nosy:
+ nadeem.vawda messages: + msg145923 |
2011-10-18 16:19:45 | pitrou | set | messages: + msg145834 |
2011-10-17 20:10:20 | paul.moore | set | messages: + msg145757 |
2011-10-17 20:07:52 | paul.moore | set | messages: + msg145756 |
2011-10-17 12:57:07 | eric.araujo | set | nosy:
+ paul.moore, jlove, higery messages: + msg145685 |
2011-10-16 21:24:52 | pitrou | create |