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

Appveyor #130

Closed
kwgoodman opened this issue Jun 21, 2016 · 31 comments
Closed

Appveyor #130

kwgoodman opened this issue Jun 21, 2016 · 31 comments

Comments

@kwgoodman
Copy link
Collaborator

Bottleneck seems to only get tested on windows right before a release---long after a commit that breaks bottleneck on windows might have been made. See, for example, #129.

It would be nice to test bottleneck on windows with Appveyor.

Anyone up for the challenge? @Midnighter?

We could skip the flake8 and sdist tests if that is helpful (we already do those tests on Travis). Maybe windows has a 32 bit option. Travis only has 64-bit OSes.

@Midnighter
Copy link
Contributor

I'm interested in taking this on in principal but I have a few deadlines coming up and thus fairly swamped with other work at the moment. 3-4 weeks is a time frame where I can commit to getting something done. If anybody else wants to give it a try I'd be happy to get them jump-started with what I have.

@kwgoodman
Copy link
Collaborator Author

That is great news!

@Midnighter
Copy link
Contributor

I've started to put something together over at this repository but I'm finding it really hard to set up properly only using AppVeyor. Does anyone have a windows machine available for local testing?

@kwgoodman
Copy link
Collaborator Author

Is appveyor giving any feedback about what is not working?

@Midnighter
Copy link
Contributor

It does, it's just a slow workflow since I have to push something to the repository then wait for AppVeyor in order to find out if it works or not. There are just a few things that'd be quicker to figure out locally.

@Midnighter
Copy link
Contributor

Midnighter commented Sep 10, 2016

You can follow the current status on AppVeyor here. Not even sure that it install Miniconda correctly at this point.

@kwgoodman
Copy link
Collaborator Author

Sounds like a painful process.

@Midnighter
Copy link
Contributor

Good news, the test project finally succeeds! I can now add AppVeyor CI testing to bottleneck. Which branch should I start with, master, c_rewrite, or a new one based off of one or the other?

@kwgoodman
Copy link
Collaborator Author

That's great!

Good timing. As of Friday the c_rewrite branch builds on Windows. How about if you branch off of c_rewrite?

After I remove cython we can then tweak the tests.

@kwgoodman
Copy link
Collaborator Author

I'm going to start removing cython from bottleneck. So you might want to wait for that to be done before adding appveyor. I'll let you know when I'm done.

@kwgoodman
Copy link
Collaborator Author

OK, I merged the c_rewrite branch into master. So you can work off of master now.

BTW I made some changes to travis: I removed cython and I removed the ci directory (ci/travis --> tools/travis).

@kwgoodman
Copy link
Collaborator Author

Bottleneck 1.2 is pretty much done. The only big ticket item is appveyor. Do you know when you'll have time to work on it?

@Midnighter
Copy link
Contributor

I'm at a conference until Sunday. I feel confident I can set up the AppVeyor testing in the week after that. Maybe already on Monday.

@kwgoodman
Copy link
Collaborator Author

Perfect.

@Midnighter
Copy link
Contributor

So basic building is set up. I did not include testing yet. You can see the results here. Lots of warnings about type conversions and some 64bit versions don't build. Do you want me to issue a pull request with this?

@kwgoodman
Copy link
Collaborator Author

Great work! I don't mind waiting for the unit tests. Probably better to have the tests running when making changes for window compat.

@Midnighter
Copy link
Contributor

So do you want a pull request so that you can play with it here or do you want to work off of my fork?

@kwgoodman
Copy link
Collaborator Author

I'll start using it after the unit tests are in place. If I start making changes without the unit tests I may break things without knowing it. But starting a PR is always good, any more commits you make to your branch will be automatically included in the PR. I wouldn't merge until you said it was done.

@Midnighter
Copy link
Contributor

Alright, added testing and those versions that build successfully also seem to be running the tests alright. It's all in #147.

@kwgoodman
Copy link
Collaborator Author

@Midnighter I merged your appveyor PR into master. Appveyor runs are here: https://ci.appveyor.com/project/kwgoodman/bottleneck/history

I see three issues:

  • compiler warnings (casting)
  • link errors on 64 bit
  • why does appveyor make a call to pypi.python.org?

The link errors are, of course, the most important. The first link error:

reduce.obj : error LNK2019: unresolved external symbol __imp_PyErr_Format
referenced in function _import_array

I don't know how to fix the link errors. @cgohlke, do you know the fix for the link errors?

@Midnighter
Copy link
Contributor

why does appveyor make a call to pypi.python.org?

It seems to check the information about pip from https://pypi.python.org/pypi/pip/json, discovers that the cached information is old and retrieves them again. Why? I'm not sure. Might be a conda thing. Doesn't seem to be doing any harm, though.

@cgohlke
Copy link
Contributor

cgohlke commented Oct 4, 2016

link errors on 64 bit

looks like you are using a 32-bit Python with a 64 bit compiler.

@kwgoodman
Copy link
Collaborator Author

@Midnighter do you know how to fix it? On the log for 64-bit py27 I see (note the 32): creating build\lib.win32-2.7. I'm not even sure which of our appveyor files is responsible.

@Midnighter
Copy link
Contributor

Unfortunately, I have no clue. I have a hunch that it's the MSVC used and therefore clues should be in tools/appveyor/windows_sdk.cmd but from what I see the correct logic is applied and it should be a build for 64bit.

Puzzlingly, for Python 3.5 64bit it also uses the 32bit library and yet the build and tests succeed. I wonder if I should toss this onto StackOverflow or some other forum.

@kwgoodman
Copy link
Collaborator Author

@cgohlke do you have any guidance?

@cgohlke
Copy link
Contributor

cgohlke commented Oct 4, 2016

It looks like all conda/Python environments created are 32-bit. Maybe I am missing something, but I don't see PYTHON_ARCH used anywhere in conda_wrapper.py

@Midnighter
Copy link
Contributor

It's a left over from dealing with CONDA_FORCE_32BIT which I didn't think was needed. On 64bit it should be undefined anyway and since we're dealing with 32bit on a native 32bit system that shouldn't be an issue either. However, I will play with setting that variable and see if it helps.

@Midnighter
Copy link
Contributor

According to my attempts this has nothing to do with CONDA_FORCE_32BIT but I figured out that there are 32bit and 64bit installs of Miniconda on any system. So it was just a matter of specifying the right path. Pull request is open #148.

@kwgoodman
Copy link
Collaborator Author

In previous releases Cython took care of most of Windows compatibility. Now, without Cython, we are on our own. So thank you @Midnighter for adding AppVeyor. It also gives us our first 32-bit test.

@kwgoodman
Copy link
Collaborator Author

@Midnighter after your AppVeyor fix I am seeing travis failures on osx. I don't see how it could be related. Do you use osx? If so, do the unit test pass on your computer? See #150

@Midnighter
Copy link
Contributor

Sorry, I'm a Linux guy. More on that in the other issue discussion.

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

No branches or pull requests

3 participants