Skip to content

Win32 build #118

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

Closed
wants to merge 4 commits into from
Closed

Win32 build #118

wants to merge 4 commits into from

Conversation

mwiebe
Copy link
Member

@mwiebe mwiebe commented Jul 20, 2011

Commit from Christoph Gohlke, and fix for http://projects.scipy.org/numpy/ticket/1909.

I don't have easy access to a win32 build machine at the moment, so this needs testing before it goes in.

Mark Wiebe added 2 commits July 20, 2011 11:54
This uses the gcc versions of gmtime_s and localtime_s instead of the
MS versions when __GNUC__ is defined.
@rgommers
Copy link
Member

Still failing for me. I can't open the ticket at the moment, but I think it fails at a later point with a similar error. So your fix may be correct but it may have to be applied in more places:

creating build\temp.win32-2.6\Release\numpy\core\src\multiarray
compile options: '-Inumpy\core\include -Ibuild\src.win32-2.6\numpy\core\include/numpy -Inumpy\core\src\private -Inumpy\core\src -Inumpy\core -Inumpy\core\src\npymath -Inumpy\core\src\multiarray -Inumpy\core\src\umath -Inumpy\core\src\npysort -Inumpy\core\include -IC:\Python26\include -IC:\Python26\PC -Ibuild\src.win32-2.6\numpy\core\src\multiarray -Ibuild\src.win32-2.6\numpy\core\src\umath -c'
gcc -mno-cygwin -O2 -Wall -Wstrict-prototypes -Inumpy\core\include -Ibuild\src.win32-2.6\numpy\core\include/numpy -Inumpy\core\src\private -Inumpy\core\src -Inumpy\core -Inumpy\core\src\npymath -Inumpy\core\src\multiarray -Inumpy\core\src\umath -Inumpy\core\src\npysort -Inumpy\core\include -IC:\Python26\include -IC:\Python26\PC -Ibuild\src.win32-2.6\numpy\core\src\multiarray -Ibuild\src.win32-2.6\numpy\core\src\umath -c numpy\core\src\multiarray\multiarraymodule_onefile.c -o build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o
g++ -mno-cygwin -shared build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o -LC:\Python26\libs -LC:\Python26\PCbuild -Lbuild\temp.win32-2.6 -lnpymath -lnpysort -lpython26 -lmsvcr90 -o build\lib.win32-2.6\numpy\core\multiarray.pyd
build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o:multiarraymodule_onefile.c:(.text+0x7b15): undefined reference to `localtime_r'
build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o:multiarraymodule_onefile.c:(.text+0x8139): undefined reference to `gmtime_r'
build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o:multiarraymodule_onefile.c:(.text+0x98d1): undefined reference to `localtime_r'
collect2: ld returned 1 exit status
error: Command "g++ -mno-cygwin -shared build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o -LC:\Python26\libs -LC:\Python26\PCbuild -Lbuild\temp.win32-2.6 -lnpymath -lnpysort -lpython26 -lmsvcr90 -o build\lib.win32-2.6\numpy\core\multiarray.pyd" failed with exit status 1

@mwiebe
Copy link
Member Author

mwiebe commented Jul 20, 2011

Hmmm, that's a big strike against the idea of using mingw... It supports neither MSVC's gmtime_s nor gcc's gmtime_r, only allowing for the ancient non-threadsafe gmtime. Is it necessary to support mingw? This is rather bad.

@rgommers
Copy link
Member

According to http://gcc.gnu.org/ml/fortran/2009-05/msg00348.html it should be threadsafe for MinGW. And yes, we do need to support it. It's still the default open source solution on Windows. We build our Windows binaries with it.

@mwiebe
Copy link
Member Author

mwiebe commented Jul 20, 2011

There's a patch which should hopefully work on mingw, let me know what it does.

@rgommers
Copy link
Member

Next problem:

...
numpy\core\src\multiarray\multiarraymodule.c:3932: warning: dereferencing type-punned pointer will break strict-aliasing rules
numpy\core\src\multiarray\multiarraymodule.c:3934: warning: dereferencing type-punned pointer will break strict-aliasing rules
numpy\core\src\multiarray\multiarraymodule_onefile.c: At top level:
numpy\core\src\multiarray\mapping.c:76: warning: '_array_ass_item' defined but not used
error: Command "gcc -mno-cygwin -O2 -Wall -Wstrict-prototypes -Inumpy\core\include -Ibuild\src.win32-2.6\numpy\core\include/numpy -Inumpy\core\src\private -Inumpy\core\src -Inumpy\core -Inumpy\core\src\npymath -Inumpy\core\src\multiarray -Inumpy\core\src\umath -Inumpy\core\src\npysort -Inumpy\core\include -IC:\Python26\include -IC:\Python26\PC -Ibuild\src.win32-2.6\numpy\core\src\multiarray -Ibuild\src.win32-2.6\numpy\core\src\umath -c numpy\core\src\multiarray\multiarraymodule_onefile.c -o build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o" failed with exit status 1

Full build log attached on #1909

@mwiebe
Copy link
Member Author

mwiebe commented Jul 21, 2011

Those warnings suggest that -fno-strict-aliasing should be added to the compiler flags. Python 3+ allows for the removal of that compiler option, but I don't think that has been rigorously checked with NumPy.

These functions allegedly use thread-local storage on mingw,
so hopefully it's ok.
@mwiebe
Copy link
Member Author

mwiebe commented Jul 21, 2011

I've updated the patch to fix the error that was in the logfile.

@rgommers
Copy link
Member

Builds fine, that's progress. Not sure what to do about the MinGW issues though (see #1909).

@mwiebe
Copy link
Member Author

mwiebe commented Jul 21, 2011

That's ugly, it would be better to use the gmtime_s variant like in the original patch, since it is in msvcr90.dll. I'll give you a patch with an attempted hack. - on second thought, the error was happening at link-time, so would probably have to use whatever localtime_s and gmtime_s translate to at the binary level, like _localtime32_s or _localtime64_s.

@mwiebe
Copy link
Member Author

mwiebe commented Jul 21, 2011

There's a new commit for you to try.

@rgommers
Copy link
Member

Looks like we're back to the original problem:

creating build\temp.win32-2.6\Release\numpy\core\src\multiarray
compile options: '-Inumpy\core\include -Ibuild\src.win32-2.6\numpy\core\include/numpy -Inumpy\core\src\private -Inumpy\core\src -Inumpy\core -Inumpy\core\src\npymath -Inumpy\core\src\multiarray -Inumpy\core\src\umath -Inumpy\core\src\npysort -Inumpy\core\include -IC:\Python26\include -IC:\Python26\PC -Ibuild\src.win32-2.6\numpy\core\src\multiarray -Ibuild\src.win32-2.6\numpy\core\src\umath -c'
gcc -mno-cygwin -O2 -Wall -Wstrict-prototypes -Inumpy\core\include -Ibuild\src.win32-2.6\numpy\core\include/numpy -Inumpy\core\src\private -Inumpy\core\src -Inumpy\core -Inumpy\core\src\npymath -Inumpy\core\src\multiarray -Inumpy\core\src\umath -Inumpy\core\src\npysort -Inumpy\core\include -IC:\Python26\include -IC:\Python26\PC -Ibuild\src.win32-2.6\numpy\core\src\multiarray -Ibuild\src.win32-2.6\numpy\core\src\umath -c numpy\core\src\multiarray\multiarraymodule_onefile.c -o build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o
g++ -mno-cygwin -shared build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o -LC:\Python26\libs -LC:\Python26\PCbuild -Lbuild\temp.win32-2.6 -lnpymath -lnpysort -lpython26 -lmsvcr90 -o build\lib.win32-2.6\numpy\core\multiarray.pyd
build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o:multiarraymodule_onefile.c:(.text+0x7b1d): undefined reference to `_localtime64_s'
build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o:multiarraymodule_onefile.c:(.text+0x813f): undefined reference to `_gmtime64_s'
build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o:multiarraymodule_onefile.c:(.text+0x98dc): undefined reference to `_localtime64_s'
collect2: ld returned 1 exit status
error: Command "g++ -mno-cygwin -shared build\temp.win32-2.6\Release\numpy\core\src\multiarray\multiarraymodule_onefile.o -LC:\Python26\libs -LC:\Python26\PCbuild -Lbuild\temp.win32-2.6 -lnpymath -lnpysort -lpython26 -lmsvcr90 -o build\lib.win32-2.6\numpy\core\multiarray.pyd" failed with exit status 1

