Thanks for continuing to push us forward on this, Thomas :)

A small PEP readability request: given how the number of hooks has
grown, could we get a section that just lists the required hooks and
the optional hooks?

Alternatively, give each hook its own subsection under "Build backend
interface", so the table of contents at the top of the PEP serves as a
quick summary.

On 16 June 2017 at 06:43, Nathaniel Smith <n...@pobox.com> wrote:
> On Thu, Jun 15, 2017 at 6:12 AM, Thomas Kluyver <tho...@kluyver.me.uk> wrote:
>> Yes, it's PEP 517 again! Here's the current text:
>>
>> https://www.python.org/dev/peps/pep-0517/
>>
>> We currently say:
>>
>>> All other hooks [except get_build_requires] are executed in an environment 
>>> which contains both the bootstrap requirements specified in the 
>>> pyproject.toml hook and those specified by the get_build_requires hook.
>>
>> It's not clear to me whether this should be required for the build_sdist
>> and prepare_build_files hooks, nor whether there are any adverse
>> consequences of specifying it like this anyway. Thoughts?
>
> I think we should rename get_build_requires to
> get_build_wheel_requires, and add a get_build_sdist_requires. And the
> rule would be:
>
> get_build_sdist_requires: can assume build-system.requires are available
> get_build_wheel_requires: can assume build-system.requires are available
>
> build_sdist: can assume build-system.requires and
> get_build_sdist_requires are available
> prepare_wheel_metadata, build_wheel: can assume build-system.requires
> and get_build_wheel_requires are available

+1 from me

> Rationale: (a) conceptually the sdist and wheel phases are totally
> separate, so we shouldn't couple them by requiring a single hook to
> provide the union of requirements for both. (b) there are known cases
> where building an sdist has different requirements than building a
> wheel. Examples: packages that run cython at sdist generation time.

Another example: Thomas expects flit to require VCS interaction
support for sdist generation, but not for wheel building (or wheel
build file preparation).

> It's not immediately obvious to me how prepare_build_files would fit
> into this; if it's only supposed to be used for building from an
> sdist, then it's like an extra half-phase in between the sdist and
> wheel phases -- maybe it's more part of the wheel phase and should get
> the wheel requirements? Is that the only time it's used? I guess I'll
> wait to worry about it until after I see how people respond to my
> argument in the other thread that prepare_build_files shouldn't exist
> at all :-).

As with get_build_requires, this was less ambiguous when "build_wheel"
was the only build hook. Now that we have "build_sdist" as well, then
it may make more sense to rename it to "prepare_wheel_input_files".

Something else that I believe the PEP currently leaves implicit is the
assumptions that backend hook implementations are allowed to make
regarding the current working directory when they're invoked.

As I understand it, there are three defined possibilities:

- original source tree (potentially VCS metadata, no PKG-INFO file)
- unpacked sdist (no VCS metadata, PKG-INFO file)
- prepared wheel input files (no VCS metadata, maybe PKG-INFO file)

Given those options, the hooks that can be called given a particular
kind of input directory are:

* original source tree:
    * all hooks
* unpacked sdist:
    * all hooks
* prepared wheel input files (if `prepare_wheel_input_files` is defined):
    * prepare_wheel_metadata
    * build_wheel
    * NOT get_build_wheel_requires (see below)

My rationale for requiring get_build_wheel_requires to be called in
the source directory is that it means that `prepare_wheel_input_files`
can rely on those dynamic dependencies, appropriately reflecting it's
status as an optional substep of the `build wheel` process.

The available dependencies for each hook are then:

* build-system.requires only:
    * get_build_sdist_requires
    * get_build_wheel_requires

* build-system.requires + get_build_sdist_requires:
    * build_sdist

* build-system.requires + get_build_wheel_requires:
    * prepare_wheel_input_files
    * prepare_wheel_metadata
    * build_wheel

Make sense?

If folks agree with that, we could make the above explicit in the PEP
using a pair of dedicated paragraphs in each hook description:

* One starting "Current working directory: ..."
* One starting "Available dependencies: ..."

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
_______________________________________________
Distutils-SIG maillist  -  Distutils-SIG@python.org
https://mail.python.org/mailman/listinfo/distutils-sig

Reply via email to