Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly declare our build backend #1674

Closed
wants to merge 1 commit into from

Conversation

pganssle
Copy link
Member

@pganssle pganssle commented Feb 5, 2019

Summary of changes

This change more accurately reflects that setuptools is intended to use the build_meta backend from a previous installation. This may cause some problems with source builds if existing wheels for wheel and setuptools are not available. In that event, manually building a source wheel is recommended.

This may break pip install --no-binary :all:, though it's not entirely clear that it will. I would take the position that that this is basically revealing a bug in pip, but we could also just call this a "breaking change".

Closes #1644

Pull Request Checklist

  • News fragment added in changelog.d. See documentation for details

This change more accurately reflects that ``setuptools`` is intended to
use the ``build_meta`` backend from a previous installation. This may
cause some problems with source builds if existing wheels for ``wheel``
and ``setuptools`` are not available. In that event, manually building a
source wheel is recommended.
@pganssle
Copy link
Member Author

pganssle commented Feb 5, 2019

@pfmoore @pradyunsg Does pip actually need to do anything for this case, or have we worked out that this won't be a problem?

@pradyunsg
Copy link
Member

I expect this to break --no-binary :all: when setuptools has to be installed.

pip needs to be updated for improving that handling and I expect we won't have that until the Discourse discussion reaches some consensus.

None the less, until we figure out self-bootstrapping, this should be fine. :)

@pganssle
Copy link
Member Author

pganssle commented Feb 6, 2019

@pradyunsg So you are saying you're fine with --no-binary :all: breaking?

@pfmoore
Copy link
Member

pfmoore commented Feb 6, 2019

