On Sun, May 28, 2017 at 10:37 PM, Donald Stufft <don...@stufft.io> wrote:
>
> Bleh, I had it on my stack to respond to PEP 517, but life has been super
> hectic so I hadn’t gotten around to it.
>
> Here are a few thoughts FWIW:
>
> 1. Using the {package-name}-{package-version}.dist-info in the
> get_wheel_metadata() metadata is a mistake I think. In pip currently we have
> a bug we have not yet been able to track down because there is nothing
> systematically preventing both foobar-1.0.dist-info and foobar-2.0.distinfo
> from existing side by side in a build directory (or inside a wheel for that
> matter). Thus I think this naming scheme is a nuisance and we shouldn’t
> propagate it any further. I would just use something like DIST-INFO/ which
> will completely side step this issue. The only reason I can think of to use
> the current scheme is to make it easier to shutil.copytree it into the
> wheel, but handling that case is trivial.
The rationale for this is to leave the door open to in the future
allowing the same sdist to build multiple wheels. Obviously that would
require a whole 'nother PEP, but I keep running into cases where this
is a blocker so I think it will happen eventually, and in any case
don't want to make new barriers...
For get_wheel_metadata() in particular there are several options
though... we could call it DIST-INFO/ and then later declare that
DIST-INFO2/, DIST-INFO3/, etc. are also valid and pip will look at all
of them.
{package-name}.dist-info might also be reasonable, both here and in
actual installs...
In general get_wheel_metadata is an optimization for the backtracking
resolver (that doesn't exist yet) to more efficiently handle cases
where there are complex constraints and no built wheels (both of which
we hope will be rare). Robert thinks it's very important, and he knows
more about that bit of the code than I do, but if it becomes an issue
we could even defer get_wheel_metadata to a future PEP.
> 2. As I mentioned in previous discussions on this, I think that this
> interface *needs* some mechanism to ask it to build a sdist. Ideally this
> would either fail or recreate the sdist when being run from something that
> is already an sdist. In pip when we’re doing ``pip install .`` we currently
> copy tree the entire thing into a temporary directory, and we run the build
> out of there. For a variety of reasons we’re going to keep build isolation,
> but the current mechanism is painful because it also grabs things like .tox,
> .git, etc. At one point we tried to simply filter those out but it broke
> some packages expectations. The planned (which is on my long list of things
> to do…) mechanism to fix this is to create a sdist from a ``.`` and then
> install from that (building an intermediate wheel as well). This also solves
> another class of bugs that people run into where ``pip install .`` and
> ``python setup.py sdist && pip install dist/*`` give different results. As
> written, this PEP prevents that from happening (and thus, when I implement
> it, I’ll only be able to implement it for old style sdists, and will need to
> tell people to continue to use old style if they don’t want pip to grab
> random directories from ``.``).
>
>
> Other than that, it looks fine, and #2 is the one that I think is going to
> be the bigger issue in pip.
I think there's some pip bug somewhere discussing this, where Ralf
Gommers and I point out that this is a complete showstopper for
projects with complex and expensive builds (like scipy). If 'pip
install .' is going to replace 'setup.py install', then it needs to
support incremental builds, and the way
setup.py-and-almost-every-other-build-tool do this currently is by
reusing the working directory across builds.
I think there's some hope for making both of us happy, though, because
our concern is mostly about the
install-this-potentially-dirty-working-directory case, and your
concern is mostly about the build-this-package-for-release case
(right)?
Right now pip doesn't really have a good way to expressing the latter.
'pip install directory/' is relatively unambiguously saying that I
want a local install of some potentially-locally-modified files, and
while it might involve a temporary wheel internally there's no need to
expose this in any way (and e.g. it certainly shouldn't be cached), so
I think it's OK if this builds in-place and risks giving different
results than 'pip install sdist.tar.gz'. (Also note that in the most
common case where a naive user might use this accidentally, where
they've downloaded an sdist, unpacked it manually, and then run 'pip
install .', they *do* give the same results -- the potential for
trouble only comes when someone runs 'pip install .' multiple times in
the same directory.)
The other option would be to say that 'pip install .' is *not* the
preferred way to build-and-install from a working directory, and
there's some other command you have to use instead (maybe
build-system-specific) if you want efficient builds. This seems
unreasonably unfriendly to me, though; 'pip install .' is really the
obvious thing, so it should also be the one you want for common usage.
OTOH the closest thing to a do-a-release-build command currently is
'pip wheel', but it actually has the wrong semantics for making
release builds, because it downloads and builds wheels for the entire
dependency chain. I don't care if 'pip wheel directory/' copies the
directory or makes an sdist or what. If you want to build from an
sdist here it's fine with me :-). But I'm also uncertain of the value,
because 'pip wheel directory/' is a somewhat weird command to be
running in the first place, and it's rare and heavy-weight enough that
the shutil.copy() thing seems as good as anything.
And what seems like it's really missing is a command to like...
generate an sdist, build it into a wheel, and then leave them both
somewhere twine can see them. Maybe 'pip release .'? This would
definitely require a build-an-sdist hook in the build backend, and to
me it seems like the Right way to address your concern. In PEP 517 as
currently written this is punted to build-backend-specific tooling,
and I agree that that's unpleasant. But given that 'pip release'
doesn't even exist yet, maybe it makes more sense to defer the
build-an-sdist hook to a new PEP that we can write alongside 'pip
release'?
It might also make sense to have a 'pip ci .' that makes a release
build and installs it for testing...
-n
--
Nathaniel J. Smith -- https://vorpus.org
_______________________________________________
Distutils-SIG maillist - Distutils-SIG@python.org
https://mail.python.org/mailman/listinfo/distutils-sig