@mwiebe
Copy link
Member Author

mwiebe commented Jul 21, 2011

Can you use some dumping tools to symbol dump msvcr90.dll or msvcr90.lib and find what the actual exported symbols are for the localtime and gmtime functions?

@rgommers
Copy link
Member

Full dump attached to #1909.

[ 626] _localtime32
[ 627] _localtime32_s
[ 628] _localtime64
[ 629] _localtime64_s

[ 499] _gmtime32
[ 500] _gmtime32_s
[ 501] _gmtime64
[ 502] _gmtime64_s
[ 788] _mkgmtime32
[ 789] _mkgmtime64

@mwiebe
Copy link
Member Author

mwiebe commented Jul 24, 2011

That seems to indicate that I got the signature right. In your latest build error, there is:

multiarraymodule_onefile.c:(.text+0x7b1d): undefined reference to `_localtime64_s'

which is in the objdump output. One thing which is sometimes tricky is that C adds an '_' at the beginning of normal functions, however objdump appears to be stripping that as can be seen starting from symbol [1205] abort.

Maybe the .lib file being used to describe the symbols in msvcr90.dll is the the default mingw uses, which doesn't have these symbols? In that case, perhaps that .lib file could be regenerated specifically for msvcr90.dll to include these symbols?

@rgommers
Copy link
Member

That's what I understood to be the problem from http://bugs.python.org/issue3308, and regenerating the .lib file worked for one user: http://article.gmane.org/gmane.comp.gnu.mingw.user/27013. This is not a nice fix though, it may work for me but we can hardly tell all MinGW users to do this.

It is possible that this is fixed in a later gcc version, I see 4.5.2 is available on SF. If that works, also with g77 for scipy, perhaps we should just clearly state that the default gcc that comes with MinGW (3.4.5) is too old and won't work for numpy >1.6.

@mwiebe
Copy link
Member Author

mwiebe commented Jul 26, 2011

I'm unable to think of a better option than using a .lib file which matches the linked version of msvcr. The datetime stuff needs these functions to work properly with local times.

@rgommers
Copy link
Member

I'll try to test gcc 4.5.2, but that not going to be before Sunday. If that works without problems I'd be okay switching to that. Otherwise a .lib file would be okay, we could just put it under tools/ somewhere.

@charris
Copy link
Member

charris commented Jul 26, 2011

"... a .lib file would be okay..."

IIRC, I once had to do that to get mingw working. I don't think it is a bad solution to a bad situation as long as keeping the .lib file current doesn't become a hassle.

@charris
Copy link
Member

charris commented Aug 13, 2011

What is the status of this?

@rgommers
Copy link
Member

I haven't tried with gcc 4.5 yet - will try to make time for that soon.

@87
Copy link
Contributor

87 commented Aug 23, 2011

MinGW is missing the declarations for localtime_s in time.h, that is why it won't work.. I think it could be added to the datetime headers for compatibility, but that seems a bit of a hack.. I'll try it out and let you know.

@87
Copy link
Contributor

87 commented Aug 23, 2011

Ahh, that was only half the story -- sorry! If MinGW doesn't find the correct entries in msvcr90.dll, I don't know..

@mwiebe
Copy link
Member Author

mwiebe commented Aug 23, 2011

I believe this is because mingw includes a .lib version based on visual c++ 6.0. It looks like what is needed is an updated .lib file generated from msvcr90.dll.

@rgommers
Copy link
Member

Do you happen to know if that's still the case for gcc 4.5.2 in MinGW? If not, I'll finally make time for testing that.

@87
Copy link
Contributor

87 commented Aug 23, 2011

I'll have to check it at work.. I downloaded its latest MinGW version from sourceforge[1], but I didn't check the actual compiler version.

[1] http://sourceforge.net/projects/mingw/files/Automated%20MinGW%20Installer/mingw-get-inst/

@87
Copy link
Contributor

87 commented Aug 24, 2011

Yes, I checked, the compiler version is 4.5.2.

@87
Copy link
Contributor

87 commented Aug 24, 2011

Good news! Creating a customized libmsvcr90.a file seems to work, throwing no localtime_s error at the tests anymore.

To make it work in my specific case, I had to add to datetime_strings.c:

#if defined(_WIN32) && defined(__GNUC__)
    #define localtime_s _localtime32_s
    #define gmtime_s _gmtime32_s
#endif

And hacked the original msvcrt.def.in to make a .a lib with dlltools, as described here:
http://www.emmestech.com/moron_guides/moron1.html

C:\MinGW\bin>dlltool --input-def ..\in\msvcrt90.def --dllname MSVCR90.dll --output-lib ..\out\libmsvcr90.a -v -k

(See this Gist for the source(s): https://gist.github.com/1167840#file_msvcrt90.def , I basically stripped the original .def.in file from all exports outside of version 9.0 and added the missing functions.)

The resulting file can be placed in MinGW\lib and the linker will find the references. (It seems time.h was imported from the visual studio repo, because I didn't have to modify it..)

@87
Copy link
Contributor

87 commented Aug 24, 2011

The method is not very portable, though, you have to do a lot of extra steps as user to make it work, and I haven't looked at the debug versions, the other compiler versions, or the header file. It would be nicest if MinGW supported these functions out of the box, but http://old.nabble.com/Need-localtime_s-td13087586.html seems to indicate they wont..

@mwiebe
Copy link
Member Author

mwiebe commented Aug 24, 2011

In the patch 0ed2d56 above, I put code in for calling _localtime64_s (which should be available on both 32-bit and 64-bit platforms). Can you try it with that instead of the 32-bit one, since it's prefereable to always use the 64-bit time_t?

@87
Copy link
Contributor

87 commented Aug 24, 2011

I can try the above patch to see if it works, but it seems a bit complicated? I think, if one has access to the time.h definitions and the .a files needed to link with msvcrt[x].dll, it should work. The time.h definitions could be included in the source directly if necessary, you need the time struct and timestamp types. Only, not every version of visual c has the *_s definitions I believe.

I'll put in _localtime64_s and see how it goes.

@87
Copy link
Contributor

87 commented Aug 26, 2011

#if defined(_WIN32) && defined(__GNUC__)
    #define time_t __time64_t
    #define localtime_s _localtime64_s
    #define gmtime_s _gmtime64_s
#endif

Works on WinXP 32 (with VC9 dll and modified mingw lib).

@87
Copy link
Contributor

87 commented Sep 8, 2011

I was thinking, would it be an idea to make one inlined function to handle all the different cases?

There is a distinction between:

  • Linux (localtime_r: theadsafe)
  • Windows VS2003- (localtime: non-threadsafe)
  • Windows VS2005+ (localtime_s: threadsafe)
  • Windows MinGW (localtime: non-threadsafe), but with hacks it could be possible to link with localtime_s

@mwiebe
Copy link
Member Author

mwiebe commented Sep 8, 2011

Sounds like a good idea to me. I think the MinGW one should be fixed up to always use localtime_s, especially since that's what the official Windows binaries are made with.

@87
Copy link
Contributor

87 commented Sep 8, 2011

Is there a way to detect at runtime if a certain dll-function can be found? That would make it easier for the different MinGW paths. Also, should non-threadsafe function calls be wrapped with the GIL?

@87
Copy link
Contributor

87 commented Sep 14, 2011

Ok, I made a pull request which should fix these issues. Would you want to take a look at it? I tested it against Python 2.6, the Python 2.5 build crashed for MinGW on an unrelated issue.

I am not completely sure on the thread-safety of the fallback mechanism, which uses the plain localtime and gmtime. I didn't know if it would be a good idea though to just put in *ALLOW_THREADS in reverse order..

@rgommers
Copy link
Member

Han's pull request works for me now (at least on py2.6, will test some more). I haven't tried that in combination with the commits in this pull request, but they look unrelated.

@charris
Copy link
Member

charris commented Oct 1, 2011

This can be tested again as soon as Han's pull request goes in.

@87
Copy link
Contributor

87 commented Oct 2, 2011

I think the commits here are not necessary anymore..

@charris
Copy link
Member

charris commented Oct 8, 2011

Should I close the pull request then?

@charris
Copy link
Member

charris commented Oct 22, 2011

Closing. If needed the fixes can be submitted again.

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.

4 participants