Skip to content

fix compilation on Windows Python 2.7 + support for MINGW32 and Cygwin#133

Merged
szabadka merged 6 commits into
google:masterfrom
anthrotype:py27win
Aug 11, 2015
Merged

fix compilation on Windows Python 2.7 + support for MINGW32 and Cygwin#133
szabadka merged 6 commits into
google:masterfrom
anthrotype:py27win

Conversation

@anthrotype

@anthrotype anthrotype commented Aug 10, 2015

Copy link
Copy Markdown
Member

This includes:

  • a patch to setup.py to force distutils use MS Visual C++ 10.0, from Visual Studio 2010, instead of the default 9.0, Visual Studio 2008, when compiling for Windows Python 2.7. The latter does not support many of the modern C++ features required to compile the Brotli encoder. Windows Python version 3 and above are compiled with 2010 so they are ok.
  • two patches to setup.py to allow building Brotli with GCC for Windows, instead of the default Microsoft compiler. This can be done by passing the --compiler=mingw32 option to setup.py.
  • a patch to streams.cc to fix compilation under the Cygwin environment. Here GCC complains that 'malloc' and 'free' weren't declared in the current scope:
enc/streams.cc: In constructor ‘brotli::BrotliFileIn::BrotliFileIn(FILE*, size_t)’:
enc/streams.cc:92:32: error: ‘malloc’ was not declared in this scope
       buf_(malloc(max_read_size)),
                                ^
enc/streams.cc: In destructor ‘virtual brotli::BrotliFileIn::~BrotliFileIn()’:
enc/streams.cc:96:22: error: ‘free’ was not declared in this scope
   if (buf_) free(buf_);
                      ^
error: command 'gcc' failed with exit status 1

@anthrotype

anthrotype commented Aug 11, 2015

Copy link
Copy Markdown
Member Author

I just added a try/except clause to skip the distutils.msvc9compiler monkey patch when running setup.py under MinGW environment, whereby attempting to import the msvc9compiler module would raise a DistutilsPlatformError.

@szabadka

szabadka commented Aug 11, 2015

Copy link
Copy Markdown
Collaborator

I created a separate PR for the header issues. Could you merge your python changes in one PR? After I merge that, we should be good to cut the release, right?

@anthrotype

anthrotype commented Aug 11, 2015

Copy link
Copy Markdown
Member Author

thanks! sure, I'll do that now.

Cosimo Lupo added 6 commits August 11, 2015 11:09
[brotlimodule.cc] fix RuntimeError on OS X: keyword list must be NULL…
f141729 
… terminated

Sorry, my fault...
[setup.py] use MSVC 10.0 when compiling for Windows Python 2.7
c3540e2 
Python 2.7 for Windows is compiled using MS Visaul C++ 9.0 (Visual Studio 2008).
However the latter does not support many modern C++ features which are
required to compile the Brotli encoder. So we monkey-patch distutils
to always look for MSVC version 10.0 instead of 9.0.
[setup.py] support MINGW32 compiler: fix clashing 'hypot' definition;…
54baf43 
… statically link libgcc and libstdc++
[brotlimodule.cc] add __version__ string attribute to brotli module
ac33d35
[bro.py] print --version as brotli.__version__
8c7edd3
[setup.py] retrieve version string from brotlimodule.cc so we don't n…
d2c8b27 
…eed to modify it more than once
@anthrotype

anthrotype commented Aug 11, 2015

Copy link
Copy Markdown
Member Author

OK, i have consolidated the two PR branches in a single one (the current one).

I have also added a __version__ string constant to the brotli Python module, and set it to "0.1.0".
When installing the module, the setup.py uses regex to find the version string in the brotlimodule.cc file, so that we don't have to set it twice.

I compiled and run the tests on my local Windows and OS X machines.
All seems good now on the python side. You may wish to proceed cutting the release.

Thanks for the good work!

@anthrotype anthrotype mentioned this pull request Aug 11, 2015
Closed
szabadka added a commit that referenced this pull request Aug 11, 2015
@szabadka
Merge pull request #133 from anthrotype/py27win
d811b18 
fix compilation on Windows Python 2.7 + support for MINGW32 and Cygwin
@szabadka szabadka merged commit d811b18 into google:master Aug 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@anthrotype @szabadka