-
Notifications
You must be signed in to change notification settings - Fork 133
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
Windows build broken #37
Comments
Unfortunately the commit didn't fix it. Luarocks output ends here;
searching on this |
It works on VC 12. |
Not on MinGW (and apparently not on any older VC versions). Here's the code; https://github.com/diegonehab/luasocket/blob/76ed24fe8ae8c728de0d7d065918a5cd74fe7303/src/inet.c#L515-537 |
Digging around, found this: |
Could you try the last version. Note:
|
Tried it today, but failed again. When building the rockspec:
After modifying the rockspec to define "LUASEC_INET_NTOP" it still fails;
I'm travelling for a couple of weeks, so its hard to test for me right now. @ignacio can you give this a try? Note: I don't understand the issue. Apparently inet_ntop is defined from Vista onwards, yet on my Win7 system it continues to fail. I understand from @brunoos that it does work with the MS compilers. But I'm using MinGW, with gcc 4.8.1, on Win7 (32bit). |
I tried on WinXP (SP3) with VS 10.
|
Again, these defines work for WinXP (SP3) The last time I tested on Win 7 with VS 12, I didn't have to define anything. I will test again. |
gosh, I missed the last part... I will way for more feedback. |
Tried today with VS2008 and WIndows 7, using It compiles but fails to link:
We need this hack in order to compile with luarocks with MS compilers. By the way, once the problem with MS compilers is sorted out, I have a PR adding appveyor integration, so we can test using different MS compilers, plus MinGW on Windows. |
Rockspec updated. It seems to work now. |
Sadly, it still won't build with LuaRocks and a MS compiler. |
What is the error? If you have:
It is a LuaRocks issue. |
Yes, that is the error. Could we get this fix added in the meantime? I know this is a LuaRocks thing, but it'd really nice to get luasec to build with ms compilers. |
sorry, I will not add this patch to deal with this LuaRocks issue. I realized that luasec has 6 variations on Windows:
Besides, we have linux, osx and *bsd. I will not add workarounds to deal with build tools in the code if it is not luasec fault. This is a patch to luarocks that add an "exports" field, which informs the modules that must be exported from DLL.
Using with rockspec:
|
Fair enough. Thanks for the patch above. Would you mind submitting it to LuaRocks? There is an issue here were this matter has been discussed. |
Ah, also, while in the process of setting up a continuous integration server (appveyor) I found that the mingw build is failing with this error:
It seems that it is using mingw 4.8.2 (http://www.appveyor.com/docs/installed-software#mingw-msys-cygwin). If I edit the rockspec and only add this defines: defines = {
"WIN32", "NDEBUG", "_WINDOWS", "_USRDLL", "LSEC_EXPORTS", "BUFFER_DEBUG", "LSEC_API=__declspec(dllexport)",
"LUASEC_INET_NTOP", "_WIN32_WINNT=0x0501"
} the it compiles ok. What would be the minimum mingw version that is supported? |
Hi. Any thoughts on this? I'm talking about the possibility of having a patch in the rockspec to support MS compilers. Would you accept a PR for that? |
I can add the "defines" without problem. |
About the "defines", I'll need @Tieske thoughts on this since he is more knowledgeable on all things mingw. I was also talking about adding a patch in the rockspec, to let luasec compile with MS compilers. Could you add that too to the rockspec if I sent you the required changes? |
Yes, I add. |
Hi @brunoos I did not change anything to the defines because mingw is not my area of expertise and I'm afraid I can break something inadvertently. |
I just tried seems to work just fine |
Great to hear that, @Tieske |
frankly I don't have a clue... |
#21 broke the Windows build by using unix only code.
The text was updated successfully, but these errors were encountered: