cibuildwheel icon indicating copy to clipboard operation
cibuildwheel copied to clipboard

Build sdist?

Open YannickJadoul opened this issue 4 years ago • 27 comments

While advertising cibuildwheel to the PyPy team, I got the following suggestion:

<arigato> if you want my opinion, the azure-pipelines is making an "artifact" that contains all the wheels, which is almost perfect---if it also contained the source package (sdist) it would be perfect :-) <YannickJadoul> Hmm, that's a good point. I just have a line python setup.py sdist before <arigato> yes, it's more of a minor plus. It's not a problem to run "python setup.py sdist upload" locally in parallel with "twine upload *.whl" <arigato> ...as long as I remember to go in a clean dir

I'm not entirely sure how it would fit into cibuildwheel (on which platform we would build them, etc), but indeed, if cibuildwheel's goal is to hand over a bunch of wheels without worries, the source distribution could be part of that?

YannickJadoul avatar Oct 16 '19 16:10 YannickJadoul

I got bitten by this; I assumed cibuildwheel would take care of it. Although as you say adding a python setup.py sdist before works fine.

MaxHalford avatar Oct 23 '19 13:10 MaxHalford

It's kinda funky that a tool called 'Build Wheel' should build sdists too, but I definitely see how useful this could be.

I'm thinking this should be a command line argument cibuildwheel --also-sdist, which can be added to one of the platforms' script in CI.

Maybe controlled via an env var too CIBW_ALSO_SDIST. Crazy thought - should it be on by default?

In terms of implementation, I think python setup.py sdist --dist-dir {output_dir} is still best practise, since the fabled pip build hasn't appeared yet.

joerick avatar Oct 23 '19 21:10 joerick

Adding sdist cannot be default. To build wheels on three systems I need use 3 separated machines. No one need 3 copies of sdist result. In my case I manually add this step in yaml file and I do not know why this solution is not optimal?

Czaki avatar Oct 24 '19 07:10 Czaki

It's kinda funky that a tool called 'Build Wheel' should build sdists too, but I definitely see how useful this could be.

Adding sdist cannot be default. To build wheels on three systems I need use 3 separated machines. No one need 3 copies of sdist result.

Those are indeed the two main reasons I made an issue to discuss, rather than a quick PR :-)

A compromise would be to not add this to cibuildwheel (or maybe revisit the idea once we get pip build), but clearly state this at the relevant locations in the README/docs? If we just add a line python setup.py sdist --dist-dir {output_dir} before cibuildwheel to the minimal examples, it should be clear that cibuildwheel isn't doing that?

Another thing to be aware of (if we implement this): you need a clean directory to make source distributions or there's a chance a bunch of generated/compiled/... files get dragged in, so it's probably the first thing we do, before starting to build binary wheels.

YannickJadoul avatar Oct 24 '19 12:10 YannickJadoul

If we just add a line python setup.py sdist --dist-dir {output_dir} before cibuildwheel to the minimal examples, it should be clear that cibuildwheel isn't doing that?

This is a sensible suggestion. I'd add that maybe it should be commented out in the example configs and possibly change output_dir from wheelhouse to dist in the config since it's not all wheels.

joerick avatar Oct 25 '19 09:10 joerick

Good point. If I find the time, I'll have a look at a quick PR.

YannickJadoul avatar Oct 25 '19 13:10 YannickJadoul

FYI in the Vispy project I need to do this and handled it outside of cibuildwheel but in the same azure pipeline that cibuildwheel is then executed. You can see it here: https://github.com/vispy/vispy/blob/master/azure-pipelines.yml#L27

I believe @larsoner (another vispy dev) has adopted this approach in some of his other projects as well.

djhoese avatar Nov 05 '19 01:11 djhoese

@djhoese Is this something you expect to be done by cibuildwheel, then, or does it make sense to you that cibuildwheel only builds wheels and no source distribution?

YannickJadoul avatar Nov 05 '19 14:11 YannickJadoul

That's a tough question to answer. When I needed it I was surprised to find that cibuildwheel had no way of allowing me to build the sdist and that it wasn't documented. BUT I didn't make an issue about it because it also makes sense that cibuildwheel doesn't do this for you; it only builds wheels.

Since an sdist only has to be built once I could see one of a couple options:

  1. cibuildwheel doesn't do anything and just adds a note to the README that sdist is not part of this project.
  2. Add building of sdist to the examples as something that gets done before cibuildwheel is executed. I'm not sure how this can be done to make it clear that this is optional though. The solution may also differ between CI service.
  3. Add an option to cibuildwheel to run python setup.py sdist before building the wheels inside the container. You only need one sdist though so this may not be the easy 2-4 line solution that I hope it to be on every platform.

djhoese avatar Nov 05 '19 17:11 djhoese

Just wanted to offer a reason for adding support for sdist: I've started using cibuildwheel on Azure Pipelines, and generating all the builds as part of the same process makes the build more reliable. If I run the sdist on my own machine there's room for error – I have to make sure I'm using the same rev that got built in Azure. Plus, surfacing the sdist from the CI build artifact makes it easy for others to verify that the generated sdist is identical the one uploaded to PyPI.

