On 24 August 2017 at 23:11, Thomas Kluyver <tho...@kluyver.me.uk> wrote: > Nathaniel seems to be busy with other things at the moment, so I hope he > won't mind me passing on this list of things he'd like to resolve with > the draft PEP. I'll quote his comments and put my responses inline. > >> - needs some sort of NotImplemented(Error) handling specified > > We've gone round return-value vs exception a few times, but I don't > think that debate is particularly heated, so it probably just needs > someone to say "this is what we're doing". I can be that someone if > needs be. But does anyone feel strongly about it?
Aye, I do, and it should be "raise NotImplementedError('Explanation of the failed check')" Rationale: - Python isn't C or Go, so we indicate failures with exceptions, not error codes (NotImplemented is an necessary performance hack for operand coercion in tight loops, not an example to be emulated in other APIs) - allows the backend to provide information on what went wrong - means an unhandled backend error results in a traceback pointing to where the build failed, not some later point in the frontend code - if a backend developer is sufficiently worried about accidentally propagating NotImplementedError that they want to pretend they're not writing Python any more, they can use this idiom: def public_hook_api(*args, **kwds): try: result, error_msg = _internal_hook_implementation(*args, **kwds) except NotImplementedError as exc: raise RuntimeError("Unexpected NotImplementedError") from exc if result is NotImplemented: raise NotImplementedError(error_msg) return result That provides the backend with all the same assurances against accidentally letting NotImplementedError escape that a return code based public API would, without frontends even needing to be aware of the backend developer's aversion to reporting errors as exceptions. >> - The out-of-tree build example (that makes an sdist and then builds >> it) seems like exactly the kind of implementation that Donald says he >> doesn't want to see? OTOH I think Nick said he wants to see a more >> elaborated specification of out-of-tree build strategies with this >> specifically as one of them. Not really - I raised that prospect because Nathaniel was insisting that out-of-tree builds would be too hard for backend developers to implement, and I pointed out that they really weren't that complicated: - if you're wrapping a backend that supports them, pass down the directory setting - if you're not, implement them as semantically equivalent to build sdist -> unpack sdist -> build wheel - since build_sdist can fail with NotImplementedError, out-of-tree builds are also permitted to fail that way >> - Nick has suggested that the to-be-defined NotImplemented(Error) >> handling can be used by build_wheel to indicate that it can't do >> out-of-tree builds, so maybe the frontend should try an in-tree build >> instead. I don't know how to convert this into a real spec, though -- >> like in general, if I'm a frontend and I call `hook(*args, **kwargs)` >> and it says "can't do that", then how do I know what the problem is >> and what I should try instead? The fallback chains are defined by frontends, not the spec, and they depend on: - what the frontend is trying to do - what the fronted is trying to prevent For the case of "build a wheel from a source tree" for example, a reasonable fallback chain might be: - try build_sdist - if that raises NotImplementedError, try an explicitly out-of-tree build_wheel call - if that also raises NotImplementedError, try an unqualified build_wheel call It would also be equally reasonable to skip either or both of the first two options. >> - What happens if prepare_build_metadata returns NotImplemented / >> raises NotImplementedError? Up to the frontend, but failing outright seems reasonable (since there isn't any real reason to expect generating the entire wheel to succeed if generating the metadata failed) >> - I don't understand how out-of-tree builds and prepare_build_metadata >> are supposed to interact. They don't, since the backend should only implement prepare_build_metadata if it can generate the metadata without actually triggering a full build of all the binary artifacts. >> - Also, AFAICT the out-of-tree build feature has no motivation >> anymore. The only mentioned use case is to support incremental build >> features in automatic build pipelines like Travis, but there are a >> number of these build pipelines, and lots of existing build systems >> that support out-of-tree builds, and AFAICT no-one actually uses them >> together like this. (I think it's because most build systems' >> out-of-tree build features are designed on the assumption that there's >> a human running the show who will deal with any edge cases.) > > I believe the out-of-tree build option Nathaniel is referring to is the > build_directory parameter to build_wheel(). His proposed APIs remove > that parameter, and elsewhere in his email he describes that no-one > seems to want it: everyone thinks someone else wants it. > > By my understanding, the reasons for including the build_directory > parameter are: > > 1. Without an explicit build directory, the developers of pip are > concerned that build backends will modify the source tree and cause > issues which users report as bugs to pip. This is the motivation that Donald and Paul have indicated isn't necessarily valid any more, since they'd be OK with a setup where pip uses the following build model by default: 1. First try the explicit build_sdist -> unpack sdist -> build_wheel path 2. If build_sdist raises NotImplementedError, fall back to trying build_wheel directly > 2. A frontend-controlled build directory could be used for caching > intermediate build artifacts. This was a secondary argument after we had > the idea, and we've never really fleshed out how we expect it to work. > There's also a concern that if the first round of frontends always use > an empty tempdir, backends will end up relying on that. I think this > second argument is a weak one unless we spend the time to figure out the > details. > > Do other people see the situation in a similar way? How might we move > forwards on this? As long as Donald & Paul are OK with it for pip, I'm OK with omitting the build_directory parameter for now - since we're going to make supporting it optional regardless, that means it doesn't matter as much whether its in the initial iteration of the API or not. >> - needs some sort of conclusion to the srcdir-on-sys.path issue. > > While Nathaniel is in the minority regarding srcdir-on-sys.path, he > argues that most of us are assuming a default position without really > thinking through it, which is certainly true of me. I don't feel > strongly about this topic, but I do want to come to a conclusion. As > Nathaniel does feel strongly about it, does anyone object to either: > > A. Leaving the source dir off sys.path when loading the hooks, and > making a special backend which exposes hooks from the CWD. > B. Adding another key in the TOML file to control whether hooks can be > loaded from the source dir. I'm mainly interested in the way things fail, and who has the power to fix them when they break. - when the default is "source tree is set as sys.path[0]": - that's essentially the same as the status quo with setup.py - if a project's build process has a name shadowing problem, the publisher is going to hit that locally and fix it prior to release - when the default is "the source tree is not set as sys.path[0]", we know that: - any setuptools backend is going to have to ensure that the old default is in place when running setup.py - any backend that runs a Python script is going to end up with the CWD as sys.path[0] in that script anyway due to Python's default behaviour - if a project self-hosts its own build backend, we either have to say "that's not supported", or else provide a way to change the default So I think this is a case where we have an established precedent (i.e. the way setup.py currently works), and the potential for accidental name shadowing doesn't offer a sufficiently compelling rationale for changing that default. Cheers, Nick. -- Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia _______________________________________________ Distutils-SIG maillist - Distutils-SIG@python.org https://mail.python.org/mailman/listinfo/distutils-sig