Build Python wheels for windows on travis #160

Czaki · 2019-09-04T08:05:52Z

This changes allow to build parts of windows wheels on travis-ci.
Supported python version: 3.5+
For python 2.7 I do not found method to install VCForPython27.msi. Try of installation with msiexec stuck without any action https://travis-ci.com/Czaki/cibuildwheel/builds/125771121
Try of installation it from choco fail because bug in DotNet3.5 formula. https://travis-ci.com/Czaki/cibuildwheel/builds/125753492

For python 3.4 there is collision with python installed by some tool under strange path. And I do not know how to write script for search its path. The collision is only for 32 bits version, butt I decided to remove both.

on the end of yaml file I put the lines which fails. Maybe someone with better windows knowledge find a way to solve it.

If there will be decision to not fight for old releases then also is possible to use https://www.nuget.org/packages/python/ as python source (which is suggested in python docs). But it contains only python from version 3.5.

YannickJadoul · 2019-09-09T09:53:15Z

@Czaki Amazing, thanks! Looks pretty good to me, as well.

I have one major suggestion: as far as I know, part of the idea is to abstract away the management of different versions of Python. So just like we download and install all the different pkg-files on macOS, it would be cibuildwheels philosophy to install the wheels inside of the cibuildwheel script (right, @joerick?).
Any idea how feasible that would be, @Czaki ?

Czaki · 2019-09-09T15:30:28Z

I change code to install python inside cibuildwheel. Initial python still need to be installed manually.

YannickJadoul · 2019-09-09T15:55:02Z

@Czaki Great, thank you very much! I'll have a close-up review later :-)

Czaki · 2019-09-09T20:37:20Z

@YannickJadoul You can also look on https://github.com/Czaki/cibuildwheel/tree/windows_on_travis2 branch. There is try of better protection of collision with existing installation based on [https://www.python.org/dev/peps/pep-0514/](pep 514).

Problem with python 3.4 is that there exist this version installation in windows image. But hidden on very strange path and windows installers do not allow to add second copy. In future this tools may upgrade python version and generate problems.

I do not found pep 514 entries for python 3.4, but maybe this protection will be usefull? If yes then I will clean the code. Or maybe You know how find existing python installation on windows.

joerick · 2019-09-09T21:14:33Z

This is heading in a good direction! Agree that installing within cibuildwheel is the right approach - the user shouldn't be concerned with that.

I have lots of smaller comments on the diff as it is now, but before that, would it be easy/possible to install Python directly from downloads from python.org rather than using choco? That would be preferable to me, since it reduces our dependency on other tools, which can be slow to publish packages, and they add potential security holes to the build process.

Czaki · 2019-09-09T21:22:55Z

I have bad experience with direct calling msiexec because it blocks all process without any error message, but I will try this in this week.

Czaki · 2019-09-10T11:04:26Z

@joerick What do you think about usage of nuget for installing? It is recommended in python docs as method of installng python for CI.
https://docs.python.org/3/using/windows.html#windows-nuget

YannickJadoul · 2019-09-10T16:08:05Z

.travis.yml

  include:
    # Linux Python 2
    - sudo: required
+      if: branch=master


Let's not forget about this when merging :-)

YannickJadoul · 2019-09-10T16:14:20Z

README.md

+| Travis CI       | ✅    | ✅    | ✅*     |
 | AppVeyor        |       |       | ✅      |
 | CircleCI        | ✅    | ✅    |         |
+> \* Travis provides windows machines but user need install python manually 


Fix typos to "Travis provides a Windows environment but Python needs to be installed manually" ?

YannickJadoul · 2019-09-10T16:15:19Z

cibuildwheel/__init__.py

@@ -1 +1 @@
-__version__ = '0.11.1'
+__version__ = '0.11.2'


Leave this out, please. @joerick will update the version when he makes a new release :-)

YannickJadoul · 2019-09-10T16:21:40Z

cibuildwheel/windows.py

+    bace_choco_args = "--no-progress --force -y --allowmultiple --override".split() 
+    instal_args = "'/quiet  InstallAllUsers=1 TargetDir={}'"
+
+    class PythonConfiguration(object):


Just a though: could we keep this as a simple namedtuple, and format/create the full args when we need them.

We could even just make a simple function get_choco_args that takes the version and arch and returns all the arguments? Then we don't need to change the current PythonConfiguration.

YannickJadoul · 2019-09-10T16:23:55Z

cibuildwheel/windows.py

-    if IS_RUNNING_ON_AZURE:
+    if IS_RUNNING_ON_AZURE or IS_RUNNING_ON_TRAVIS:
        # Python 3.4 isn't supported on Azure.
+        # I meet problem with install python on travis.


What problems are you getting, exactly? Do you have a link we can use for future reference? Also, please move this line down, to not split up "Python 3.4 isn't supported on Azure." and "See https://..." since these lines belong together.

Exact problem is that python installer do not allow to install two instances of same python version. One of pre-installed program has own instance of python 3.4 x64. But it is installed under strange path. I try to write part of code for discovery installation https://github.com/Czaki/cibuildwheel/commit/b0cefb63c75e17952a19dabe4ffe410b75af3254#diff-bf885b72fecfdf7485cab147f7957d5c but all info which i found in internet is connected with pep 514, but python 3.4 do not satisfy it.

should I describe it in comment inside code?

Aha, I see. It's probably just fine to write a single line that this version clashes with an already installed version of Python 3.4 on Travis CI's Windows image. Thanks!

YannickJadoul · 2019-09-10T16:24:55Z

test/shared/utils.py

        env.update(add_env)

-    subprocess.check_call(
+    p = subprocess.check_call(


What are these changes?

I will remove this. Artefact after debug.

Oh, don't worry. Sorry to be too quick about it!

YannickJadoul · 2019-09-10T16:28:09Z

@joerick @Czaki The current, very incomplete Travis CI docs for Windows seem to suggest using chocolatey: https://docs.travis-ci.com/user/reference/windows/

YannickJadoul · 2019-09-10T16:32:13Z

.travis.yml

+    - os: windows
+      language: shell
+      before_install:
+       - choco install python3 --version 3.5.4 --no-progress --force -y --allowmultiple --override --installargs "'/quiet  InstallAllUsers=1 TargetDir=C:\Python35-x64'"


Just for future reference, is there a reason we need all of these different arguments, here?
https://docs.travis-ci.com/user/languages/python/ seems to be suggesting to just choco install python (or maybe python3, I guess) and use the python executable afterwards.

I understand we need more flags/control for the different versions, but for the one that's just running cibuildwheel in the examples, it would be great to keep things simple.

--no-progress means not show progress of download. In other cases the 4MB limit of log on travis may run out.
-y accept all questions during installation.
-allowmultiple allow multiple versions of package. Not sure if need here. Needed with next installations
--override --installargs "'/quiet InstallAllUsers=1 TargetDir=C:\Python35-x64'" - set install directory

Czaki · 2019-09-12T12:59:59Z

I found that python installing through nuget do not conflict with existing python installation. So I switch to use nuget (installed with choco). This protect code from part of fail on changes in windows image.
Switch to nuget allows to simplify yaml file also.

I have cleaned a code.

Czaki · 2019-09-12T13:26:46Z

Error on Travis is connected with macos and problem with resolving dns name? Could You restart this test?

joerick · 2019-09-21T15:17:28Z

@Czaki is this ready for review? Let me know when and I'll take a proper look at the code :)

Czaki · 2019-09-22T20:48:58Z

Generally yes.
In second PR there shown this link https://github.com/pybind/python_example that show some method which will allow for enable support of python 2.7 on travis. But I think that this change can be target of next PR with enhance of c++ modern standards support.

joerick

Thank you @Czaki, this is getting there. I've got a few suggestions and comments of how to neaten it up before it's ready to merge.

cibuildwheel/windows.py

.travis.yml

README.md

cibuildwheel/windows.py

test/shared/utils.py

Co-Authored-By: Joe Rickerby <[email protected]>

YannickJadoul · 2019-10-13T12:19:06Z

test/01_basic/cibuildwheel_test.py

    # identifiers
    expected_wheels = utils.expected_wheels('spam', '0.1.0')
    build_identifiers = utils.cibuildwheel_get_build_identifiers(project_dir)
+    print("bb", build_identifiers)


What's this, still?

No problem. Very easily overlooked. I was just lucky to notice it ;-)

YannickJadoul · 2019-10-13T12:19:35Z

test/shared/utils.py

-
-
+ 
+ 


A small spurious change left?

cibuildwheel/windows.py

Czaki and others added 3 commits September 2, 2019 16:44

upgrade for windows on travis

bd583e2

Install python on travis windows, update readme, update tests

b4249c4

update readme, travis config and clean code

c6965bf

install python inside cibuildwheel run

e0b6881

Czaki force-pushed the windows_on_travis branch from 1205328 to e0b6881 Compare September 9, 2019 13:24

use register to get python path

b0cefb6

YannickJadoul reviewed Sep 10, 2019

View reviewed changes

Czaki and others added 5 commits September 12, 2019 07:14

fixes from review

eddc9c5

fix pip version

e38c203

use nuget for instal python

9003836

clean code

1cb337f

Merge branch 'master' into windows_on_travis

6e25817

Czaki mentioned this pull request Sep 15, 2019

upgrade macos version for support c++11 #156

Merged

joerick requested changes Oct 12, 2019

View reviewed changes

Czaki and others added 3 commits October 13, 2019 10:50

Merge branch 'master' into windows_on_travis

6d1bf25

Update cibuildwheel/windows.py

6b2d18f

Co-Authored-By: Joe Rickerby <[email protected]>

fixes from review

71e889a

joerick approved these changes Oct 13, 2019

View reviewed changes

YannickJadoul reviewed Oct 13, 2019

View reviewed changes

joerick added 2 commits October 13, 2019 21:30

Merge remote-tracking branch 'origin/master' into windows_on_travis

db38b33

Final bits of tidy up

5746456

joerick merged commit 80974e5 into pypa:master Oct 23, 2019

Czaki deleted the windows_on_travis branch October 24, 2019 07:00

Czaki mentioned this pull request Oct 24, 2019

Windows support in Travis CI #144

Closed

		@@ -1 +1 @@
		__version__ = '0.11.1'
		__version__ = '0.11.2'

Build Python wheels for windows on travis #160

Build Python wheels for windows on travis #160

Uh oh!

Conversation

Czaki commented Sep 4, 2019

Uh oh!

YannickJadoul commented Sep 9, 2019

Uh oh!

Czaki commented Sep 9, 2019

Uh oh!

YannickJadoul commented Sep 9, 2019

Uh oh!

Czaki commented Sep 9, 2019

Uh oh!

joerick commented Sep 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Czaki commented Sep 9, 2019

Uh oh!

Czaki commented Sep 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YannickJadoul commented Sep 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Czaki commented Sep 12, 2019

Uh oh!

Czaki commented Sep 12, 2019

Uh oh!

joerick commented Sep 21, 2019

Uh oh!

Czaki commented Sep 22, 2019

Uh oh!

joerick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

joerick commented Sep 9, 2019 •

edited

Loading