paulmelnikow avatar Nov 05 '19 18:11 paulmelnikow

@djhoese @paulmelnikow Thanks for that input! That's exactly the kind of answers and use cases I was hoping for when asking :)

@paulmelnikow A single line with python setup.py sdist before calling cibuildwheel in your CI would have the same impact, though?

Adding something to the README/docs was already on my TODO list, and I would agree that it's the least we can do. Adding a (commented?) line in the example configurations would also make sense to me.

But now I'm thinking some option like CIBW_MAKE_SDIST (INCLUDE_SDIST, CREATE_SDIST, ...?) maybe isn't as crazy as it sounds. The one thing that might be annoying is that setup.py sdist has a few extra options: https://docs.python.org/3/distutils/sourcedist.html, so we'd need to start thinking about those and provide an extra CIBW_ option to customize this? But at that point, writing export CIBW_MAKE_SDIST="1", export CIBW_SDIST_ARGS="--formats=gztar,zip" is not a lot simpler than python setup.py sdist --formats=gztar,zip"?

YannickJadoul avatar Nov 06 '19 16:11 YannickJadoul

But at that point, writing export CIBW_MAKE_SDIST="1", export CIBW_SDIST_ARGS="--formats=gztar,zip" is not a lot simpler than python setup.py sdist --formats=gztar,zip"?

True -- but if you're willing to make the default CIBW_MAKE_SDIST=1 then it is simpler because sdist will "just work" for most people (i.e, as long as they don't need special options). FWIW I maintain a few packages and have never used special sdist args.

Not sure if that's on the table, though.

larsoner avatar Nov 06 '19 16:11 larsoner

@djhoese @paulmelnikow If you build on Azure or other CI provider you need to use at least 3 machines. So if CI build sdist you will have 3 copies of sdist.

If you would like to use matrix in specification of CI then still you can use bash if for this like (Azure Pipelines):

- bash: | 
    if [ "$(expr substr $(uname -s) 1 5)" == "Linux" ]; then
     python -m pip install -r requirements.txt; 
     python setup.py sdist -d wheelhouse; 
    fi
  displayName: sdist

here whole example configuration

@YannickJadoul But this option need to specify on which machine sdist is created.

Czaki avatar Nov 06 '19 16:11 Czaki

Not sure if that's on the table, though.

It is, I think; that's what I was thinking out loud about. It shouldn't be too hard to implement, but it will always take a bit of thinking on the user's side: you need to run 3 different CI configs with cibuildwheel, so a) if we make it default, you'll suddenly get 3 different (similar/same/...?) source builds (and maybe turn it off on 2 platforms?) and b) if we don't make it default, the user needs to turn it on (CIBW_MAKE_SDIST=1 on 1 platform instead of writing python setup.py sdist).

@Czaki In my experience, the 3 different platforms need slightly different installations and almost always have separate blocks in the config anyway. So setting CIBW_MAKE_SDIST will only affect 1 run, and I don't think the platform needs to be specified.

YannickJadoul avatar Nov 06 '19 16:11 YannickJadoul

@Czaki Yes, I already do this with vispy's azure pipeline that I linked to above: https://github.com/vispy/vispy/blob/master/azure-pipelines.yml#L27. We keep our different platforms separate so it is just an extra command and making sure the sdist is included in the artifacts. Our config also has a separate step for publishing to PyPI using Azure secrets for the token.

Do the options have to be environment variables? Could it be a command line option on the cibuildwheel command line? Not sure that makes it any easier. As for duplicate sdist tarballs and uploading to PyPI, there is an option for ignoring existing sdist tarballs of the same version (to avoid exiting with an error). At least I know travis has that.

djhoese avatar Nov 06 '19 16:11 djhoese

Alternatively, we do have something like CIBW_MAKE_SDIST that takes a comma-separated list of platforms and just defaults to linux?

EDIT: Does anyone nót agree with expecting Linux would be the default platform to be used to make sdists?

YannickJadoul avatar Nov 06 '19 16:11 YannickJadoul

@YannickJadoul so i still do not know what would be the profit of CIBW_MAKE_SDIST instead of python setup.py sdist -d wheelhouse

Using option -d allow to put sdist in same directory as wheel so not need to create separated artifacts etc.

Czaki avatar Nov 06 '19 17:11 Czaki

so i still do not know what would be the profit of CIBW_MAKE_SDIST instead of python setup.py sdist -d wheelhouse

That's what I'm also arguing. But at the same time, it's better integrated with cibuildwheel.

Alternatively, we do have something like CIBW_MAKE_SDIST that takes a comma-separated list of platforms and just defaults to linux?

If we want to support this, this ís simpler, though, since the default would be good in 90% of cases.

YannickJadoul avatar Nov 06 '19 17:11 YannickJadoul

@paulmelnikow A single line with python setup.py sdist before calling cibuildwheel in your CI would have the same impact, though?

Yes; though the less boilerplate I have to understand, maintain, and replicate as a library developer, the better. I'd also have to repeat the wheel output location. Including a --with-sdist flag is meaningfully less boilerplate than python setup.py sdist -d python/wheelhouse.

Do the options have to be environment variables? Could it be a command line option on the cibuildwheel command line? Not sure that makes it any easier.

The command line seems a little nicer, though having two ways to do it seems nice, since what's most convenient may depend on how it's being used.

Also apologies if this has been covered already. Has there been any consideration of putting config in a config file like setup.cfg or pyproject.toml? Config file snippets are easier to replicate across projects, and would also have the advantage of being portable to the different environments where the tool runs. (Env vars and command lines are portable, but the config format is not.)

EDIT: Does anyone nót agree with expecting Linux would be the default platform to be used to make sdists?

It's a good default, however that's assuming people are building for Linux at all. It seems possible a user might only care about Mac, or only care about Windows. You could also imagine a library that interfaces with some Windows software, and can only run on Windows to begin with. In that case there wouldn't be a need for a Linux build.

paulmelnikow avatar Nov 06 '19 18:11 paulmelnikow

It's a good default, however that's assuming people are building for Linux at all. It seems possible a user might only care about Mac, or only care about Windows. You could also imagine a library that interfaces with some Windows software, and can only run on Windows to begin with. In that case there wouldn't be a need for a Linux build.

I'd say the opposite is more common in my experience (Linux support first, probably Mac support because Unix-like, and Windows support is least likely).

I agree that Linux seems like the best default.

djhoese avatar Nov 06 '19 18:11 djhoese

Do the options have to be environment variables? Could it be a command line option on the cibuildwheel command line?

Has there been any consideration of putting config in a config file like setup.cfg or pyproject.toml?

Currently, all these options are environment variables, so while a do think these are good points, that's probably a different discussion? I feel like this was mentioned in some discussion, but I can't find an issue. So feel free to open an issue.

The basic 2 question are independent, though:

  • Is it cibuildwheel's responsibility/job to create source distributions?
  • How do we set a sensible default that makes sure it actually is more intuitive than the extra python setup.py sdist -d wheelhouse line?

YannickJadoul avatar Nov 06 '19 18:11 YannickJadoul

Currently, all these options are environment variables, so while a do think these are good points, that's probably a different discussion? I feel like this was mentioned in some discussion, but I can't find an issue. So feel free to open an issue.

👍

paulmelnikow avatar Nov 06 '19 18:11 paulmelnikow

There maybe one profit of build sdist. If someone know how check package before publishing them to pypi (if all fields have proper values ex.).

Maybe anyone in this thread know how to do this without try of upload..

Czaki avatar Nov 08 '19 08:11 Czaki

There maybe one profit of build sdist. If someone know how check package before publishing them to pypi (if all fields have proper values ex.).

There's python setup.py check, but it doesn't do a lot. Something else to think about, though: it would/could be nice to actually test the installation (and compilation) of a source distribution?

YannickJadoul avatar Nov 09 '19 17:11 YannickJadoul

Checking if it is installable is good case, but for this best is tox. And still package which complete build, pass installation, pass test still can be rejected by pypi.

Czaki avatar Nov 09 '19 20:11 Czaki

I think it's build's responsibility to build SDists. pip install build; python -m build --sdist. ci-build-wheel should build wheels. Setting this up so that only one job builds wheels would be a pain, and counter productive. Should we close, or should we add SDist generation to one or more examples? Or maybe add a "tutorial" page, similar to https://scikit-hep.org/developer/gha_wheels (I wrote most or all of that page, so happy to reproduce here if you want to).

henryiii avatar Feb 01 '21 04:02 henryiii

As a lazy user, I like to have a one-stop option with batteries included. So when I build in the CI I want the output to be everything I need: native wheels, pure wheels and sdists. I understand the concern to make this project do too much, but then may be having another "cibuild" project that could wrap it all and build all the things expected from a build could make sense?

See also https://github.com/pypa/cibuildwheel/discussions/1007

pombredanne avatar Feb 11 '22 07:02 pombredanne

Going through some old issues. Closing as 'not planned'. Use build python -m build --sdist or pipx run build --sdist instead. See https://github.com/pypa/cibuildwheel/blob/main/examples/github-deploy.yml for how to do this as part of a cibuildwheel release workflow.

joerick avatar Oct 21 '22 14:10 joerick

Use build python -m build --sdist or pipx run build --sdist instead

Just my two cents: I already do it in my project. The problem is that this will simply build the sdist package, but it will not install it and run any test.

See the above linked PR, that fixed a trivial error in the MANIFEST that broke the sdist.

Marco-Sulla avatar Apr 09 '23 13:04 Marco-Sulla

I’d recommend https://github.com/henryiii/check-sdist. You can also build from and check your SDist. Cibuildwheel actually supports building from SDist these days.

henryiii avatar Apr 09 '23 16:04 henryiii