On Jul 5, 2017, at 2:02 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:

On 5 July 2017 at 15:49, Donald Stufft <donald@stufft.io> wrote:
I’ve had a niggling feeling about this hook from the beginning, but I
couldn’t quite put my finger on it until Nathaniel’s email made me realize
it. I feel like this hook is really *only* useful for flit, and for other
projects it is largely either going to be completely redundant or be an
attractive nuisance that ends up only causing issues. It’s a pretty narrow
use case where this hook is both able to do something AND doesn’t have the
exact same requirements as build_sdist.

When I felt this was a more generic hook, I was OK with it, but I don’t
think it’s a good idea now that I’ve thought on it more and it feels
entirely ungeneric.

I don't think Thomas's plans for it are unusual, as it's normal for a
build system to only be aware of the input files that are actually
referenced by the build recipe, and also normal for published source
archives to include additional files that *aren't* used by the build
process for the binary artifacts.

Except in this case the build system and the thing that builds the binary artifacts are one in the same so it needs to know both of those things. This hook is only useful if you have two different mechanisms for declaring those types of files *AND* the requirements for the additional files imposes some other non-python level prerequisites on the system that could maybe be avoided.

Quite literally, the only case I can think of that fits into this is flit’s “I will use git to figure out additional files, but you will have to configure in a static file the name of the Python package (as in import package) that you’re distributing and I’ll just glom down that entire package directory”. I know setuptools/distutils doesn’t work this way nor do any of the plugins that I am aware of. Best I can tell numpy.distutils nor enscons.

Looking at https://github.com/takluyver/flit/blob/master/flit/sdist.py (added in https://github.com/takluyver/flit/pull/106) it appears that flit *also* doesn’t work this way, when it builds the sdist today it is looking at the VCS tracking and including any file mentioned in the VCS directory. So unless flit changes it’s logic so it only uses the “is this file in a VCS” for non installed files, it also can’t use this hook without the VCS tools installed and guarantee that the output is the same.

So not only is this a hook that I think is only going to be used by flit, but flit itself can’t even use it right now without changing the logic on how they generate sdists to no longer reference the VCS for what installable files get added. The more I dig into this, the more I think Nathaniel is correct and we’re trying to add a hook without any real world experience guiding it’s inclusion that doesn’t actually solve the problem it’s trying to solve and which it’s primary use case is creating foot guns.


If you'd prefer some external validation for the concept, I see the
"prepare_input_for_build_wheel" hook as fairly analagous to the
"%prep" phase in the process of building an RPM:
https://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25prep_section

The current difference is that we expect backends to be able to cope
with frontends *not* calling that implicitly when building from an
unpacked sdist.


I don’t think this is external validation at all and the only similarities I see between he two hooks is they both reference the concept of “preparing” in their names. The %prep’s hook in RPMs primary use case is running tar to unpack a tarball and running patch on the resulting unpacked tarball (in fact, this is so common they have a %autosetup which basically does that). It’s really an entirely different hook from what we’re discussing here.



Donald Stufft