Skip to content

Build wheels from sdist (separate entrypoint)#1072

Closed
joerick wants to merge 6 commits intomainfrom
from-sdist
Closed

Build wheels from sdist (separate entrypoint)#1072
joerick wants to merge 6 commits intomainfrom
from-sdist

Conversation

@joerick
Copy link
Copy Markdown
Contributor

@joerick joerick commented Apr 3, 2022

Alternative to #1020. Fixes #597.

This provides a separate entrypoint for building wheels from sdist, rather than from a source tree.

I'm using a different entrypoint because the way that command line arguments are parsed are different. This is because cibuildwheel is run with working dir as the root of the sdist, not the current working dir. That is because the docker 'project' is the contents of the sdist, and paths in options (e.g. options within pyproject.toml) should be relative to the sdist root.

The command line options --output-dir and --config-file are the exceptions to this - they are options of cibuildwheel-from-sdist and thus resolved relative to the working dir.

Docs are still to come, but help output is:

$ cibuildwheel-from-sdist --help
usage: cibuildwheel-from-sdist [-h] [--output-dir OUTPUT_DIR]
                               [--config-file CONFIG_FILE]
                               package

Build wheels from an sdist archive.

Extracts the sdist to a temp dir and calls cibuildwheel on the
resulting package directory. Note that cibuildwheel will be
invoked with its working directory as the package directory, so
options aside from --output-dir and --config-file are relative to
the package directory.

positional arguments:
  package               Path to the sdist archive that you want wheels for.
                        Must be a tar.gz archive file.

optional arguments:
  -h, --help            show this help message and exit
  --output-dir OUTPUT_DIR
                        Destination folder for the wheels. Default:
                        wheelhouse.
  --config-file CONFIG_FILE
                        TOML config file. To refer to a file inside the sdist,
                        use the `{project}` or `{package}` placeholder. e.g.
                        `--config-file {project}/config/cibuildwheel.toml`
                        Default: "", meaning the pyproject.toml inside the
                        sdist, if it exists.

Any further arguments will be passed on to cibuildwheel.

joerick added 6 commits April 2, 2022 17:18
when calling like 'python -m cibuildwheel', we get errors like

usage: __main__.py [-h] [--platform {auto,linux,macos,windows}] [--archs ARCHS] [--output-dir OUTPUT_DIR] [--config-file CONFIG_FILE]
                   [--print-build-identifiers] [--allow-empty] [--prerelease-pythons]
                   [package_dir]
__main__.py: error: unrecognized arguments: --sad

With this change, we get error outputs like:

usage: cibuildwheel [-h] [--platform {auto,linux,macos,windows}] [--archs ARCHS] [--output-dir OUTPUT_DIR] [--config-file CONFIG_FILE]
                    [--print-build-identifiers] [--allow-empty] [--prerelease-pythons]
                    [package_dir]
cibuildwheel: error: unrecognized arguments: --asda
@henryiii
Copy link
Copy Markdown
Contributor

henryiii commented Apr 6, 2022

I'm still not very fond of this at all, especially after seeing it. It is unavailable from the GitHub action, much harder to get from pipx run, a new command to learn, all for exactly no user benefit. It has exactly the same commands, and is exactly orthogonal in primary argument - you can only run this on SDists, you can only run the original command on folders. From what I understand, this is entirely to keep the original command simple? Adding a brand new command to maintain and test likely negates that benefit. And document! Since Pip already takes folder or SDist, there's already a mental model that you can pass a folder or an SDist to a builder. If pypa/build adds SDist support, it will work the same way.

From a user perspective, they are passing in the exact same arguments, the API is identical. The only difference is some relative paths - but that's already a little murky, so I think just a little documentation would cover this just fine. Something like "if you pass an SDist, all paths except --output-dir and --config-file are relative to the temporary build folder".

Since there's no rush, can you leave this around for a week or two so I can try doing it without a new command in an alternative PR to see how bad it actually looks? At the very least, it could be split into two internal commands that have a single entrypoint (the current one) which would at least still be much better for users. But I think it can be even better.

@joerick
Copy link
Copy Markdown
Contributor Author

joerick commented Apr 6, 2022

Since there's no rush, can you leave this around for a week or two so I can try doing it without a new command in an alternative PR to see how bad it actually looks?

Sure. I did try that when reviewing #1020 which was my motivation for this approach. The problem I hit was changing the cwd to the expanded tar directory in-process meant that all the paths in options were wrong. At that point, not wanting to further complicate things, a separate process seemed a much more pragmatic solution. But maybe I missed something.

Another option to avoid a new command (though it might appear slightly perverse at first blush) would be for cibuildwheel to call /itself/ as a subprocess, once it's done the expansion of the sdist. That would avoid the extra entrypoint, and keep complexity down. But the complications around relative paths remain of course.

@henryiii
Copy link
Copy Markdown
Contributor

Update: will try to work on this before PyCon, but might have to be after...

@henryiii henryiii mentioned this pull request Apr 27, 2022
@henryiii henryiii deleted the from-sdist branch May 12, 2024 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Idea: build from SDists

2 participants