-
Notifications
You must be signed in to change notification settings - Fork 247
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 sdist? #173
Comments
I got bitten by this; I assumed |
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 Maybe controlled via an env var too In terms of implementation, I think |
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? |
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 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. |
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 |
Good point. If I find the time, I'll have a look at a quick PR. |
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 Is this something you expect to be done by |
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:
|
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. |
@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 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 |
True -- but if you're willing to make the default Not sure if that's on the table, though. |
@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):
here whole example configuration @YannickJadoul But this option need to specify on which machine sdist is created. |
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 @Czaki In my experience, the 3 different platforms need slightly different installations and almost always have separate blocks in the config anyway. So setting |
@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 |
Alternatively, we do have something like EDIT: Does anyone nót agree with expecting Linux would be the default platform to be used to make sdists? |
@YannickJadoul so i still do not know what would be the profit of Using option |
That's what I'm also arguing. But at the same time, it's better integrated with
If we want to support this, this ís simpler, though, since the default would be good in 90% of cases. |
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
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.)
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. |
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:
|
👍 |
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.. |
There's |
Checking if it is installable is good case, but for this best is tox. |
I think it's |
As a lazy user, I like to have a one-stop option with batteries included. See also #1007 |
Going through some old issues. Closing as 'not planned'. Use |
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. |
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. |
Well, now I've done a deeper search and I found this: https://github.com/pypa/cibuildwheel/pull/1096/files but honestly I failed to understand how it works. |
You can give cibuildwheel an SDist instead of a directory as input. |
From command line, I suppose? How can you setup from a pipeline like Github Actions? |
It’s the main positional input parameter on the cli, or “package-dir:” for the GitHub action. The name was picked before it grew the ability to take an SDist. :) Just |
Good, it works! Unluckily, it seems a little more complicated: I was not able to pass *.tar.gz, since the path is read by Can I add a PR to the tips and tricks? |
While advertising cibuildwheel to the PyPy team, I got the following suggestion:
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?The text was updated successfully, but these errors were encountered: