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

build with visual studio. #12

Closed
wants to merge 2 commits into from

Conversation

tempbottle
Copy link

build with "python setup.py build install", and run the tests with "nosetests -v -w tests"

@saghul
Copy link
Owner

saghul commented Mar 27, 2012

Hi,

Thanks for your effort! Sorry to be pedantic, but here are some preliminary comments:

  • The patch does not apply on current master, you'll need to rebase it

  • For the moment pyuv is built without gyp. I see you added a couple of patches to the gyp files, but gyp_uv is not called.

  • Don't bump the libuv revision number as part of a big patch like this one

  • Variable names are cheap, use self.compiler_type instead of self.cc

  • Use that exec_process function instead of os.system

  • Remove this log line tempbottle@d4305dc#L3R151

  • Can't his be simplified? tempbottle@d4305dc#L4R59
    I was thinking something like this:

    #if defined(_MSC_VER)
    #define func FUNCTION

  • Why this change? tempbottle@d4305dc#L5R94

Again, sorry to be so picky, but the one which will maintain this code later will be me :-)

@saghul
Copy link
Owner

saghul commented Apr 12, 2012

I spent some time trying out this today and I can't explain with bare words how much I hate the Windows ecosystem.

We need VS 2008 support because the Python binaries distributes at python.org are built with it. Unfortunately, libuv is not adapted to work with VS2008. I made some changes and it did compile, but when the Python module is built a ton of errors happen at link stage.

I'm sticking with MinGW for now...

@ghost ghost assigned saghul Aug 29, 2012
@saghul
Copy link
Owner

saghul commented Oct 24, 2012

Closing, finally fixed this in master.

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 this pull request may close these issues.

None yet

2 participants