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 wheel from sdist (optionally?) #257

Closed
altendky opened this issue Mar 16, 2021 · 21 comments · Fixed by #304
Closed

Build wheel from sdist (optionally?) #257

altendky opened this issue Mar 16, 2021 · 21 comments · Fixed by #304
Labels
enhancement New feature or request

Comments

@altendky
Copy link

The proposal is to build the wheel from an sdist rather than directly from the source. Perhaps optionally, perhaps only when both --sdist and --wheel are specified, perhaps always. Presently I have this implemented in towncrier as below.

# build the sdist
python -m build --sdist --outdir ${toxinidir}/dist/ ${toxinidir}
tar -xvf ${toxinidir}/dist/*
cd *
# build the wheel from the sdist
python -m build --wheel --outdir ${toxinidir}/dist/ .

It was suggested that perhaps this would be a useful feature for build to provide itself. See more context and discussion below.

twisted/towncrier#323 (comment)

Following Trio's lead, I implemented package builds for towncrier such that first an sdist is built and then a wheel is built from that sdist. Following that, those two files are distributed throughout the CI tests and checks and (upcoming) automated publishing. The latter bit makes sure that the specific wheel to be published is what is being tested. The former bit makes sure that the sdist is able to generate a functional wheel. I don't mean this to test that build and wheel and so on work correctly, but rather to test that the package being built is setup properly such that it will result in a functional sdist. Twisted #10103 was shared as an example issue that this workflow would have caught.

@layday
Copy link
Member

layday commented Mar 16, 2021

This is mentioned in PEP 517 as a possibility, under § build-sdist:

However, some frontends may prefer to make intermediate sdists when producing wheels, to ensure consistency.

My opinion is that this should be the default behaviour in build, if pip should be able to build wheels from published sdists. I'm lukewarm about allowing wheels to be built in the absence of an sdist as it's bound to catch someone off-guard - I think that --wheel should always imply --sdist.

@gaborbernat
Copy link
Contributor

gaborbernat commented Mar 16, 2021

pip actually did switch to build sdist and then wheel, but caused so many issues they reverted to copy + build a wheel. IMHO these things are different, so I'd expect a --wheel, --sdist and a --wheel-via-sdist 🤷🏻 to cover all scenarios/use cases.

@uranusjr
Copy link
Member

pip is so burdened with backwards compatibility people get unreasonably mad when something "breaks" for them. I would hope new tools like build default to doing the most deterministic thing possible so people can try to do things better by default. So I'd want something like build (which builds sdist and wheel from sdsit), build --only-sdist (does not build wheel), and build --wheel-from-source-tree.

@layday layday added the enhancement New feature or request label Mar 18, 2021
@FFY00
Copy link
Member

FFY00 commented Apr 21, 2021

I think the best way to proceed is to switch the default behavior to this -- build a sdist and then a wheel from that sdist. The --wheel and --sdist arguments would build "standalone" sdists/wheels, which is the current behavior.

@gaborbernat
Copy link
Contributor

Wait are you proposing that: python -m build and python -m build --wheel --sdist behave differently?

@FFY00
Copy link
Member

FFY00 commented Apr 21, 2021

Yes, --wheel would build a wheel from source.

I was envisioning something like this.

usage: python -m build [-h] [--version] [--sdist] [--wheel] [--outdir OUTDIR] [--skip-dependencies] [--no-isolation] [--config-setting CONFIG_SETTING] [srcdir]

This command will build a source distribution and then use it to build a wheel, unless the
--sdist or --wheel options are specified.

positional arguments:
  srcdir                source directory (defaults to current directory)

optional arguments:
  -h, --help            show this help message and exit
  --version, -V         show program's version number and exit
  --sdist, -s           build a source distribution
  --wheel, -w           build a wheel from the project source
  --outdir OUTDIR, -o OUTDIR
                        output directory (defaults to {srcdir}/dist)
  --skip-dependencies, -x
                        do not check that build dependencies are installed
  --no-isolation, -n    do not isolate the build in a virtual environment
  --config-setting CONFIG_SETTING, -C CONFIG_SETTING
                        pass options to the backend. options which begin with a hyphen must be in the form of `--config-setting=--opt(=value)` or `-C--opt(=value)`

I don't know, it may not be supper clear. We would have more in depth documentation in the website, explaining why this behavior is, we can even have the link there.

@gaborbernat
Copy link
Contributor

For me, that would be surprising behavior. I personally view the --sdist, --wheel, --wheel-from-source-tree variation the cleanest/simplest to use/understand approach.

@FFY00
Copy link
Member

FFY00 commented Apr 21, 2021

Why would it be surprising if it's documented? I think the description of --wheel would be probably better as "build a wheel from the specified source".

My reasoning behind this design is that the tool should have the recommended behavior by default, while still being accessible for advanced users (that is a big part of this tool). Instead of telling users to python -m build -w or something like that, we should recommend users to just do python -m build. Advanced users would be the ones using --sdist and --wheel, and they would get their expected behavior, which would be as close as simply calling the PEP 517 hooks as possible.
I understand that this behavior may surprising coming from the current releases, but that's why we are pre 1.0.0. We should curate the behavior for new users.

I suggest you take a look at the other side (advances users). --wheel would no longer work with --no-isolation, I mean, we could make it work, but it would be very surprising and most definitely unwanted behavior, these users would have to use --wheel-from-source-tree. Unless we change the behavior of --wheel based on --no-isolation, but I think that's just messy and confusing.

Having --wheel buid from a sdist would also mean that we are actually also building a sdist, which would be confusing to me. We could hide this from the user, but why? python -m build -w would be effectively doing the same job as python -m build -sw, so why have it at all?

So if we are implementing this, I think it should be the default behavior. I also think this option should be mutually exclusive from --sdist and --wheel. At which point, why would have an explicit option?

@altendky
Copy link
Author

Why would it be surprising if it's documented?

Documenting surprising behavior just moves the moment of surprise for users that read the docs in detail first.

Advanced users would be the ones using --sdist and --wheel, and they would get their expected behavior, which would be as close as simply calling the PEP 517 hooks as possible.

Having --wheel build the wheel in a not-preferred way seems weird. Sounds like an option such as --517-wheel-hook or whatever is what's being described in this suggestion. Better yet, other subcommands or such since building a wheel in multiple ways presumably ends up with a collision at the end and mutually exclusive command line options aren't the best thing ever.

python -m build -w would be effectively doing the same job as python -m build -sw, so why have it at all?

--sdist would control the user facing output of the program to be different than --sdist --wheel and --wheel. I would think that --sdist and --wheel would be most likely associated by the user with the output, not the implementation details of the 517 hooks.

I don't know my way around all the nuances but those particular comments didn't seem to form strong arguments.

@gaborbernat
Copy link
Contributor

gaborbernat commented Apr 22, 2021

Where did you read that the recommended behavior is to build a wheel from a sdist rather than from the source tree? 🤔 I don't think that's true. pip itself builds a wheel from the source tree and not the sdist.

I think what we want is --sdist, --wheel and --wheel-via-sdist 🤔

@layday
Copy link
Member

layday commented Apr 22, 2021

I don't know if it's recommended anywhere, but I'd consider it good practice that for published sdists the wheel is built from the sdist for consistency with the published wheel. With all of the nuances surrounding packaging data in setuptools in particular, it's not hard to imagine that a source-tree build might produce a very different output.

@uranusjr
Copy link
Member

Where did you read that the recommended behavior is to build a wheel from a sdist rather than from the source tree? 🤔 I don't think that's true. pip itself builds a wheel from the source tree and not the sdist.

See my comment above. pip wants to build wheel against sdist and is working toward the goal, it just can’t right now.

@altendky
Copy link
Author

@gaborbernat, yes, I should have used more words. It seems there is some feeling that building the wheel from the sdist is a good thing to do generally. This is not the same as an 'official consensus' (however that might be defined) that wheels ought only be built from sdists. Sorry I jumped ahead there.

But, if it is concluded that just -m build should build the wheel from the sdist then it seems like a decision from this project's perspective that that is in fact a preferred approach. I believe @FFY00 has suggested they are at least tending in this direction on this specific point. At that point I would expect the simple --wheel option to do the same thing. Having the default behavior correspond with the more arcane --wheel-via-sdist isn't obvious to me.

@gaborbernat, you have switched between --wheel-from-source-tree and --wheel-via-sdist so I'm not clear where you stand on this now. :]

@gaborbernat
Copy link
Contributor

Either one :-) no strong feelings. At the moment I'm more in favor of --wheel-via-sdist``. Either way, I'd not change --wheel` to build from sdist before we codify in a PEP that it's the official way of building wheels. Let's not jump ahead, because we might very well not agree on this.

@FFY00
Copy link
Member

FFY00 commented Apr 22, 2021

Like I said, I believe the --sdist and --wheel options should be pretty straightforward, without any weird handling like building a sdist first. We should have separate options for that.

Having --wheel build the wheel in a not-preferred way seems weird. Sounds like an option such as --517-wheel-hook or whatever is what's being described in this suggestion. Better yet, other subcommands or such since building a wheel in multiple ways presumably ends up with a collision at the end and mutually exclusive command line options aren't the best thing ever.

I have a pretty strong position on this. The project should abstract such low-level details, while still being as close to the low-level mechanism as possible. I don't want CLI breakage if Python decides to move to another backend mechanism.
Arch Linux currently has 1870 python packages, I am looking into moving them all to build as a package builder. If Python decides to move to another backend mechanism, I don't want to have to change the build instructions in all those packages to replace --517-wheel-hook with the new option.

In this light, I think these two options, --sdist and --wheel, should stay pretty much the same regardless of how the rest changes, as that's what packagers will be using, and I need to commit to long term compatibility here.

Either one :-) no strong feelings. At the moment I'm more in favor of --wheel-via-sdist. Either way, I'd not change ``--wheel` to build from sdist before we codify in a PEP that it's the official way of building wheels. Let's not jump ahead, because we might very well not agree on this.

I think we should figure all this out before introducing anything. I am gonna open a discourse thread.

@FFY00
Copy link
Member

FFY00 commented Apr 22, 2021

@EpicWink
Copy link
Contributor

EpicWink commented Apr 28, 2021

On a similar vein, is build being able to build from a user-provided sdist directly (rather than manually extracting, then building) in-scope? I'm happy to do that PR.

My use case is to build the sdist in one environment in CI (eg python:alpine Docker image), then build the wheels in each environment (python version vs OS matrix).

@webknjaz
Copy link
Member

webknjaz commented May 8, 2021

I want to document another clear use case. Various Linux distro packagers rely on sdists from PyPI specifically to produce things like RPMs so it'd make a lot of sense if it could be done as seamlessly as possible.

@FFY00
Copy link
Member

FFY00 commented May 8, 2021

As explained above, I think --sdist and --wheel should just invoke the low level mechanisms. And given the feedback on the discourse thread, I think we should make build wheels via sdists the default behavior.

So what I am leaning to is simply making this the default behavior, instead of implying --sdist --wheel. If there are no major issues with this, that is probably the call I am going to make.

@webknjaz
Copy link
Member

webknjaz commented May 8, 2021

@FFY00 do I understand correctly that it would be possible to pass the tarball path instead of the source dir path and the rest of the UI would preserve its look&feel?

@FFY00
Copy link
Member

FFY00 commented May 8, 2021

I was not thinking about that, but we can indeed do it, but it will only be valid for python -m build --wheel.

FFY00 added a commit to FFY00/python-build that referenced this issue Jun 7, 2021
Fixes pypa#257

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Jun 7, 2021
Fixes pypa#257

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Jun 15, 2021
Fixes pypa#257

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Jun 15, 2021
Fixes pypa#257

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Jun 15, 2021
Fixes pypa#257

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Jun 16, 2021
Fixes pypa#257

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit that referenced this issue Jun 16, 2021
Fixes #257

Signed-off-by: Filipe Laíns <lains@riseup.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants