-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] recent versions of setuptools broke editable mode #3557
Comments
Something else that broke after setuptools 64 release. See tox-dev/tox#2479 for more details. Reported something at pypa/setuptools#3557
Something else that broke after setuptools 64 release. See tox-dev/tox#2479 for more details. Reported something at pypa/setuptools#3557
Thank you very much @dvarrazzo for reporting this issue. Could you please try to create a minimal/standalone reproducer (that does not involve using tox or pytest)? I tried to do the following: sudo apt update -y
sudo apt install -y libpq5 libpq-dev
cd /tmp && rm -rf /tmp/psycopg /tmp/empty_dir
git clone https://github.com/psycopg/psycopg.git /tmp/psycopg
cd /tmp/psycopg
python3.8 -m venv .venv
.venv/bin/python -m pip install -U pip
.venv/bin/python -m pip install -e ./psycopg
.venv/bin/python -m pip install -e ./psycopg_c
mkdir /tmp/empty_dir
cd /tmp/empty_dir # <-- To avoid any problems with automatic injection of CWD into `sys.path`
cat <<EOS | /tmp/psycopg/.venv/bin/python -
import psycopg, psycopg_c
print(psycopg.Cursor)
print(psycopg.AsyncCursor)
print(dir(psycopg_c))
EOS
# Output ==>
# <class 'psycopg.Cursor'>
# <class 'psycopg.AsyncCursor'>
# ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', '_psycopg', 'pq', 'sys', 'version'] And I can see that the module members mentioned in the tox issue seem to exist. Is there any chance this happens because Python automatically adds the current working dir to # Continuation of the previous script
# still inside /tmp/psycopg
mkdir psycopg
echo "raise SystemError('kaboom!')" > psycopg/__init__.py
/tmp/psycopg/.venv/bin/python -c 'import psycopg'
# Output ==>
# Traceback (most recent call last):
# File "<stdin>", line 1, in <module>
# File "/tmp/empty_dir/psycopg/__init__.py", line 1, in <module>
# raise SystemError('kaboom!')
# SystemError: kaboom! |
I thought that, and in my investigation with tox I tried to chdir away, but it didn't fix the problem. Anyway, does it mean that it is now mandatory to adopt a "src" style project organization? |
I don't think so... In terms of "plain Python" usage", I would say things should not change a lot. The flat-layout should have the same flaws as before, right? In terms of The limitations of the implementation are documented here. So far the biggest impact is for old-style namespace packages using |
So do you confirm that the package itself works on an editable install, but things start to get weird when a 3rd tool is added to the mix? (Maybe tox, maybe pytest?) Could it be the case one of these tools relies on a specific assumption about the editable installation? (I know that some static analysis tools will try to read a |
I can reproduce the problem with:
changing cwd works. Using
Using a rather older set of libraries, editable mode works as expected:
Tested on my laptop using Python 3.8.10 (Ubuntu 20.04 package) |
Thank you very much @dvarrazzo. So it is confirmed that it is happening because of the existence of the package CWD... I can see that a regular installation does not have the same problem. If I had to guess I would say that Python starts by creating a namespace package, but then when it finds the other folder (the actual package) on This behaviour does not seem to be very well defined/coherent though... For the time being let's consider these "accidental namespace packages" as a limitation of the editable mode. |
Ok, but what does it mean? Is this a regression that you will address (why would a random directory in cwd be more meaningful that an explicit entry in the pythonpath?) Or is this going to stay broken? Should we pin a dependency on setuptools < 64 for development? Should we rename our code directory and make all git history operations more painful in order to restore editable mode? Please let us know. |
I am seeking for advice about this matter, before taking any final decision.
This is how Python operates, right? Directories in CWD are meant to take precedence over other things in the site-packages. We can do the following test: rm -rf /tmp/development
mkdir -p /tmp/development/pkgA/pkgA
cd /tmp/development
touch pkgA/pyproject.toml
echo "x = 42" > pkgA/pkgA/a.py
# --- Simulate regular installation ---
python3.8 -m venv .venv
cp -r pkgA/pkgA .venv/lib/python3.8/site-packages/
tree .venv/lib/python3.8/site-packages/pkgA
# .venv/lib/python3.8/site-packages/pkgA
# └── a.py
# --- Experiment ---
rm -f /tmp/workdir
mkdir -p /tmp/workdir/pkgA
echo "raise ValueError('kaboom')" > /tmp/workdir/pkgA/a.py
cd /tmp/workdir
/tmp/development/.venv/bin/python -c 'from pkgA import a; print(a.x)' # <------ error
# ValueError: kaboom If we add a
You don't need to rename your directory. Right now, you can try |
I don't think so, no. This is not how Python used to operate until a couple of weeks ago. Giving precedence to directories in cwd seems a design error and a security problem. A program would change its behaviour according to the cwd and an attacker could convince someone to run a program from their directory to hijack it. It is the same reason why cwd is not in PATH: it's considered insecure since MS-DOS time. |
I don't see such a thing:
|
Sorry for that. I never remember it by heart :/ These are the options, copied straight from the docs: pip install -e . --config-settings editable_mode=strict
pip install -e . --config-settings editable_mode=compat |
Both these options work. The documentation says about strict: "The exact details of how this mode is implemented may vary"; about compat: "The compat mode is transitional and will be removed in future versions of setuptools, it exists only to help during the migration period". So it seems that default is broken and these options are not reliable solutions. |
Hi @dvarrazzo , thank you very much for testing these solutions and confirming that they work. The implementation details for both
Indeed, the opposite is quite true: if we don't commit to a particular mechanism, as Python and the ecosystem evolves, setuptools can select a different approach for the implementation, and potentially surpass existing limitations. This should be no different from any functionality implemented in a Python project: the public API is the contract and the exact implementation details may vary. It does not make these functionality more or less reliable. If you consider that "the default solution is broken", than you also have to consider that the legacy behaviour ( Editable installs are complicated and all the existing implementation mechanisms in the ecosystem have severe limitations. If you go around asking people what are their requirements for editable installs, you are going to realise that is technically impossible to meet all the requirements at the same time. You have to select a subset. Right now setuptools offers two options for the user to select: the default + strict. They are equally reliable and they both satisfy just a subset of the requirements. But togheter they should cover the entire list. |
@abravalheri I have reported a regression. Things were working previously and now they don't. I appreciate the hard work behind the hard problem of packaging, don't get me wrong. Regressions happen. It is not true that a local directory used to have, or even has, precedence over installed packages:
Since a certain version, the default behaviour of editable packages changed and now presents a noticeable difference w.r.t. normally-installed packages, and present difficulties developing certain repositories layout (a monorepo where the directory name matches the package name, for instance - like psycopg 3 repository). So, this is a regression. My questions are:
Thank you |
Maybe I oversimplified things, used the wrong words, and accidentally made things more confusing, sorry for that. There are a few ways that you can run Python and having something automatically inserted as the first entry to
This is also documented in There are a few ways you can avoid that. After a quick search I could not find in the documentation anything explicitly about the orders the import machinery traverse
I think you are correct here. In fact with some social engineering this can be possible. python -m pip <pip arguments> This makes the following attack possible: rm -rf /tmp/workdir
mkdir -p /tmp/workdir/pip/
cd /tmp/workdir
touch pip/__init__.py
echo 'print("... doing malicious stuff...")' > pip/__main__.py
python -m pip --help
# ... doing malicious stuff...
Thank you very much for the understanding and the patience. I am very sorry for the inconvenience. This subject is a bit tricky... I don't think we can easily classify things in terms of "backwards" compatibility or "regression" because we are talking about the interaction between 2 different tools + a standard that came into play a few years ago. In terms of implementation, the behaviour you used to experience in the past is the behaviour of However, motivated by the approval of PEP 660, For a while, there was discussion on what to do on the setuptools side and a lack of consensus (that I still believe exists). So I had to make a judment about how to implement PEP 660. The result that you observe today is what I think to be a compromise between arguments on the two fields1. It "fixes" some limitations of the previous approach, but in turn, it has its own limitations.
Some aspects of the current implementation are wanted, and I would definetely not want to throw the baby out with the bathwater.
It is expected that every single editable installation method will have flaws and that the users might have to switch between different editable modes to achieve different objectives. I plan to investigate different alternatives to see if we can overcome this limitation but this may take some time. If we overcome the limitation, it should definetely be added to the tests. That is my personal plan, but of course, other setuptools maintainers might have different ideas... @dvarrazzo, I am sorry that I cannot provide you definitive answers at this stage. Finally, as long as Footnotes
|
I am not able to install https://github.com/tensorflow/tfx-bsl in editable mode. Setting SETUPTOOLS_ENABLE_FEATURES="legacy-editable" fixed the issue. |
Just to cross link it, there are some limitations in how What's at stake for me is the ability to use vscode/pylance with my editable repos: Per recommendation above, I tried: export SETUPTOOLS_ENABLE_FEATURES="legacy-editable"
pip install -e . but I get the error:
based on some quick searching, I'm not clear if there's a way to get |
Hi @qci-amos, please note the following caveat for
Pip no longer allows backends to not implement PEP 660. So that is no longer an option.
I think the right question here is to ask how |
Well to be clear, what I don't currently have a solution for is how to use |
shouldn't this just be an option:
then poetry can pass that flag along when installing? or even
so we have full control over how packages are installed going forward as tooling changes without poetry needing to know about all possible flags probably should even have a |
Not really. This does not look like something setuptools would work with. Setuptools is a build backed and does not directly work with dependencies. It also does not do any installation. |
Yes, it has a config settings flag. Look at docs for the build command
…On Sun, Mar 31, 2024, 10:37 PM Anderson Bravalheri ***@***.***> wrote:
shouldn't this just be an option:\n\nmypackage = { path = "../mypackage/",
develop = true, editable_mode=strict}
Not really. This does not look like something setuptools would work with.
Setuptools is a build backed and does not directly work with dependencies.
—
Reply to this email directly, view it on GitHub
<#3557 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAMMUOEFZAPT6C6UEM4KR3Y3DB5RAVCNFSM57HCJ6T2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBSHEYDKNZWGQ2A>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Even though you can pass a dict to setuptools via Setuptools handle the build of a single distribution as a whole. It does not make sense to express editable installations piecewise, package by package. There is already If you want to pass |
It seems like the regression comes from the "new-style" I think the current behavior is unintuitive and a bug. When the user requests an editable installation, it should take precedence before |
The new implementation of editable packages with custom meta path finders has caused issues such as pypa#3557 because the meta path finders are installed _after_ Python's default `PathFinder`, causing them to be treated as fallback handlers rather than the preferred method of importing the package. The default `sys.meta_path` order is `BuiltinImporter`, `FrozenImporter`, `PathFinder`. This changes the `install()` method to insert `_EditableFinder`s after `BuiltinImporter` so they should take precedence over directories and files in the working directory.
Please note that |
Yes, although it seems to me that the system was designed for adding finders/loaders for filetypes that
Could you elaborate on this? I assume you're referencing pypa/pip#11812 here. Does that bug occur with both |
I have a different impression. It is well documented when it comes to loading Python packages, and for editable installs, it is mentioned in the PEP that standardised editable installs as one of the possible forms of implementations. However, I agree that it is not straightforward to finders or loaders for namespace packages... that is in my opinion an area that the import machinery is lacking.
Yes, please see the details on that issue. I believe that both approaches would be subject to that problem, but I have not tested. |
setuptools version
65.2
Python version
3.9.13
OS
Ubuntu
Additional environment information
No response
Description
Since some recent releases (likely 64, when PEP-660 was implemented), installing psycopg with -e results in a broken package, where
psycopg
is an empty namespace.This is an example of broken run. This is a similar run whose difference is installing
psycopg
without the-e
flag, in order to test a depending package.Expected behavior
See above
How to Reproduce
A more complete reproduction with stable git reference can be found in tox-dev/tox#2479.
Output
See above
The text was updated successfully, but these errors were encountered: