Adding CIBW_BUILD option #101
Conversation
YannickJadoul
force-pushed
the
cibw-build
branch
from
bcda88f
to
737d05b
Compare
|
Thanks. Quick look over the code mostly makes sense. I'll give it a shot over in twisted/twisted#1067. |
| - Only build on Python 3.6: `cp27-* cp34-* cp35-*` | ||
|
|
||
| *** | ||
| - Only build on Python 3.6: `CIBW_BUILD`:`cp36-*` |
There was a problem hiding this comment.
ohhhh CIBW_BUILD can do wildcards too! 🤯that's nice
There was a problem hiding this comment.
Your original code, just duplicated ;-)
| - Skip Python 2.7 on 32-bit Windows: `CIBW_SKIP`:`cp27-win32` | ||
| - Skip Python 3.4 and Python 3.5: `CIBW_SKIP`:`cp34-* cp35-*` | ||
| - Skip Python 3.6 on Linux: `CIBW_SKIP`:`cp36-manylinux*` | ||
| - Only build on Python 3 and skip 32-bit builds: `CIBW_BUILD`:`cp3?-*` and `CIBW_SKIP`:`*-win32 *-manylinux1_i686` |
cibuildwheel/__main__.py
Outdated
| @@ -99,7 +99,7 @@ def main(): | |||
| traceback.print_exc(None, sys.stderr) | |||
| exit(2) | |||
|
|
|||
| skip = BuildSkipper(skip_config) | |||
| selection = BuildSelector(build_config, skip_config) | |||
There was a problem hiding this comment.
BuildSelector seems like a reasonable name to me. selection is a little confusing though? Maybe just call it build_selector? open to other ideas though
There was a problem hiding this comment.
Myeah, I liked how skip(...) read, but couldn't really reproduce that
There was a problem hiding this comment.
The other option is to keep this as BuildSkipper, btw, and just add the extra argument of build_config? Depending on your point of view, this class is still determining what builds to skip.
There was a problem hiding this comment.
BuildSkipper feels like it relates too closely with CIBW_SKIP. I like it the way it is, I think, just the selection(identifier) is a bit too vague.
| return any(fnmatch(build_id, pattern) for pattern in self.patterns) | ||
| def match_any(patterns): | ||
| return any(fnmatch(build_id, pattern) for pattern in patterns) | ||
| return match_any(self.build_patterns) and not match_any(self.skip_patterns) |
There was a problem hiding this comment.
So the logic is 'match any of the build patterns, and none of the skip patterns.' Makes sense!
(b1 or b2 or b3) and not (s1 or s2 or s3)
(b1 or b2 or b3) and not s1 and not s2 and not s3
There was a problem hiding this comment.
Credit to @altendky for coming up with those semantics. But indeed, I've been thinking a little bit more, and this gives a really intuitive interpretation (there's not even really a 'first CIBW_BUILD is applied, and then CIBW_SKIP' kind of sequential semantics)
test/04_build_skip/environment.json
Outdated
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "CIBW_BUILD": "cp3?-*", | |||
| "CIBW_SKIP": "cp33-*" | |||
There was a problem hiding this comment.
Python 3.3 is about to be removed from cibuildwheel - maybe better to change this to Python 3.4
There was a problem hiding this comment.
Leaving it keeps this PR concise. :]
There was a problem hiding this comment.
Changed the folder name anyway, and added the line. So to almost any diff, this file is (almost) completely changed anyway, so might as well clean this up? :-/
|
It's a pretty minimal 'test' but I think it did what I wanted. |
|
Tests are failing because on OSX, there's now only 3 wheels being produced for the |
Pfff, I wanted to add a file with the list of files to be produced by a test, but then I need to start detecting the platform, as well. I could split the test again into one for |
Hmm. We've talked in the past of the need to update how these tests work with a .py file for each script. Maybe we create an issue to track that refactoring, and for now we can reduce that magic number to 3 (there's nothing special about minimum 4 wheels per platform). |
|
Done. As far as I'm concerned, I think this is ready, then. Let's keep subcommands for a completely different refactoring and PR? |
|
Do we need more review here? Trying it out in twisted/twisted#1067. |
|
Yeah, I'm still happy with it from what I've checked. @joerick, do you see anything else we need to address? |
|
Seems I'll need to rebase my changes. I'll do that in a bit. But as far as I'm concerned, this is probably OK. |
YannickJadoul
force-pushed
the
cibw-build
branch
from
bed2f19
to
076e81f
Compare
…sion in test 04_build_skip, and adding unit tests for BuildSelector
YannickJadoul
force-pushed
the
cibw-build
branch
from
076e81f
to
22a6f0c
Compare
|
Released in 0.10.0! Thanks both! |
Cfr. #98, this is adding CIBW_BUILD to positively select a set of build targets (rather than deselecting everything else).
I'm not completely happy with the naming of
BuildSelectorandselection(replacingBuildSkipperand theskipargument to the different platforms), but I can't think of anything better, right now.Comments, remarks, and identification of bugs are very welcome, @altendky, @astrofrog, and @joerick!