On Sat, Aug 26, 2017 at 12:54 PM, Paul Moore <p.f.moore@gmail.com> wrote:
On 26 August 2017 at 20:17, Nathaniel Smith <njs@pobox.com> wrote:
Eh... I would really prefer something that's (a) more explicit about what specifically went wrong, and (b) harder to return by accident. It's not at all obvious that if the list of requirements is 'None' that means 'this build supports making sdists in general but cannot make them from this source tree but might still be able to make a wheel'. And if you forget to put in a return statement, then python returns None for you, which seems like it could lead to some super confusing error modes.
Well, we've had an extensive discussion about how frontends need to trust backends to get things right. I don't really see it as reasonable to now argue that backends might "forget" to return the right value - they might just as well "forget" to properly isolate builds...
It's not about division of responsibilities, it's about handling errors gracefully when they happen. There are three bins: - creating an sdist succeeded - creating an sdist failed for expected reasons, and a clever frontend might be able to handle the problem automatically if it understands what the problem is (sdist creation isn't supported in this case) and understands its goals (just trying to build a wheel really, so the sdist isn't crucial) - creating an sdist failed for unexpected reasons, that need a human to sort out (due to a broken system, or bugs – hey, they happen – or ...) The whole discussion has been about how we can most reliably distinguish between the second and third categories, and give good error messages for the third category. The argument for NotImplemented is that it avoids cases where some internal call raises NotImplementedError and it "leaks out" accidentally, causing a unexpected error to be incorrectly treated as expected error -- we don't want pip to be hiding real bugs in backend code. The argument for NotImplementedError is that it produces better error messages on buggy frontends. 'return None' is kind of the worst of both worlds, in that it's an easy thing to return accidentally, and it gives confusing error messages if the frontend fails to handle it properly. (Even more confusing, actually, because 'NoneType object has no attribute ...' is even harder to track down than 'NotImplementedType object has no attribute ...'.)
As regards an explicit description of what went wrong, why can't we just use the same reporting methods that we will for any other build issue (backends simply report the problem on stdout/stderr)? I don't see why the backend has to package up its error information and send it to the frontend to report, when we already have a perfectly effective way for backends to report errors and/or warnings to the user. If you're worried that the frontend might suppress the information (maybe because it's planning on falling back to a direct wheel build) then isn't that just the converse - backends need to trust frontends to do the right thing?
What I mean is more, if you're some random user and you see this in a build backend, what do you guess it means? def get_requires_for_build_sdist(config_settings=None): return None Now how about these? def get_requires_for_build_sdist(config_settings=None): return NotImplemented def get_requires_for_build_sdist(config_settings=None): raise NotImplementedError def get_requires_for_build_sdist(config_settings=None): raise SdistBuildNotSupported I mean, obviously return None will work. Basically anything that's different from "return a list or string" will work :-). That's what makes this a bikeshed topic, and I still think we're mostly just spinning our wheels here until Nick and Donald have a chance to hash something out that they both can agree on. But I really don't see any advantages to 'return None' compared to the other options that have been discussed -n -- Nathaniel J. Smith -- https://vorpus.org