-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix --user --editable installs for pep-517 #9990
base: main
Are you sure you want to change the base?
Fix --user --editable installs for pep-517 #9990
Conversation
@@ -29,7 +29,7 @@ def prepare_distribution_metadata(self, finder, build_isolation): | |||
self.req.load_pyproject_toml() | |||
|
|||
# Set up the build isolation, if this requirement should be isolated | |||
should_isolate = self.req.use_pep517 and build_isolation | |||
should_isolate = self.req.use_pep517 and build_isolation and not self.req.editable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break some other use-cases...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the bug is somewhere higher up and that build_isolation
should be False
here. (It's also possible to use self.req.user_supplied
.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think the bug should be fixed somewhere higher in the call stack. build_isolation
should not be True
when the requirement is editable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What use case does this break though? I feel this should work as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This broke. Maybe it would be ok to add and not self.req.user_supplied
instead, but I'll try find out why build_isolation
is set to the wrong value instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why it breaks? I wouldn’t expect that since it’s conceptually not different from any other editable installs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it looks like this:
First on main
:
> git status
On branch main
Your branch is up to date with 'origin/main'
> which python3
/usr/bin/python3
> python3 --version
Python 3.7.3
> python3 -m pip install .
Defaulting to user installation because normal site-packages is not writeable
...
Successfully installed pip-21.2.dev0
> python3 -m venv venv
> source venv/bin/activate
(venv) > python -m pip install -e .
Looking in indexes: https://pypi.org/simple
Obtaining file:///home/simonsm/pip
Installing build dependencies ... done
Getting requirements to build wheel ... done
Preparing wheel metadata ... done
Installing collected packages: pip
Attempting uninstall: pip
Found existing installation: pip 21.2.dev0
Uninstalling pip-21.2.dev0:
Successfully uninstalled pip-21.2.dev0
Running setup.py develop for pip
Successfully installed pip-21.2.dev0
(venv) >
Then with my change:
(venv) > deactivate
> rm -fr venv
> git checkout -
Switched to branch 'fix-editable-pep-517'
Your branch is up to date with 'origin/fix-editable-pep-517'.
> python3 -m pip install .
Defaulting to user installation because normal site-packages is not writeable
...
Successfully installed pip-21.2.dev0
> python3 -m venv venv
> source venv/bin/activate
(venv) > python -m pip install -e .
Looking in indexes: https://pypi.org/simple
Obtaining file:///home/simonsm/pip
Installing build dependencies ... done
Installing collected packages: pip
Found existing installation: pip 18.1
Not uninstalling pip at /home/<user>/pip/venv/lib/python3.7/site-packages, outside environment /home/<user>/pip/venv
Can't uninstall 'pip'. No files were found to uninstall.
Running setup.py develop for pip
Successfully installed pip-18.1
(venv) >
So pip doesn't pick up the user installation of pip in the venv with this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. The message doesn’t even make sense (the path is inside the environment?) so I’d probably look into why that error is triggered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem seems to be that the default value for build_isolation
is True
, so this works (on main
):
python3 -m pip install --no-build-isolation -e <path>
I guess the default value should still be False
when -e
is provided though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I... have no idea what is actually going on, just that the above suggestion with --no-build-isolation worked for me, when having a pyproject.toml a setup.cfg and a setup.py. So .. thanks I guess ?
For reasons that at-present escape me, pipenv insists on creating a stub pyproject.toml file. This file is a nuisance, because its mere presence changes the behavior of various tools. For instance, this stub file will cause "pip install --user -e ." to fail in spectacular fashion with misleading errors. "pip install -e ." works okay, but for some reason pip does not support editable installs to the user directory when using PEP517. References: pypa/pip#9990 pypa/pip#7953 As outlined in ea1213b, it is still too early for us to consider moving to a PEP-517 exclusive package. We must support older distributions, so squash the annoyance for now. (Python 3.6 shipped Dec 2016, PEP517 support showed up in pip sometime in 2019 or so.) Add 'pyproject.toml' to the 'make clean' target, and also delete it after every pipenv invocation issued by the Makefile. Signed-off-by: John Snow <[email protected]> Reviewed-by: Willian Rampazzo <[email protected]> Reviewed-by: Wainer dos Santos Moschetta <[email protected]> Message-id: [email protected] Signed-off-by: John Snow <[email protected]>
Hi!
This is just a quick fix based on @uranusjr comment here. I haven't really started digging deep into the source. Let me know if this is a proper solution and if I should look into writing some unit tests.
Fixes: #7953