broken Debian's pypy 2.7 causes virtualenv to fail #1770
Comments
|
Why are you changing the stdlib entries, not just purelib/platlib? This feels to me like a broken packaging; you're basically pointing the stdlib to a folder that does not exist. Either the stdlib or platstdlib should contain This kind of patching seems the wrong thing to me, if you want to support installing packages without root (which feels to me a bad idea, anyone can install now a bad package with a pth file that will break the system Python 🤔) you should be using the distutils configuration file rather than patching the source code - see https://docs.python.org/3/install/index.html#distutils-configuration-files (works just the same for python 2/ pypy). PS. Maybe would be nice to review the patches you apply, not sure all of them makes sense to me. |
gaborbernat
changed the title
|
Thanks for the reply. I'll admit that I'm a little bit miffed at such hard push-back, and the ticket immediately being closed, with a finger-waving title. There's an issue here we need to resolve, between two systems, and I'd like to have a constructive conversation. So, I'll do my best to do that.
Honestly, this dates back approx 8 years, to when that patch was first written, so I can't remember all of my thinking. But I think I just did that for consistency, and it didn't seem to break anything. I think it has to be relative to base, for things to still work correctly in the virtualenv. I can tweak things, and see what happens. The reason I filed this ticket is that everything is just working correctly for our PyPy3 package, with exactly the same patch. So something in virtualenv is treating that differently, and I'm not sure exactly what, so I was asking for your help to understand what's going on.
Fair enough.
Two reasons:
I work with the PyPy upstream team, they (at least roughly) know what I'm patching, and I commit whatever makes sense, upstream.
Totally agreed that users shouldn't be doing that. Users should almost always use virtualenvs. (That's why this bug exists :) ) However, in the modern container world, there is an argument for installing packages system-wide as root. We should support it. Also, again, I wanted PyPy on Debian to behave as similarly as possible to cPython.
I sense a hostility to patches :) I am, of course, painfully aware of every patch I apply. Patches take effort to maintain, and it has to be weighed against value. But, it is pretty common to need to patch at least something in package, to get it to work well with the rest of the system. We live with it. And try to push everything possible upstream.
Interesting. If I knew about those, I'd forgotten. I'll have a play around with them, and see if they can do what I need. I would like to maintain alignment with our cPython approach. So, I'll have to talk to the maintainer, too. |
I'm sorry if this came across as harsh worded. Wasn't my intention. I'll admit there is some annoyance over how Debian patches tools, mostly because over the years we've got multiple issues opened by annoyed users due to some of the patches. Users tend to come to us and complain before going to Debian in practice (I sense maybe because it's easier to open issues here than in Debian). For another recent example of this see pre-commit/pre-commit#1383 (comment). I'd very much prefer we don't have to do these rounds by addressing incompatible changes early on, rather than users running into it later. I've personally felt the title was finger-pointing as it implied virtualenv is broken; ideally, I've too would have preferred a less finger-pointing title.
The reason for this is because of how a virtual environment is created differs massively between Python 2 and Python 3, partially due to Python 3 mandates https://www.python.org/dev/peps/pep-0405/ that makes things much easier. Python 3 working does not necessarily mean the patch is correct, it's more due to Python 3 virtual environment not relying at the moment on what the patch changes.
It's ok to patch sysconfig but the patch should describe paths for the system python. I don't think this is the case at the moment, as in this case the systdlib and sysplatlib seems to point to unexisting paths.
Wait, I thought you've added the patch so users can install without root, but here you're saying you want to support a behaviour under root?
From my side there would be no hostility; I would like to understand the reason for the patch because ideally, I'd prefer to offer a way that makes the patch less intrusive to avoid some of such problems. |
Yeah, not the first time that's been an issue. And won't be the last. :( Some bugs are specific to upstream. Some bugs are specific to a distribution (with or without patches being involved). Sometimes the upstream bug-tracker is more active, sometimes the distribution one is (usually that's a sign of a completely dead upstream). The best path here, that I'm aware of, is good communication between distro package maintainers and their upstreams. We (the Debian Python community) have a team mailing list [email protected] and hang out on IRC in Developers are probably patching tools for good reasons, to solve problems. They should be able to motivate those changes to the upstream. Maybe the changes are desirable in a distribution, and undesirable upstream.
Yeah, de-vendoring is a good example of this. From a simplicity and security PoV, we want <=1 version of every thing we carry in a distribution. It means less work to fix security issues (and it's obvious when the work is complete). Distributions bring code together into a coherent system. The ideal there is again, one version of each thing. We don't always achieve that, we need to be pragmatic. But it's a goal. Upstream code lives in a wild, uncontrolled environment where such things are impossible.
Oops. Fair enough. I was treating this as a regression. Everything has worked for the last 8 years, and now it doesn't any more. I wanted to contribute a patch to fix it and suddenly found myself in a foreign code-base that I need to get by bearings in.
Oops. Misread you there. Missed the "without". We want to support all of these things, ideally, in order of recommendation:
|
|
So what's next to get us moving ahead on this? I don't follow IRC myself but if anyone has any questions about repackaging virtualenv I'm always happy to help and answer. |
|
By the way using a distribution configuration could solve all three of those (virtualenv ignores that config) without needing to patch the source code. |
|
Did some testing and changed the sysconfig patch to be a little more accurate (for 2.7 and 3.6). https://salsa.debian.org/debian/pypy/-/commit/dcff89a62337630f85922415b5173176d09ea026 We still don't have There's still probably an opportunity to make better use of |
|
👍 ok. To be fair I wouldn't say the patch is to support virtualenv 20, it would be more accurate to say to fix a previous faulty patch that pointed system stdlib paths to the wrong folders. Any tool using sysconfig stdlib path would be similarly broken. |
|
By the way why would it require reverting pypy upstream patches? Their sysconfig paths are accurate to reflect the pypy layout. Generally distutils.conf is reserved for downstream distribution customisations, as is the case here with Debian. |
Yep, totally agree with that. But that's far more detail than a Debian changelog needs. From an outsider's (e.g. the user)'s point of view, this patch fixed a regression with virtualenv 20, by changing stdlib paths in sysconfig.
You probably don't want to know the answer to this question... It's because of it's interaction with the check for |
Issue
virtualenv 20 doesn't support Debian's pypy (2.7) packages (which I maintain).
I'm completely new to the virtualenv 20 codebase. So, not entirely sure what's going on here. The old version was a horrible mess, but I knew my way around it :)
Environment
Provide at least:
pip listof the host python wherevirtualenvis installed: N/AOutput of the virtual environment creation
Make sure to run the creation with
-vvv --with-traceback:py_info:
{ "platform": "linux2", "implementation": "PyPy", "pypy_version_info": [ 7, 3, 1, "final", 0 ], "version_info": { "major": 2, "minor": 7, "micro": 13, "releaselevel": "final", "serial": 42 }, "architecture": 64, "version": "2.7.13 (7.3.1+dfsg-1, Apr 10 2020, 20:05:43)\n[PyPy 7.3.1 with GCC 9.3.0]", "os": "posix", "prefix": "/usr/lib/pypy", "base_prefix": null, "real_prefix": null, "base_exec_prefix": null, "exec_prefix": "/usr/lib/pypy", "executable": "/usr/bin/pypy", "original_executable": "/usr/bin/pypy", "system_executable": "/usr/bin/pypy", "has_venv": false, "path": [ "/home/stefanor/git/virtualenv/src/virtualenv/discovery", "/usr/lib/pypy/lib_pypy/__extensions__", "/usr/lib/pypy/lib_pypy", "/usr/lib/pypy/lib-python/2.7", "/usr/lib/pypy/lib-python/2.7/lib-tk", "/usr/lib/pypy/lib-python/2.7/plat-linux2", "/usr/local/lib/pypy2.7/dist-packages", "/usr/lib/pypy/dist-packages" ], "file_system_encoding": "UTF-8", "stdout_encoding": "UTF-8", "sysconfig_paths": { "stdlib": "{base}/../../local/lib/pypy{py_version_short}/lib-python", "platstdlib": "{base}/../../local/lib/pypy{py_version_short}/lib-python", "purelib": "{base}/../../local/lib/pypy{py_version_short}/lib-python", "platlib": "{base}/../../local/lib/pypy{py_version_short}/lib-python", "include": "{base}/../../local/include", "scripts": "{base}/../../local/bin", "data": "{base}/../../local" }, "sysconfig_vars": { "base": "/usr/lib/pypy", "py_version_short": "2.7", "PYTHONFRAMEWORK": "", "implementation_lower": "python" }, "distutils_install": { "purelib": "site-packages", "platlib": "site-packages", "headers": "include", "scripts": "bin", "data": "" }, "system_stdlib": "/usr/lib/pypy/../../local/lib/pypy2.7/lib-python", "system_stdlib_platform": "/usr/lib/pypy/../../local/lib/pypy2.7/lib-python", "max_size": 9223372036854775807, "_creators": null }Presumably this is caused by our virtualenv parsing our deb layout.
To support users installing packages as root, outside a virtualenv, we point the purelib path at
/usr/local/libthrough thepypy-localsysconfig layout:https://salsa.debian.org/debian/pypy/-/blob/debian/debian/patches/distutils-install-layout
We do the same thing for PyPy3, but for some reason, virtualenv seems to know about that and handle it correctly.
The text was updated successfully, but these errors were encountered: