
On Sat, Jun 17, 2017 at 7:41 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
On 17 June 2017 at 17:07, Nathaniel Smith <njs@pobox.com> wrote:
On Fri, Jun 16, 2017 at 11:28 PM, Donald Stufft <donald@stufft.io> wrote:
On Jun 16, 2017, at 11:08 PM, Nathaniel Smith <njs@pobox.com> wrote:
(because the legacy setup.py command errors out, because the build_sdist hook is missing, because the build_sdist hook errors out...); probably falling back on shutil.copytree.
Ah okay. This is where the disconnect was. I assumed if a project was using a PEP 517 build tool the result of a build_sdist failure (or a build_wheel failure) would be surfacing an error to the end user, not some sort of fallback.
What we might be able to get away with in the PEP 517 transition is to provide an explicit way of signalling "build_sdist failed because that's just not a supported operation", so that pip could distinguish that from "something unexpected blew up" and only fall back in the first case.
That's what `prepare_wheel_input_files` gives us: if the backend doesn't implement it, then `build_sdist` is expected to always succeed, and any errors should be reported to the end user.
Well, yeah, this is a discussion about a potentially simpler/more flexible alternative to prepare_wheel_input_files, so I'd hope that it gives us something similar :-).
OTOH, if `build_sdist` can fail (e.g. due to missing dependencies that aren't needed just to build a wheel), then the backend should offer a `prepare_wheel_input_files` that offers the expected guarantee (i.e. it will only fail for reasons that should be reported to the user attempting to do the build).
The frontend then *won't* have a fallback copy strategy for PEP 517 backends - if `prepare_wheel_input_files` is present and fails, or if it's missing and `build_sdist` fails, then you have a failed wheel build on your hands.
The legacy copying strategy would then only be used when falling all the way back to the `setup.py` interface, not as an attempt to continue the build in the face of a backend bug.
This all gets murkier if you try to make the build_sdist hook more complex instead of just having two different hooks for frontends to call that tell the backend what the frontend actually wants (i.e. either an actual sdist, or just a clean set of exported files to use as input to a wheel build).
Hmm, I don't think it does get murkier. Here's how I'm thinking about it: Everyone agrees: - build_sdist's semantics are that it builds an sdist, it's up to the caller to figure out why they want that - the build_sdist hook can be missing (unless we're planning to enforce its presence from the start, but that seems unlikely given that pip currently has no sdist handling at all), which is a pretty obvious signal to the caller that it's not supported - there's some problem with what to do for flit, where "can I build an sdist" is determined at runtime. I guess it could make the build_sdist attribute be defined via a __getattr__ method that sniffs the source tree before deciding whether the attribute should be present, but that seems unpleasant. The current spec's solution: - Let's define an exhaustive list of all the reasons you might want to make an sdist: 1) you're a hypothetical future version of pip that has decided to implement some specific build-from-unpacked-source-tree semantics 2) the user has specifically requested an sdist, so if one can't be made then the only thing to do is to fail and pass on some unstructured error log 3) that's it, we've made a list of all possible front-end semantics. - Then define a hook that does (1) and a hook that does (2). My suggestion: - Let's keep the spec around build_sdist very simple, all it does is worry about building an sdist and doesn't care at all what it's for, that's the frontend's job - Within that scope, one very well-defined thing that a backend might want to say to a frontend is "this backend doesn't support building an sdist from this source tree". And we even support this already, just in a kinda awkward way requiring __getattr__. - so instead let's provide a standard way to do that - and then backends and the standard don't (yet) need to know anything about pip's particular use case for sdists of wanting to potentially in the future use them as an intermediate step in building wheels from source directories; all the necessary logic is a concern for pip (and possibly some future PEP), not for PEP 517. (Hopefully this also answers Paul's objection – I'm explicitly trying to take all the fallback-y logic out of the PEP, and replace it with a nicely semantic "that operation isn't supported" signal. It just so happens that one of the things pip could do with that signal is to decide to use an alternative method of accomplishing its goal that doesn't involve creating an sdist.) -n -- Nathaniel J. Smith -- https://vorpus.org