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

Call for clarification: acceptable values for build-system.requires in pyproject.toml #6410

Closed
webknjaz opened this issue Apr 14, 2019 · 9 comments · Fixed by #7326
Closed
Labels
auto-locked Outdated issues that have been locked by automation C: PEP 517 impact Affected by PEP 517 processing PEP implementation Involves some PEP state: awaiting PR Feature discussed, PR is needed type: bug A confirmed bug or unintended behavior

Comments

@webknjaz
Copy link
Member

webknjaz commented Apr 14, 2019

Environment

  • pip version: 19.0.3
  • Python version: 3.7
  • OS: GNU/Linux

Description

It's not clear nor is clearly specified by PEP518. But I was having a need to have an in-tree build back-end.
So I've hacked it via installing with a relative path.
And it works!
Yet, @pradyunsg has pointed out that the way I used probably doesn't conform to PEP 508.
So I tried some other ways to point to the in-tree distribution. And those didn't work.

How to Reproduce

(this works)

[build-system]
requires = ["./build-aiohttp", ]
build-backend = "build_aiohttp.api"

# not yet supported, so it doesn't influence anything, it's for forward-compat:
backend-path = "./build-aiohttp"

But if instead of "./build-aiohttp" in requires I try any of "file://build-aiohttp", "file:///./build-aiohttp", "build_aiohttp @ file://./build-aiohttp", "build_aiohttp @ file:./build-aiohttp" pip fails to recognize those as installables.

Expected behavior

I don't know what to expect. The method which works seems to be undefined in PEPs so I probably shouldn't rely on it.

Pip may either decide to improve the filtering of requires option or document it being permissive...

P.S. Oh and, by the way, I was able to test my other PEP517 backend outside of the project tree via

[build-system]
requires = ["../fortunate_pkg"]

so this relative path feature proves to be quite useful for development/debugging purposes.

@pfmoore
Copy link
Member

pfmoore commented Apr 15, 2019

In-tree backend support was recently added to PEP 517. It's not yet supported in pip, however.

@webknjaz
Copy link
Member Author

Hi Paul, I do know that. My question is about whether it's legal to put a relative path in requires.

@pfmoore
Copy link
Member

pfmoore commented Apr 15, 2019

Ah, sorry, my misunderstanding. I don't believe so, PEP 518 states what can go in requires:

This key must have a value of a list of strings representing PEP 508 dependencies

So they aren't paths at all, and if pip is accepting paths, that's wrong.

@cjerdonek
Copy link
Member

Here is where pip's validation logic is currently (it only checks that it's a list of strings):

# Error out if requires is not a list of strings
requires = build_system["requires"]
if not _is_list_of_str(requires):
raise InstallationError(error_template.format(
package=req_name,
reason="'build-system.requires' is not a list of strings.",
))

@pfmoore
Copy link
Member

pfmoore commented Apr 15, 2019

I'm inclined to describe this as "user error" - which is not to say that pip couldn't give better messages, but it's the user's responsibility to supply valid input. I don't really think we need to do full PEP 508 validation here.

The odd sort-of-works result is probably because pip accepts more than just PEP 508 dependencies as "things to install", so this is arguably "behaviour undefined by the PEP that triggers an implementation defined result". The classic Undefined Behaviour nasal demons case ;-)

@webknjaz
Copy link
Member Author

Right, I'm just trying to decide whether to rely on it. Because, if it turns out broken in the next version, then it would affect already published sdists.

@pradyunsg pradyunsg added the S: needs triage Issues/PRs that need to be triaged label Jun 22, 2019
@chrahunt
Copy link
Member

@webknjaz you can use intreehooks:loader as was done for flit here. This should continue to work regardless of what pip does.

@chrahunt chrahunt added S: awaiting response Waiting for a response/more information type: question User question labels Jul 21, 2019
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Jul 21, 2019
@pradyunsg
Copy link
Member

I'm inclined to say let's actually abort in this edge case.

I don't want people trying things they shouldn't and it sort-of working.

@chrahunt
Copy link
Member

Agreed, especially since this use case is covered by intreehooks and (soon) backend-path.

@pradyunsg pradyunsg added C: PEP 517 impact Affected by PEP 517 processing PEP implementation Involves some PEP type: bug A confirmed bug or unintended behavior and removed S: awaiting response Waiting for a response/more information type: question User question labels Jul 21, 2019
@chrahunt chrahunt added the state: awaiting PR Feature discussed, PR is needed label Aug 12, 2019
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Dec 10, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: PEP 517 impact Affected by PEP 517 processing PEP implementation Involves some PEP state: awaiting PR Feature discussed, PR is needed type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants