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

Build binary distributions via sdists by default #10746

Closed
1 task done
mcarans opened this issue Dec 24, 2021 · 12 comments
Closed
1 task done

Build binary distributions via sdists by default #10746

mcarans opened this issue Dec 24, 2021 · 12 comments
Labels
type: feature request Request for a new feature

Comments

@mcarans
Copy link

mcarans commented Dec 24, 2021

What's the problem this feature will solve?

The idea is to build wheels via sdists by default. This seems like a good way to ensure sdists are valid amongst other things. I've seen this discussed in other issues eg. this one. @pfmoore said in the last comment:

"I consider the change to always build via sdist to be an independent piece of work - we could have done it ages ago long before PEP 517/518, and it always got caught up in debates about incremental builds, and tools like flit that (at the time) didn't support sdists, etc. I still think this is worth doing, and I don't think the arguments against it are particularly compelling, but I didn't want to block implementing PEP 517 in order to debate that.

So if someone wants to finally switch to build-via-sdist, then I'm all for it, but it would also need a separate issue."

Since I could not find a separate issue, I created this one.

Describe the solution you'd like

Build binary distribution via sdist by default similar to what python -m build does: pypa/build#304

Alternative Solutions

I use tox-wheel and it currently uses pip so I don't have another solution

Additional context

None

Code of Conduct

@mcarans mcarans added S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature labels Dec 24, 2021
@pradyunsg
Copy link
Member

Use pip wheel pkg.tar.gz?

@mcarans
Copy link
Author

mcarans commented Dec 24, 2021

I wasn't clear in the description. I meant to build the wheel from the sdist by default like python -m build.

@mcarans mcarans changed the title Build wheel from sdist Build wheel from sdist by default Dec 24, 2021
@pradyunsg
Copy link
Member

Ah. That's been extensively discussed as part of implementing PEP 517 in pip -- the conclusion was that its the responsibility of the build backend to ensure that tree -> sdist -> wheel is the same as tree -> wheel.

I would've linked to specific issues/comments, but I'm on mobile and on a vacation. :)

@mcarans
Copy link
Author

mcarans commented Dec 26, 2021

@pradyunsg I think the discussion you mention is what I linked to: #5407 (comment)

@pfmoore said in the last comment:
"I consider the change to always build via sdist to be an independent piece of work - we could have done it ages ago long before PEP 517/518, and it always got caught up in debates about incremental builds, and tools like flit that (at the time) didn't support sdists, etc. I still think this is worth doing, and I don't think the arguments against it are particularly compelling, but I didn't want to block implementing PEP 517 in order to debate that.

So if someone wants to finally switch to build-via-sdist, then I'm all for it, but it would also need a separate issue."

Since I could not find a separate issue on the topic, I created this one.

EDIT: I've copied the above into the description to make it clearer.

@mcarans mcarans changed the title Build wheel from sdist by default Build binary distributions via sdists by default Dec 29, 2021
@pradyunsg pradyunsg removed the S: needs triage Issues/PRs that need to be triaged label Mar 27, 2022
@sbidoul
Copy link
Member

sbidoul commented Mar 28, 2022

This topic was (re)discussed extensively as part of #7555 (a very long one) and we concluded we don't want to do that, mainly due to the performance implications.

While I see the value of build-via-sdist, and it is certainly a good default for the build project, I think it should not be the default for pip.

@mcarans
Copy link
Author

mcarans commented Mar 28, 2022

Presumably build chose the default of going via sdist because it doesn't regularly run into performance issues and these only occur in a minority of cases? If that's the case, then it makes sense to me to have the default for pip be to build via sdist and the option available if there are performance issues to build directly.

@uranusjr
Copy link
Member

uranusjr commented Mar 31, 2022

Presumably build chose the default of going via sdist because it doesn't regularly run into performance issues

We need input from build maintainers for a definitive answer, but from my loose following of the progress build made this change, performance is considered a legistimate issue, but they went for the approach anyway because mis-packaging is a bigger issue for build and they weight correctness way higher than performance penalty (packages built by build are generally published to PyPI and become immutable).

Also, the main drive behind the idea was actually to improve correctness on the sdist, not the wheel, because it is a common mistake for maintainers to mis-package an sdist, but fail to notice it until much later because most users only install wheels. This is however a non-issue for pip. When pip has a source tree, whether that source tree can buid an sdist correctly is entirely irralevant, as long as the wheel builds correctly, so forcing all wheels to build from sdist is mostly a hinderance without obvious benefits for a user.


Supplement: I’m not saying pip should not build wheel from sdist at all, but that pip should not do that by default, since that would be a bad default for most people. But I wouldn’t object at all if you add a pip install option that makes pip build wheel from sdist, since this makes sense for certain scenarios.

@pradyunsg
Copy link
Member

I'm wondering if we would benefit from having pip's CLI differentiate between "package author" and a "package user" workflows.

Build-via-sdist makes sense for the package authors, but not for package consumers. That's the real separation we want to have, and we might benefit from adding such separation in other areas? I'm not sure, which is why it's a question mark.

@mcarans
Copy link
Author

mcarans commented Mar 31, 2022

it is a common mistake for maintainers to mis-package an sdist, but fail to notice it until much later because most users only install wheels. This is however a non-issue for pip. When pip has a source tree, whether that source tree can buid an sdist correctly is entirely irralevant

I think that it benefits maintainers/authors to be able to build correct sdists and it benefits any user/consumer who is on a platform where there is no binary wheel available and the sdist gets installed instead (as happens sometimes on Docker Alpine builds for example) as they get a correct instead of a possibly broken sdist.

Having said all this, I don't know how bad are the performance issues that have been raised. What sort of difference in performance are we talking about? Does the performance issue generally occur with pure Python wheels or only with those with non-Python extensions?

@rgommers
Copy link

What sort of difference in performance are we talking about?

One data point for SciPy:

$ time python -m build --sdist
...
real    1m7.652s
user    0m56.962s
sys     0m7.688s

Compared to the build itself taking ~2 minutes. About half the sdist creation time is setting up an isolated venv (with all dependencies already in cache); zipping up some files takes over 30 seconds with setuptools as well though. So that looks like a (too) large performance impact to me.

Build-via-sdist makes sense for the package authors, but not for package consumers.

This I agree with. It also doesn't make sense for the average contributor to a package, only for a maintainer / release manager. Which is a very small percentage of all activity in a large project, so this would be a bad default imho.

I'm wondering if we would benefit from having pip's CLI differentiate between "package author" and a "package user" workflows.

Interesting, but .... if it's a new command, wouldn't that be about as hard to teach as "please use build instead for this release-type task"?

@RonnyPfannschmidt
Copy link
Contributor

i think its fair to say that this particular one is out of scope for pip, after all pip is a installer, not a builder, and python -m build already exists plus tools like cibuildwheel

@pradyunsg
Copy link
Member

pradyunsg commented Sep 16, 2022

OK, I'm gonna go ahead and say that we'd only do this if we implemented a pip build command -- #6041 -- and only within that command. I'll consolidate this issue into that one. :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

6 participants