On Sat, Aug 26, 2017 at 6:30 PM, C Anthony Risinger <c@anthonyrisinger.com> wrote:
On Aug 26, 2017 5:13 PM, "Nathaniel Smith" <njs@pobox.com> wrote:
On Sat, Aug 26, 2017 at 1:47 PM, C Anthony Risinger <c@anthonyrisinger.com> wrote:
Sure sure, I understand all that, and why we think we need some special error signal from `build_sdist`, as currently written.
What I'm suggesting, is maybe calling `build_sdist` without knowing if it can succeed is already a mistake.
Consider instead, if we make the following small changes:
1. `get_requires_for_build_*` is passed the sdist and wheel directories, just like `build_*`, giving them the chance to actually look at tree before deciding what other reqs might be necessary.
That's not a change, that's how it works :-).
2. `get_requires_for_build_*` returns None to signal `build_*` is unsupported (superceded by static reqs defined in TOML) and [...] to signal support (can be empty).
3. `get_requires_for_build_*` assumed to return None if missing (so optional and implies no support).
This is what I originally proposed, except you use None where I use NotImplemented, which has the disadvantages I noted earlier. Also, people didn't like the missing get_requires_for_build_* being treated as no-support, which makes sense, since we expect that get_requires_for_build_* won't be used very often. But one can switch the default here without affecting much else. The reason we want to let build_sdist report failure is just for convenience of backends who don't have any other reason to implement get_requires_for_build_sdist.
4. sdist reqs = `get_requires_for_build_sdist` (dynamic) + ??? (static)
5. wheel reqs = `get_requires_for_build_wheel` (dynamic) + `build-system.requires` (static)
build-system.requires contains the requirements that are always installed before we even try importing the backend, so they're available to all backend hooks equally.
6. If no reqs are found for sdist (no declared reqs in TOML and `get_requires_for_build_sdist` is missing or returns None), then `build_sdist` is unsupported.
7. If no reqs are found for wheel (no declared reqs in TOML and `get_requires_for_build_wheel` is missing or returns None), then `build_wheel` is unsupported. This one is a spec violation because at least one req is expected here (the backed itself).
The TOML requires aren't really useful as a signal about whether sdist specifically is supported. Plus I think we probably want to leave no-requires-in-TOML as a valid option for saying "I don't need anything installed" (maybe because the backend is shipped inside the source tree) rather than overloading it to have extra meanings. -n -- Nathaniel J. Smith -- https://vorpus.org