I think we need to resolve the "backend bootstrapping" discussion on Discourse before this can go in. Once we have a solution there (which may involve changes to pip and/or modifications to what you're doing here) then this can go in.

For now, my preference would be to put this on hold.

@pganssle
Copy link
Member Author

pganssle commented Feb 6, 2019

@pfmoore I think you can see my reasoning on #1644, but I am fairly certain that that discourse thread is going nowhere, certainly nowhere useful. From where I'm standing, the spec should allow for this. If a frontend (pip) cannot in some situations provide the setuptools build dependencies, that seems like a problem with that frontend or the environment, not with our configuration.

If PEP 517 is ever updated to forbid this, we can switch to the decidedly non-optimal (for everyone involved) option 3 - vendoring wheels of our dependencies in the sdist, or option 2 - opting out of PEP 517 forever.

@pganssle
Copy link
Member Author

pganssle commented Feb 6, 2019

The test failures seem to actually be due to pypa/pip#6197, so I think we can pin pip!=19.0.0,!=19.0.1 in the tests.

@pfmoore
Copy link
Member

pfmoore commented Feb 6, 2019

I'm not willing to get stuck in a confrontation over this, and in particular I'm not going to take a view on "whose problem is it?" while we're trying to establish agreement on what the responsibilities are over on Discourse (I don't share your disillusionment with that thread, although it'll be hard to make progress if setuptools isn't engaged with the process :-()

All I'm going to say here is that a cycle in build requirements is logically impossible to resolve if there's no binaries available. So whatever pip does, this change forces a bootstrapping step on anyone starting from a "sources only" environment. I'm not going to judge whether such environments constitute valid use cases, but conversely, if people raise bug reports on pip in that situation, I'll probably point the reporters at setuptools for advice. If people are using --no-binary :all:, that's a bit different, as wheels potentially are available and they have chosen to disallow them. In that case I'd say that setuptools build configuration doesn't work with --no-binary :all: and we're working on a resolution, but we don't yet know whether that resolution will need changes in pip or in setuptools.

That's a complex message to get across, and I'd prefer to avoid the need by postponing this change. But it's not my place to dictate what setuptools does, so if you want to go ahead we can work with the consequences.

@pganssle
Copy link
Member Author

pganssle commented Feb 6, 2019

All I'm going to say here is that a cycle in build requirements is logically impossible to resolve if there's no binaries available. So whatever pip does, this change forces a bootstrapping step on anyone starting from a "sources only" environment.

This is certainly true, though it's also true for gcc and clang and they seem to get along just fine. The kind of people who enforce "sources only" environments are also, generally, the kind of people who know how to deal with it. I think we can put it in a FAQ somewhere, maybe in packaging.python.org is the most appropriate place.

We can probably put this off for some time, but at some point we'll have to call the discourse thread a waste of time and give up on it. Maybe that's after the discussion peters out or maybe it's because enough people get tired of the discussion that the only people left are the ones who can agree on a path forward.

That said, if PEP 517 changes to force bootstrap builds, I imagine the "always build from source" crowd might like that there's a version of setuptools that supports PEP 517 and declares a build requirement rather than vendoring a wheel in the sdist. It's a lot easier to ban wheels at the infrastructure level than it is to patch the sdist with wheels you bootstrap by some other, totally unsupported mechanism. If we postpone the change, there will only be versions of setuptools that don't support PEP 517 and versions of setuptools that vendor wheels.

@dstufft
Copy link
Member

dstufft commented Feb 6, 2019

I would point out that my proposal that I posted yesterday explicitly allowed setuptools to basically land this exact pull request. It highly suggests not landing a PR like this, by saying that backends SHOULD avoid introducing a cyclic build dependency graph but it doesn't explicitly disallow it, since it's not a MUST.

I would also note that one of the questions I raised in my proposal was whether we should make it a SHOULD or whether we should make it a MUST. Obviously if we made it a MUST then a PR like this would make setuptools non-compliant with PEP 517 (but obviously the PEP has no teeth to force compliance), in either case, pip will have to detect and reject cyclic build dependencies when attempting to build.

In all cases it would mean that pip install --no-binary :all: ... would break, and if those people landed on pip's tracker we'd redirect them to setuptools tracker. Although if someone was generally OK with installing everything but setuptools from source, they could . do ``pip install --no-binary :all: --only-binary setuptools ...` instead.

@pfmoore
Copy link
Member

pfmoore commented Feb 6, 2019

pip install --no-binary :all: --only-binary setuptools ...

Hmm, I didn't know that --no-binary and --only-binary interacted that way (i.e., correctly ;-)) That's good to know and means that there's a much easier workaround for people who want to do this than I was assuming.

@pganssle
Copy link
Member Author

pganssle commented Feb 6, 2019

In all cases it would mean that pip install --no-binary :all: ... would break, and if those people landed on pip's tracker we'd redirect them to setuptools tracker.

I'm not entirely sure why this would be necessary given that this was a deliberate choice and not a bug. I imagine that would just be an exercise in frustration for the user. Presumably if this becomes a problem we can write some documentation to indicate what to do and send users to that, though obviously my preferred solution is something that just works for the users in the first place.

@dstufft
Copy link
Member

dstufft commented Feb 6, 2019 via email

@pfmoore
Copy link
Member

pfmoore commented Feb 6, 2019

my preferred solution is something that just works for the users in the first place.

But if the users say "I want to build everything from source (--no-binary :all:)" and setuptools says "I need setuptools and wheel installed in order to install myself", then there is no "just works". All we can do is say "you need to have setuptools available as a wheel because it says in its build instructions that it can't build from source without one" and the way to do that is --no-binary :all: --only-binary setuptools and provide a wheel (or let pip get it from PyPI). There's no magic here, you get what you say you need, and the user has to provide it.

@pradyunsg
Copy link
Member

Hmm... That's what I meant but seeing @dstufft and @pfmoore's comments, I think I'll take that back.

I'm on board for deferring this until the Discourse discussion settles down.

@pganssle
Copy link
Member Author

pganssle commented Feb 6, 2019

I ducked out of the discourse thread for a reason, so I'm not really interested in moving that whole discussion onto this random PR. I'm sorry to bring up that I would prefer if we chose a different resolution to the bootstrapping problem.

When we have a satisfactory resolution to #1659, I think we can fix the tests for this PR, then we can decide whether or not to land it. If we land it, I suspect that the most useful thing to do would be to put a note in one or more of: packaging.python.org, setuptools documentation and pip. When people inevitably complain, the best thing to do would be to point to that documentation.

@pfmoore
Copy link
Member

pfmoore commented Feb 6, 2019

@pganssle Is there a misunderstanding here? requires = ["setuptools", "wheel"] is saying that "in order to build setuptools from source, a frontend needs to create a build environment with setuptools and wheel installed in it. If a user says "don't download wheels from PyPI" (however they spell that), then the only way the frontend can get setuptools installed in that build environment is by building it from source. And that's an infinite regression right there.

I think you're hoping that frontends will be required to use wheels to satisfy build requirements. But we can't - it's not even about --no-binary, if the user is running the pip in an environment with no access to PyPI and only has sdists locally, they can't get a wheel. So we have to explain to them what's going on, and "setuptools needs setuptools to build itself, so you'll have to seed that process outside of pip" is all we can say.

@pfmoore
Copy link
Member

pfmoore commented Feb 6, 2019

@pradyunsg To be fair, I'm OK with --no-binary :all: breaking (and I suspect @dstufft is too) but we're saying that when it does we'd probably be saying "... and that's caused by a bug in setuptools", so the fact that we're OK with it breaking probably isn't actually as helpful as @pganssle hopes...

But if he wants to release this change and deal with the results, then that's fine. And I'm also not being passive aggressive by saying this, it's just the reality of how things are right now, that requires line will have that result. Once the Discourse thread produces a result, maybe there'll be another alternative.

@pganssle
Copy link
Member Author

pganssle commented Feb 6, 2019

As to the question of deferring it, I suppose having a release that explicitly declares its backend dependencies vs. including them as wheels in the sdist is not terribly important because without a corresponding version of pip that handles it smoothly it buys you nothing: when there's a version that has wheels in the sdist, it will just "work smoothly" and you won't know you're violating your "everything from source" policy, and if you do use the cyclic dependency version it will break with pip install --no-binary :all:. In any case, once you know you have to manually build a setuptools bootstrap wheel, you can patch setuptools.

So I suppose there's no advantage in actually rushing this through if pip is not going to have a corresponding fix in place. We can wait to see if the problem solves itself before landing this.

@pganssle
Copy link
Member Author

pganssle commented Feb 6, 2019

when it does we'd probably be saying "... and that's caused by a bug in setuptools"

This is the problem with the framing: I think it's uncharitable to consider this a "bug" in setuptools. It would be a deliberate choice to make explicit that setuptools needs a way to bootstrap itself. We could just as easily say it's a "bug" in pip that there's no way to express "build everything from sdists if possible and fall back to wheels to break cyclic build-time dependencies".

More charitably, we could say that bootstrapping entirely from sdists is out of scope for both projects, and if you have this unusual requirement, you need to built a bootstrap wheel.

@dstufft
Copy link
Member

dstufft commented Feb 6, 2019 via email

@pfmoore
Copy link
Member

pfmoore commented Feb 6, 2019

Yeah, I wasn't clear. When we have a revision to the PEP, then how we frame the response would depend on the wording on that (and I wouldn't expect it to be a "bug" in anything).

Right now, though, I think it's fair to say that this PR does not do what you want it to in conjunction with the existing behaviour in pip. That's why I advise not releasing it yet, and why I'd say to anyone affected that "a change in setuptools triggered this issue" (which is a better way of wording it than "there's a bug in setuptools").

@pganssle
Copy link
Member Author

pganssle commented Feb 6, 2019

Right now, though, I think it's fair to say that this PR does not do what you want it to in conjunction with the existing behaviour in pip. That's why I advise not releasing it yet, and why I'd say to anyone affected that "a change in setuptools triggered this issue" (which is a better way of wording it than "there's a bug in setuptools").

To be clear, this does improve the situation, because right now we're not shipping our pyproject.toml in the sdist at all, despite the fact that it should be part of the source. The reason we can't ship pyproject.toml is because we need to opt out of PEP 517, since we provide our own backend. If we started shipping our pyproject.toml without this change, then the following would break:

pip download -d dist --no-binary :all: setuptools
pip install dist/setuptools*.zip

So, basically, if you've gathered a source distribution (which you can do by using --no-binary :all: as part of the download requirements), you can install it or build a wheel from it safely with pip wheel, satisfying the build-time dependencies with wheels. Without this change, you'd get an error about a missing backend.

@pganssle pganssle closed this Feb 7, 2019
@pganssle pganssle deleted the add_backend branch February 7, 2019 14:37
@pganssle pganssle restored the add_backend branch February 7, 2019 14:37
@pganssle pganssle reopened this Feb 9, 2019
@pganssle
Copy link
Member Author

pganssle commented Mar 6, 2019

Closing this as not consistent with the newest version of PEP 517.

@pganssle pganssle closed this Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants