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.

Sign up for GitHub

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

Jump to bottom

Fix --user --editable installs for pep-517 #9990

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Fix --user --editable installs for pep-517 #9990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
701a472
Fix editable build for pep-517
SimonSegerblomRex May 17, 2021
7508fb7
Add news file
SimonSegerblomRex May 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/7953.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This change will fix issues with --user --editable installs.
2 changes: 1 addition & 1 deletion src/pip/_internal/distributions/sdist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Author

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...

Copy link
Author

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.)

Copy link
Member

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.

Copy link
Member

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.

Copy link
Author

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.

Copy link
Member

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.

Copy link
Author

@SimonSegerblomRex SimonSegerblomRex May 18, 2021 •
edited
Loading

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.

Copy link
Member

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.

Copy link
Author

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.

Copy link

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 ?

if should_isolate:
self._setup_isolation(finder)

Expand Down