Allow PEP 508 URL spec as editable by uranusjr · Pull Request #9471 · pypa/pip

uranusjr · 2021-01-18T09:47:26Z

See #1289 (comment)

src/pip/_internal/req/constructors.py

sbidoul

Looks good, thanks!.

src/pip/_internal/req/constructors.py

uranusjr · 2021-01-18T11:06:01Z

Updated for review comments.

sbidoul · 2021-01-18T11:20:06Z

Do we have a natural place in the documentation for that ?

uranusjr · 2021-01-18T11:53:06Z

There’s a section in pip install on editable installs, I’ll update it to use the PEP 508 format. (Should we keep the #egg= format? I’m inclined not to because our next step is to deprecate it.)

sbidoul · 2021-01-18T11:56:35Z

Should we keep the #egg= format? I’m inclined not to because our next step is to deprecate it.

👍 to drop #egg= references from the doc now. That's a good way to check that we have an alternative for all documented use cases.

sbidoul · 2021-01-18T12:12:36Z

src/pip/_internal/req/constructors.py

+                name = f"{req.name} ; {req.marker}"
+            else:
+                name = req.name
+            return (name, req.url, req.extras)


Hm, on second look... this will let non VCS URLs go through ? So we need to merge #9436 first and see how we can do the link.is_vcs here too.

Oh, good point, Requirement() does not check if the URL is actually valid.

One thing we ca do here is to add an extra check, and only treat the string as PEP 508 if the URL part contains ://. This means pseudo URL strings will automatically fall back to the old parsing logic, and can be handled independently to this new logic.

@sbidoul I added an additional check to prevent pseudo-URLs from going through here. Do you think it’d help?

@uranusjr I have not tested (I whish I had more time for pip...) but this version would accept any URLs ? Editables URLs must be file:// or vcs+...:// urls only, correct ? And even file URLs must point to a local directory, not a file.

Correct. There are already checks in a later stage to error out on invalid URL values, so I think it’s fine to pass them through here.

pip/src/pip/_internal/req/constructors.py

Lines 116 to 119 in baaf66f

raise InstallationError(

f'{editable_req} is not a valid editable requirement. '

f'It should either be a path to a local project or a VCS URL '

f'(beginning with {backends}).'

uranusjr · 2021-01-18T12:23:47Z

I did a mass find-and-replace on the documentation (and rewrote a couple of paragraphs) to ditch egg= for PEP 508. The documentation now contains no mention of egg= at all.

BrownTruck · 2021-02-04T16:02:46Z

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

This prevents "pseudo URL" VCS requirements from going through the parser and cause issues later.

sbidoul · 2021-03-20T13:22:16Z

@uranusjr this does not seem to work, unfortunately:

$ pip install -e "pip-test-package @ git+https://github.com/pypa/pip-test-package" 
ERROR: Could not detect requirement name for 'git+https://github.com/pypa/pip-test-package', please specify one with #egg=your_package_name

Somehow parse_editable is being invoked twice: first with name @ url and then a second time with url only.

uranusjr · 2021-03-21T06:49:16Z

Thanks for catching this. A code path is building editables incorrectly with a bare URL instead of the whole requirement string. This “works” right now because the URL is the requirement string, but not anymore.

uranusjr · 2021-03-21T10:38:36Z

Hmm, turns out packaging actually contains some restriction on the URL and does not accept git+file:///.... This means it’s very difficult to write tests for pip install -e "NAME @ VCS+URL" without depending on network access. I’ve opened pypa/packaging#412 for this.

Update: Need to first resolve pypa/packaging#264.

BrownTruck · 2021-03-29T04:16:34Z

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

pradyunsg · 2021-04-02T09:43:30Z

FWIW, I suggest filing a new PR for this, since our CI pipeline changed significantly; and the existing test results would likely contribute to "noise" in the checks view. :)

uranusjr · 2021-04-02T12:42:03Z

The needed upstream fix in packaging is also taking a long time, and there’s no point working on this before that’s done.

sbidoul · 2021-04-02T12:49:42Z

there’s no point working on this before that’s done.

Why ? Implementing pip install -e "pip-test-package @ git+https://github.com/pypa/pip-test-package" is not blocked by the packaging issue. I personally could live with a test that needs the network for this.

uranusjr · 2021-04-02T16:34:50Z

My wording was bad, it’s more like I don’t think it’s a good investment of my time right now 😓

uranusjr mentioned this pull request Jan 18, 2021

refactor pip's use of "#egg=" #1289

Open

uranusjr commented Jan 18, 2021

View reviewed changes

src/pip/_internal/req/constructors.py Show resolved Hide resolved

sbidoul reviewed Jan 18, 2021

View reviewed changes

src/pip/_internal/req/constructors.py Outdated Show resolved Hide resolved

src/pip/_internal/req/constructors.py Outdated Show resolved Hide resolved

src/pip/_internal/req/constructors.py Show resolved Hide resolved

xavfernandez approved these changes Jan 18, 2021

View reviewed changes

uranusjr force-pushed the editable-pep-508 branch from d6c5bb3 to a320787 Compare January 18, 2021 11:05

uranusjr force-pushed the editable-pep-508 branch 2 times, most recently from bc6fecd to 457f8a6 Compare January 18, 2021 11:09

sbidoul reviewed Jan 18, 2021

View reviewed changes

BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 4, 2021

uranusjr added 3 commits February 8, 2021 17:24

Allow PEP 508 URL spec as editable

9e97705

Update docs to always use PEP 508 instead of #egg=

9b11a41

Make sure 'NAME @ URL' actually resembles a URL

9defcc0

This prevents "pseudo URL" VCS requirements from going through the parser and cause issues later.

uranusjr force-pushed the editable-pep-508 branch from f894310 to 9defcc0 Compare February 8, 2021 09:27

pypa-bot removed the needs rebase or merge PR has conflicts with current master label Feb 8, 2021

piotr-dobrogost mentioned this pull request Feb 25, 2021

Fix --target not working with --editable #9636

Merged

Build editable from a req string, not just the URL

0fcde33

BrownTruck added the needs rebase or merge PR has conflicts with current master label Mar 29, 2021

pradyunsg added the S: awaiting response Waiting for a response/more information label Apr 2, 2021

uranusjr closed this Apr 2, 2021

sbidoul mentioned this pull request Jul 8, 2021

Document how to install provides_extras from local wheel file #9698

Merged

uranusjr mentioned this pull request Sep 12, 2021

pip freeze outputs deprecated form of VCS requirement #9106

Open

github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2021

	raise InstallationError(
	f'{editable_req} is not a valid editable requirement. '
	f'It should either be a path to a local project or a VCS URL '
	f'(beginning with {backends}).'

Conversation

uranusjr commented Jan 18, 2021

Uh oh!

Uh oh!

sbidoul left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

uranusjr commented Jan 18, 2021

Uh oh!

sbidoul commented Jan 18, 2021

Uh oh!

uranusjr commented Jan 18, 2021

Uh oh!

sbidoul commented Jan 18, 2021

Uh oh!

sbidoul Jan 18, 2021

Choose a reason for hiding this comment

Uh oh!

uranusjr Jan 18, 2021

Choose a reason for hiding this comment

Uh oh!

uranusjr Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

uranusjr Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

sbidoul Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

uranusjr Feb 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

uranusjr commented Jan 18, 2021

Uh oh!

BrownTruck commented Feb 4, 2021

Uh oh!

sbidoul commented Mar 20, 2021

Uh oh!

uranusjr commented Mar 21, 2021

Uh oh!

uranusjr commented Mar 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BrownTruck commented Mar 29, 2021

Uh oh!

pradyunsg commented Apr 2, 2021

Uh oh!

uranusjr commented Apr 2, 2021

Uh oh!

sbidoul commented Apr 2, 2021

Uh oh!

uranusjr commented Apr 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

uranusjr Feb 25, 2021 •

edited

Loading

uranusjr commented Mar 21, 2021 •

edited

Loading