Skip to content
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

editable_wheel is broken in 68.1.0 #4019

Closed
emontnemery opened this issue Aug 17, 2023 · 4 comments · Fixed by #4020
Closed

editable_wheel is broken in 68.1.0 #4019

emontnemery opened this issue Aug 17, 2023 · 4 comments · Fixed by #4020
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.

Comments

@emontnemery
Copy link

emontnemery commented Aug 17, 2023

setuptools version

setuptools==68.1.0

Python version

Python 3.11

OS

Linux

Additional environment information

No response

Description

setuptools 68.1.0 broke editable_wheels due to commit db3743a from #3995

To my understanding, the problem is that _find_nested_spec passes the wrong path to PathFinder.find_spec(rest, path=[parent_path])

Expected behavior

Pass the full path to PathFinder.find_spec.
Doing that requires a lookup to happen first though, which pretty much defeats the purpose of db3743a though as I understand it, which was to let Python handle the case insensitive lookup.

How to Reproduce

This is how PathFinder.find_spec treats its input path, note how it only cares about the tail of the name to lookup:

python3 -vv
>>> from importlib.machinery import PathFinder
>>> PathFinder.find_spec("components.persistent_notification.loader", ["/home/erik/development/home-assistant_fork/"])

Output

# trying /home/erik/development/home-assistant_fork/loader.cpython-311-x86_64-linux-gnu.so
# trying /home/erik/development/home-assistant_fork/loader.abi3.so
# trying /home/erik/development/home-assistant_fork/loader.so
# trying /home/erik/development/home-assistant_fork/loader.py
# trying /home/erik/development/home-assistant_fork/loader.pyc
@emontnemery emontnemery added bug Needs Triage Issues that need to be evaluated for severity and status. labels Aug 17, 2023
@abravalheri
Copy link
Contributor

Hi @emontnemery, thank you for opening the issue.

Could you please provide the full reproducer? In your example it is not clear where the home-assistant_fork comes from, or components.persistent_notification.loader...

@emontnemery
Copy link
Author

emontnemery commented Aug 17, 2023

I was trying to illustrate that the way in which _EditableFinder._find_nested_spec calls importlib.machinery.PathFinder.find_spec does not seem to be correct.

Here's a reproduction of the actual error we see:

Prerequisite:

Python 3.11 is the current interpreter, for example selected by pyenv

Step by step

mkdir tmp
cd tmp
python3 -m venv .venv
source .venv/bin/activate
pip3 install setuptools==68.1.0
git clone https://github.com/home-assistant/core
cd core
git checkout d44847bb239e180f86c0979bee443bd4c966ecdc
cd ..
pip3 install -e core
python3
# try to load a module which does not exist
>>> from homeassistant.components.persistent_notification import config
>>> config
<module 'homeassistant.components.persistent_notification.config' from '/home/erik/tmp/core/homeassistant/config.py'>

The problem is that, in this example, _EditableFinder._find_nested_spec calls PathFinder.find_spec like this:

PathFinder.find_spec("components.persistent_notification.config", ["/home/erik/tmp/core/homeassistant/"])

Which makes PathFinder.find_spec look for a config.py in /home/erik/tmp/core/homeassistant/ instead of in /home/erik/tmp/core/homeassistant/components/persistent_notification.
In other words, PathFinder.find_spec only cares about the tail of the name when a path list is passed to it.

https://docs.python.org/3/library/importlib.html#importlib.machinery.PathFinder.find_spec

@abravalheri
Copy link
Contributor

abravalheri commented Aug 17, 2023

Hi @emontnemery, thank you very much for the reproducer.

Could you please try if #4020 works for you?
You can install it with python -m pip install 'setuptools @ https://github.com/abravalheri/setuptools/archive/refs/heads/issue-4019.zip'

(You can also use setuptools @ https://github.com/abravalheri/setuptools/archive/refs/heads/issue-4019.zip in your pyproject.toml's [build-system] requires.

@emontnemery
Copy link
Author

Thanks @abravalheri, that works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants