feat: stricter selector parsing, refactor to platforms module
#2291
Conversation
joerick
force-pushed
the
selector-strict
branch
3 times, most recently
from
f7c0412 to
6942c47
Compare
|
This change made the unit tests quite a bit slower (26s), so I broke out the profiler. In doing so I spotted a few places things could be sped up. I'll split out the unrelated changes to a separate PR. |
|
Do you know why this change made it slower, though? |
|
Yeah, it was these lines, it ended up doing a lot of TOML loading and running of the BuildSelector. |
joerick
force-pushed
the
selector-strict
branch
from
6942c47 to
7b94bcd
Compare
|
I've kept only the optimisation relevant to this change, the rest are in #2293. |
|
Can be rebased now. |
- Use a different method to build nothing - Make the check aware of enable groups
Unit test time: 26.2s -> 13.1s
joerick
force-pushed
the
selector-strict
branch
from
7b94bcd to
52c3e8d
Compare
| def build(self, options: Options, tmp_path: Path) -> None: ... | ||
|
|
||
|
|
||
| ALL_PLATFORM_MODULES: Final[dict[PlatformName, PlatformModule]] = { |
There was a problem hiding this comment.
This requires all platforms be imported no matter what platform you are targeting, but I guess since get_python_configurations is now needed from every platform, that's fine.
| from cibuildwheel.typing import GenericPythonConfiguration, PlatformName | ||
|
|
||
|
|
||
| class PlatformModule(Protocol): |
There was a problem hiding this comment.
Not in this PR, but I think this will be a good structure to build on in the future; we could have several required methods. Now that we have gone from 3 backends to 5 (soon 6), more structure is good and Protocols are a good way to do it.
There was a problem hiding this comment.
BTW, this module type is working well - mypy caught that the ios module was missing the new all_python_configurations function after the rebase.
Implementation of an idea from #2070 (comment)
cibuildwheel will error out if the
buildconfig contains something that it doesn't recognise. Forskipandtest-skip, it just prints a warning.This is to highlight misconfiguration as seen in #2070. Notably, the set of identifiers that cibuildwheel will consider is not dependent on platform, so users can still do a global CIBW_BUILD that's shared across platforms.
Also in this PR, I wanted to iterate through platform modules, and so decided it was time to factor them out into their own package.
I'll keep this draft to merge #2286 first, since that adds a new platform.