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

ARCH and REQUIRES_PYTHON plan #528

Closed
henryiii opened this issue Jan 10, 2021 · 23 comments · Fixed by #536
Closed

ARCH and REQUIRES_PYTHON plan #528

henryiii opened this issue Jan 10, 2021 · 23 comments · Fixed by #536

Comments

@henryiii
Copy link
Contributor

henryiii commented Jan 10, 2021

  1. We continue with CIBW_ARCHS as we are now; "auto" is the default, and auto (could) include universal2 for macOS, rather like it already includes 32bit for 64bit on the other OSs. I think most users will find this ideal for enabling or playing with the new architectures, since they are using things like cp36* now in build and that doesn't have to change unless they want to split based on it, see the final comments. macOS 11 considers "universal2" to be auto.
  2. We add a CIBW_MIN_PYTHON flag, with the usual variants (and maybe command line flag? Don't know what the policy is for that). If it is not set, we read the value from PEP 621's location if set (eventually), and for now use 2.7 (27?) if not set. For CIBW 2.0, we can change the default to 3.6 (36?), but users could get at least 2.7 back by manually specifying this. This is mostly so we can keep the defaults "nice", while also supporting the full range that we can support. From 2.0 on, we raise the default minimum to always match the official Python EOL. feat: support brace expansion in BUILD/SKIP #527
  3. (Optional) We upgrade to using the wcmatch library; this gives {} syntax, which I love using on the shell (partially because I'm a fish user and tab completion works correctly when using it, so it's perfect for moves and such). cp{27,35}* seems like it should work. Ve can activate the BRACE and NEGATE flags; this is simpler than my example above (basically just BUILD + SKIP), and is already well documented so we don't have to try to teach people how to use it, we can just point at this. Eventually, if this works well, we could push users away from the SKIP flag.
  4. (Optional) We could add a little javascript selection in the docs as shown above. I just think that's very handy and will be "flashy" :)

ARCH/MIN_PYTHON are about telling cibuildwheel what you are interested in supporting, and is something we can adjust over time to "ideal" defaults, unless a user overrides them, in which case they specifically want that thing. With SKIP, which is in this category sort of, user overrides blend CPython/PyPy, Python version, and OS-ARCH, so we can't control each default separately without the new variables. BUILD is more about telling it what "this" job is trying to build out of the total allowed set.

Further; I propose we split the unreleased "special" value for "arch" into three special values:

"native": The native architecture (always one value).
"auto": Produces the native architecture + 32bit on 64bit non-macOS systems, or + universal2 on macOS. Basically "everything you can run tests on". This would be default, just like auto for CIBW_PLATFORM. I think this was first proposed by @mayeut ?
"all": Every known architecture, 3 archs on macOS, 2 on Windows, and 5 on linux, currently. Could grow in the future.

This makes universal2 fit in nicely, and provides a simple way to recover classic behavior. If you want to run on macOS 11 but want the old behavior, just set CIBW_ARCHS_MACOS: native. That would work nicely if you have separate runs on hypothetical Apple Silicon runners someday. You could also use native if you use the architecture setting on the setup-python action, which is currently non-trivial to implement (due to different spellings on Windows and Linux).

And "all" lets you put all the work on build selectors; users would have to be aware that the "all" set could grow in future versions if new manylinux archs were added.

Native example:

matrix:
  - os: [windows-latest, ubuntu-latest]
  - architecture: [x86, x64]
...
- uses: actions/setup-python@v2
  with:
    python-version: 3.8
    architecture: ${{ matrix.architecture }}

# What to put here for CIBW to avoid two copies of 32bit wheels?

This is probably best worked on after #484 goes in.

Originally posted by @henryiii in #516 (comment)

@henryiii
Copy link
Contributor Author

henryiii commented Jan 10, 2021

@joerick and @YannickJadoul, feel free to edit the above as needed, tried to paste together something actionable. @mayeut feel free to commit. @Czaki too.

@YannickJadoul
Copy link
Member

Just one minor question about 2: is that something we want to introduce now already, or only when 2.0 is in sight?

@henryiii
Copy link
Contributor Author

henryiii commented Jan 10, 2021

I would introduce it now, so users who care can start explicitly pinning the min Python to 2.7. Maybe we could even print a warning if not pinned and an old wheel is built, saying it will change in 2.0 to defaulting to 3.6. I wouldn't change the default until 2.0, though, so bumping to 1.8 won't suddenly cause missing 2.7/3.5 wheels when manylinux still supports them.

Not that I really expect people to read warnings in CI logs. The folding at least helps.

I don't really know what @joerick plans for 2.0, basing this reasoning on SemVer.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 11, 2021

The days for PyPy 2 might be numbered after packaging drops support pypa/packaging#376 . Given they want to merge this before releasing a version that supports universal2 wheels, pypy2 might not be able to ever work properly on Apple Silicon.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 14, 2021

Minor change, now that I've played with packaging, let's make this CIBW_REQUIRES_PYTHON instead, and use packaging.selectors.SelectorSet. That will be identical to how project.requires-python will work, so it will be easier to set from that when we support it. So >=3.6 would be the default in 2.0, and >=2.7 will be the default now (we already don't support 3.0-3.4, so it's okay if they happen to match).

Sadly, setuptools spells this python_requires, but the Wheel metadata slot is Requires-Python, so that's the more "official" name and the one going forward. https://packaging.python.org/specifications/core-metadata/#requires-python

This was referenced Jan 14, 2021
@joerick
Copy link
Contributor

joerick commented Jan 15, 2021

Just to chime in, this plan makes sense to me! The only thing really outstanding on this is (2), the CIBW_MIN_PYTHON option.

let's make this CIBW_REQUIRES_PYTHON instead, and use packaging.selectors.SelectorSet. That will be identical to how metadata.requires-python will work, so it will be easier to set from that when we support it. So >=3.6 would be the default in 2.0, and >=2.7 will be the default now (we already don't support 3.0-3.4, so it's okay if they happen to match).

Hmmmmmmmm I'm 50/50 on this idea, versus the CIBW_MIN_PYTHON approach. I worry it might make the option more expressive than we really need, potentially creating confusion between this option and BUILD/SKIP. Also, if we can pull compatibility from metadata.requires-python, I think I'd rather encourage users to put that info there instead of using our option at all. We could document our option as a way to 'change the default' Pythons built, but present metadata.requires-python as the 'right' way to do it.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 15, 2021

It will complicate the option to have both CIBW_MIN_PYTHON and project.requires-python both with different semantics (okay, now that I think about it, not by much at all, just adding >= should convert them). I could also see it drawing people into trying to use CIBW_MIN_PYTHON instead just because they like the syntax better or see it as simpler / more correct. I like having it literally just a stand-in for metadata.requires-python if someone doens't want to change pyproject.toml yet, or doesn't have a pyproject.toml, or needs to override an incorrect setting in pyproject.toml.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 15, 2021

Feel free to check out the docs example of #536; after writing the docs I'm moderately/strongly in favor of CIBW_REQUIRES_PYTHON. It is much more of a mental burden to have an override for project.requires-python with a totally different name and syntax (CIBW_MIN_PYTHON), that we don't really want to push people toward anyway; it's more of an escape hatch when you are building a project when don't have control over the pyproject.toml file (or refuse to add one). It's not because CIBW_REQUIRES_PYTHON is more expressive, it's the extra overload of having two different ways to do the same thing. Also, every project should already have at exactly this string in setup.py/setup.cfg under python_requires= already.

It's pretty obvious that CIBW_REQUIRES_PYTHON overrides project.requires-python, but I could see a user thinking that CIBW_MIN_PYTHON complements or combines with project.requires-python like BUILD/SKIP somehow.

Notice how we'd have to draw attention to a MIN_PYTHON setting, while this slips in as just another spelling for project.requires-python.

@joerick
Copy link
Contributor

joerick commented Jan 16, 2021

I see your points. The implementation/docs are neater if they share syntax. But I'm still concerned about confusion between this and BUILD/SKIP, and the name is confusing, because it reads 'cbuildwheel requires python', which is isn't right because it's the package that requires python, but even then it doesn't really require it in this context, etc. etc.

Perhaps there's a way to reframe this by changing the name? I was wondering about CIBW_DEFAULT_PYTHON_VERSIONS, but using the same syntax as project.requires-python. I kinda like the long option name, because it seems like more of a niche or unusual option compared to BUILD/SKIP. Also it implies that you're changing a default, but that the package contains the 'real' setting - project.requires-python should override this, if both are set.

But I must admit CIBW_MIN_PYTHON still seems like a simpler solution to me. The purpose is that is clear - we can change the default, but users can set it if they want to build something older. But, I concede, it's less clear how that would interact with project.requires-python when both are set.

@henryiii
Copy link
Contributor Author

  • How about CIBW_PROJECT_REQUIRES_PYTHON? That still makes it clear that it's an override for project.requires-python.
  • A simpler solution isn't really simpler if there is already a more complex solution too, and people have to read both. Notices in the current docs, I don't have to discuss the variable at all, since it's simply the same as the item in the file. A new section would have to be added to explain MIN_PYTHON and how that relates. Also, simpler solutions tend to draw people in.
  • If someone wanted to drop Python 3.5 support but not 2.7 support, CIBW_MIN_PYTHON isn't powerful enough to describe that. Again, I'm not all that worried, since we have a better solution (the metadata read), but something to keep in mind; and there are reasons someone might not want to add the file/setting yet, not the least of which is setuptools has not added support yet. The goal here is to use BUILD to pick what you want to build out of all valid build configurations, and to have invalid configurations (due to Python versions a package does not support) not even part of the equation.
    • Even the current solution isn't powerful enough to drop CPython without PyPy. I think that's a clear special case - if you support PyPy 2.7 but not CPython 2.7, you have to use SKIP to filter CPython 2.7 out, since you do support the Python 2.7 language, just not a specific instance (cp27-*), and that's a job for build selectors. @mattip could correct me, but I think PyPy follows the Requires-Python metadata setting.
  • I think it is important to make it an override, as people sometimes have to build packages where they don't control the setup files. 99% of the time, the files should be right, but there are special cases. NumPy, IIRC, kept Python compatibility for one extra release after dropping it from Requires-Python. If you were building your own NumPy wheelhouse, you might want to override it. Especially for PyPy?...
  • Just a wild thought; we could look in setup.cfg:python_requires if pyproject.toml does not contain it. It is clearly stated in the PEP that pyproject.toml:project settings is considered canonical and overrides all other files, so the ordering here is easy; it comes after. It would be technically easy to add with configparser, it would enable projects today to start using it without adding a pyproject.toml setting. In fact, I use that instead of setup.py for non-dynamic values already in most of my projects, so no change would be needed for those, cibuildwheel would simply stop listing python versions I don't support. I have no idea if this is a good idea, just thought of it.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 16, 2021

PS: The implementation is actually the easiest part, I'm worried about adding more complexity for users by having two ways to do this that are not the same, but not implementation.

Something like this:

requires_python = f'>={os.getenv("CIBW_MIN_PYTHON", "2.7")}'

If we were to drop 3.5 sooner than 2.7, this would be more complicated.

@YannickJadoul
Copy link
Member

@henryiii's point about mixing the two is a valid concern, I think, though I do see the attraction of CIBW_MIN_PYTHON's simplicity.

If we were to drop 3.5 sooner than 2.7, this would be more complicated.

Not really, though. We've also dropped 3.4 without issues. Once manylinux forces us to drop it (soonish?), it's just removed from the lists of identifiers.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 16, 2021

Not really, though. We've also dropped 3.4 without issues.

This is not relevant. I'm talking about dropping a default but still allowing users to build with it by setting a different value (somewhere). Once it's dropped entirely, this all becomes unneeded because we never try to build for it anyway. When we drop 3.5, it's gone. I'm just saying if our default were to become 2.7 and 3.6+ but we still do support 3.5, that would not expressible with MIN_PYTHON. I think we plan to make our default 3.6+ in 2.0 and not change it before.

I do see the attraction of CIBW_MIN_PYTHON's simplicity

That's part of why I don't like it. It's attracting people to use it instead of the proper pyproject.toml setting, and since we have to document and explain both, it's less simple than simply not having it at all and only having a direct override for pyproject.toml.

@mattip
Copy link
Contributor

mattip commented Jan 16, 2021

Even the current solution isn't powerful enough to drop CPython without PyPy. I think that's a clear special case - if you support PyPy 2.7 but not CPython 2.7, you have to use SKIP to filter CPython 2.7 out, since you do support the Python 2.7 language, just not a specific instance (cp27-*), and that's a job for build selectors. @mattip could correct me, but I think PyPy follows the Requires-Python metadata setting.

Are there any packages that support PyPy2.7 but not CPython 2.7? I would love to think that is possible, but can not think of any examples, especially as python2.7 dies out. I am not sure what you mean by "PyPy follows the Requires-Python metadata setting". Where is that setting and how would a python implementation read it?

@henryiii
Copy link
Contributor Author

henryiii commented Jan 16, 2021

@mattip It's a metadata setting, set by python_requires in classic setuptools. Pip reads it (probably via packaging?) and normally looks for older and older versions until it finds a package that matches. I don't think it supports markers, and I assume pypy2 identifies as 2.7, so I'd be quite surprised if PyPy didn't follow this when installing packages with pip. (There is a way to get pip to ignore this, IIRC).

@henryiii
Copy link
Contributor Author

@mattip Quick followup: you can get pip to lie about the Python version here: https://pip.pypa.io/en/stable/reference/pip_install/#cmdoption-python-version

Or you can just ignore it altogether here: https://pip.pypa.io/en/stable/reference/pip_install/#cmdoption-ignore-requires-python

@joerick
Copy link
Contributor

joerick commented Jan 17, 2021

I had an idea about this earlier. Instead of a new option, we could change the default CIBW_BUILD from * to auto. auto means:

  • it pulls requires-python from your project.toml (or setup.cfg) and selects everything that's compatible with that
  • if it can't find requires-python, it selects all non-EOL Pythons - currently CPython 3.6+.

This means:

  • packages are only built with compatible Pythons by default
  • no extra option that could cause confusion with BUILD/SKIP

What I really like about this is that if a user explicitly says CIBW_BUILD: cp27*, they get that build. They don't have to also fiddle with a REQUIRES_PYTHON or a MIN_PYTHON to make that happen.

I also don't think this would conflict with existing uses of CIBW_BUILD, because behaviour when you're specifying CIBW_BUILD would be unchanged - it would only affect people not setting it. It should probably be a cibuildwheel 2.0 thing though, since under this new system you'd need to specify CIBW_BUILD: auto cp27* to build 2.7. Or CIBW_BUILD: *. Or just set your requires-python correctly :).

What do you think?

@YannickJadoul
Copy link
Member

YannickJadoul commented Jan 17, 2021

if it can't find requires-python, it selects all non-EOL Pythons - currently CPython 3.6+.

Having EOL Python versions doesn't occur that often, though? 2.7 is a special case, and 3.5 is kind of an exception (I surprised it hasn't been ejected from manylinux yet).

What I really like about this is that if a user explicitly says CIBW_BUILD: cp27*, they get that build. They don't have to also fiddle with a REQUIRES_PYTHON or a MIN_PYTHON to make that happen.

But I kind of like this thought, yes.

What about having * mean "I want to build all defaults", though? That would kind of change the meaning?
Or, a more realistic case: users having CIBW_BUILD="*-manylinux*". That would suddenly cancel all our defaults? :-/

@henryiii
Copy link
Contributor Author

henryiii commented Jan 17, 2021

CIBW_BUILD="*-manylinux*" This was just what I was about to bring up; this should still respect Requires-Python. This makes the simple build selector much more complex, and I think we get very little from it. Really, 99% of the time, users should not be building wheels for a Python version that is eliminated via Requires-Python - it will not be installable with pip unless you pass a special flag to ignore the check! CIBW_PROJECT_PYTHON_REQUIRES was just more of an emergency escape hatch for very special cases where this is needed. We could have a CIBW_IGNORE_PYHTON_REQUIRES setting instead, really (though I'm not fond of boolean env vars). The biggest issue is that it would be nice to piggy back on this feature to raise the default if this is not given - but maybe it's better to not worry about that and instead push users to always specify this in setup.cfg or pyproject.toml if they don't want to build Pythons they don't support. Everyone is supposed to be doing this already, but sometimes it's set in setup.py, where it's harder for us to pull it out before running the build. (unless there's a trick to run a PEP 517 build process to just get metadata, which I don't think there is, but could be wrong).

With universal and the new archs, *<arch> is likely to be common and shouldn't require also listing the Python versions.

How about making it an error to run cibuildwheel with nothing selected after BUILD and SKIP applied, and then we could make the error message smart if someone writes a selector that would have worked if Requires-Python was the only reason it came up empty? Then cp27* would show a nice "you should set CIBW_PROJECT_PYTHON_REQUIRES to >=2.7 or CIBW_IGNORE_PYHTON_REQUIRES: 1 or whatever we decide.

@henryiii
Copy link
Contributor Author

This is why I stopped pushing for left to right selection and !; most selector usage is not that complicated and users don't want to study a custom language. Requires-Python is a standard piece of metadata used since pip 8 (though worded backwards in setuptools). I feel auto and the possible workarounds to try to cautiously workaround Requires-Python would be much more complicated and fragile a solution than something explicit disabling it.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 17, 2021

Just a thought; how about we don't plan on changing the default? cibuildwheel then will always try to build all Pythons it supports, unless Requires-Python is set (in pyproject.toml or setup.cfg, we can't pull python_requires= out of setup.py). This would allow "good" Python projects to build correctly out of the box (improving #468)1, but users would never have to look in the documentation to "fix" a missing build that we support. I still like having CIBW_PROJECT_REQUIRES_PYTHON (or the IGNORE if that's preferred) as an emergency override for the (very rare) reasons I listed, but it no longer has to be used by a setup.py only project supporting Python 2.7 (for example). 99% of projects could just ignore it, but we get a better experience for those using the static setting.

Footnotes

  1. Sadly, "good" here is defined as projects with a statically accessible value for it; setting python_requires="..." in setup() is perfectly valid, but can't be safely read before you pick a Python to build the wheel. There is a mechanism for this in pep518, but you can't run setup.py if it had a if sys.version_info<(3,0) in it, for example. Parsing the AST is how setuptools picks up versions nowadays if you ask for them using attr:, but (as seen there) is fragile as well if someone does something "clever".

@joerick
Copy link
Contributor

joerick commented Jan 18, 2021

Or, a more realistic case: users having CIBW_BUILD="*-manylinux*".

Ah, dang, I knew it was too good to be true.

Anyway, I think we're all on the same page about reading python-requires, and by default deselecting identifiers that the package doesn't support, so that's good.

So maybe I'm coming around to the CIBW_PROJECT_REQUIRES_PYTHON idea.... if we can write docs like it's letting you 'override compatibility', maybe it won't feel much like its the option to 'choose what Pythons to build with'....

Playing with docs...

## CIBW_PROJECT_REQUIRES_PYTHON

By default, cibuildwheel reads your package's Python compatibility from
pyproject.toml or setup.cfg. If you need to override this behaviour for some
reason, you can use this option.

When setting this option, the syntax is the same as `project.requires-python`,
using 'version specifiers' like `>=3.6`, according to
[PEP440](https://www.python.org/dev/peps/pep-0440/#version-specifiers).

Default: reads your package's Python compatibility from pyproject.toml
(`project.requires-python`) or setup.cfg (`options.python_requires`). If not
found, assumes the package is compatible with all versions of Python.

!!! note
    Rather than using this option, it's recommended you set
    `project.requires-python` in `pyproject.toml` instead - that way, `pip`
    knows which wheels are compatible when installing.

    Example `pyproject.toml`:

    ```toml
    [project]
    requires-python = ">=3.6"

    # aside- pyproject.toml also requires you specify some build system
    # options, like this
    [build-system]
    requires = ["setuptools", "wheel"]
    build-backend = "setuptools.build_meta"
    ```

    Example `setup.cfg`:

    ```ini
    [options]
    python_requires = ">=3.6"
    ```

#### Examples

```yaml
CIBW_PROJECT_REQUIRES_PYTHON: ">=3.6"
```

(I started writing the above and then saw your docs in the PR @henryiii, so I've combined a few of your ideas in there too)

Just a thought; how about we don't plan on changing the default?

Haha, I was also thinking about the same thing. Respecting requires-python will go a long way to improving the first run experience and not throwing devs into Python 2 errors immediately. Also, how many more releases do we need to do with Python 2.7/3.5 support anyway? Probably no more than a few months.

But, as previously discussed, this option would still give us the ability to deprecate old versions of Python, simply by changing what we assume when we can't find Python-Requires.

@joerick
Copy link
Contributor

joerick commented Jan 18, 2021

It looks like that's already what's happening in #536!

@henryiii henryiii changed the title ARCH and MIN_PYTHON plan ARCH and REQUIRES_PYTHON plan Jan 22, 2021
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 a pull request may close this issue.

4 participants