-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feature: added support for MSVC compiler. #1222
base: master
Are you sure you want to change the base?
Conversation
@geniuss99 Thanks for your hard work on this! It seems that the official nginx can already build on Win32 without patching: http://nginx.org/en/docs/howto_build_on_win32.html Maybe we should instead adjust the ngx_lua, ngx_devel_kit, and etc modules to accommodate nginx's official Win32 way of doing things? Basically we try hard to minimize OpenResty's own patches for the nginx core unless absolutely necessary. Or is there anything significant I'm overlooking here? |
Yes, it can. Pure nginx (without lua-nginx-module) with OpenSSL, Zlib and PCRE can be built without any problems. Though you won't be able to build x64 version with OpenSSL because nginx configure scripts support only x32 version of OpenSSL for msvc (see %NGINX_ROOT_DIR%/auto/lib/openssl/makefile.msvc, line 9, "perl Configure VC-WIN32 no-shared").
The way I described is actually official one but more detailed and with some minor changes :) Problem 1: use of precompiled header <ngx_config.h>I described the problem in details in my first post.
Problem 2: ngx_lua's config scriptCurrent version of ngx_lua's config script uses That's it. Once those 2 problems are solved ngx_lua can be compiled with stock nginx (x32 builds only). As for my nginx patches I think it's possible to submit them to nginx developers (they don't break anything actually, except for disabling of precompiled headers which can be circumvented through new configure option). |
@geniuss99 Sorry, I didn't know nginx only supports 32-bit OpenSSL on windows. That's indeed unfortunate and I can now see the justification of patching the nginx core. Thank you for your detailed explanation! I had a quick glance at your ngx_lua patch and it looks fine to me, though we'll definitely have a closer look before it can be merged. Regarding to the nginx core patch, will you please create a pull request to the openresty/openresty repository so that it's easier for us to review and merge. OpenResty's own nginx patches all go here: https://github.com/openresty/openresty/tree/master/patches We're very interested in shipping official Win32 and Win64 binaries produced by the MSVC toolchain. We currently only ship a slow MinGW binary build for Wn32. Thank you again for the hard work on this! |
@geniuss99 If I understand it correctly, the other 3rd-party nginx C modules in OpenResty which do not involve feature tests do not require a separate |
echo "using untested compiler to build the module"; | ||
. $ngx_addon_dir/auto/config.gcc; | ||
;; | ||
esac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add back the following commented lines at the end of this file. They are meaningful to OpenResty's build toolchain:
#CFLAGS=$"$CFLAGS -DLUA_DEFAULT_PATH='\"/usr/local/openresty/lualib/?.lua\"'"
#CFLAGS=$"$CFLAGS -DLUA_DEFAULT_CPATH='\"/usr/local/openresty/lualib/?.so\"'"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@geniuss99 Have you tried loading lua-cjson and |
It's not that nginx itself doesn't support x64 OpenSSL - its code base is fully compatible with x64 OpenSSL.
Theoretically, yes. If they don't have other incompatibilities. I haven't checked all of them.
Oh I'm not sure of that :) To tell you the truth I only use Basically to make all OpenResty MSVC-compatible we need to assure the following:
I had a quick glance at OpenResty sources. There are many! modules in the folder "bundles". Some of them are pure @agentzh What modules are of utmost importance to OpenResty application in your opinion? I can examine them and try to compile with MSVC. |
@geniuss99 Thank you for your help here! The other most important ones are ngx_stream_lua and lua-resty-core, and lua-cjson. I guess ngx_stream_lua should be VERY similar to ngx_http_lua (i.e., this repository). It would be great if you can help supporting these on native Windows builds (Win32 & Win64). BTW, ngx_lua will soon rely on lua-resty-core to work properly soon. The existing CFunction-based Lua API is slow and can also be buggy in some corner cases, and should be replaced by the FFI-based implementation in lua-resty-core at all times. |
okay, I'll look into the modules you've mentioned. |
@geniuss99 Great. Thank you very much! |
I've got some news. Many problems are now solved. I took OpenResty's forks of Nginx and LuaJIT. Looks like both are pretty heavily patched compared to original versions (not sure if vanilla LuaJIT might still be used for OpenResty). I've successfully made a custom build of OpenResty (stripped version) using MSVC compiler with the following packets: Nginx 1.13.6 (OpenResty's fork), LuaJIT 2.1.0 (OpenResty's fork), lua-nginx, stream-nginx, lua-cjson. Each of them needs patching for the whole product to compile and work :) Nginx 1.13.6 LuaJIT 2.1.0 lua-nginx stream-nginx lua-cjson lua-resty-core So in the end I've recreated the same files/folders structure as in official OpenResty package compiled using MinGW GCC. Now, the most interesting question is: does it all work?
Yes, loading works. Functions from module work.
Yes, ffi mechanism works.
I've tested some of the functions and they work without problems.
The example from official documentation works as expected. I've used the following config to test the modules:
There are two remaining issues as of now: 1. Support for perl's test scripts in "\t" folder. I tried to port perl's 2. External build script. There needs to be some external build script which automates build steps which I do manually now (there are not so many of them actually). It can be separate script or your big perl script might be updated. I would prefer to write a separate script for MSVC though. @agentzh I've attached an archive with all the patches. Please, examine them and give your opinion. |
@geniuss99 This is great progress! We'll try your build and patches out ourselves as soon as we can manage. Many thanks! |
@geniuss99 Right. |
do you mind of getting this build on app veyor ? |
This pull request is now in conflict :( |
f924579
to
fef2581
Compare
This pull request is now in conflict :( |
This pull request is now in conflict :( |
This pull request is now in conflict :( |
This pull request is now in conflict :( |
This pull request is now in conflict :( |
nginx-patches.zip
I hereby granted the copyright of the changes in this pull request to the authors of this lua-nginx-module project.
These patches will allow lua-nginx-module to be compiled using MSVC compiler. In order to make successful builds nginx files MUST also be patched. Overall there are two patches: one for nginx, another one - for lua-nginx-module (please, see an archive in attachment). The explanations for the patches are given below.
Patch for Nginx.
The following measures are provided:
Disable use of precompiled headers. This is because precompiled header which is produced for <ngx_config.h> file is used for every ".c" file with every "cl.exe" command (due to the nature of Makefile generated by nginx configure scripts). But if we do it like this then (according to MSVC rules) "#include <ngx_config.h>" line MUST be the first line for every ".c" file compiled. This is true for nginx sources but not for nginx-lua-module and ngx_devel_kit-0.3.0. So either the code for these modules should be rewritten or separate Makefile for them must be created. I decided that switching off the use of precompiled headers is the best option for MSVC.
Update "auto/feature" helper script to support MSVC compiler syntax. By default only gcc-style test command is supported. They are not compatible.
Update and create new "auto/lib/openssl/*" scripts to support MSVC x64 compiler. Also for x32 builds NASM is used instead of no-asm variant. It looks like this variant is faster than building without asm.
Please, see the following link: http://openssl.6102.n7.nabble.com/asm-vs-no-asm-performance-test-results-and-security-concern-td36571.html
I suppose patch for nginx (see attach) should go here (it's not for me to decide though):
https://github.com/openresty/openresty/tree/master/patches.
Patch for nginx-lua-module
The following changes are made:
How to compile using MSVC
Prerequisites
To build nginx on the Microsoft Win32 platform you need:
STEP 1
Set MSVC environment (x86 or x64):
"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" x86
or
"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" x64
STEP 2
Add path for MSYS and NASM into %PATH% environment variable:
SET PATH=%PATH%;J:\Tools\compilers\nasm-2.13.02;C:\MinGW\msys\1.0\bin
STEP 3
Start MSYS bash:
bash
STEP 4
Download nginx sources from the hg.nginx.org repository and patch them with nginx-1.13.7.1-lua-nginx-msvc-support.patch:
STEP 5
Create a build and lib directories and unpack zlib, PCRE, OpenSSL libraries sources into lib directory:
STEP 6
Put already compiled LuaJIT library into the lib folder:
tar -xzf ../../luajit-2.0.1-msvc.tar.gz
STEP 7
Create "modules" directory and unpack lua-nginx-module-0.10.12rc1 (patched version) and ngx_devel_kit-0.3.0 modules there:
STEP 8
Define LuaJIT variables used by lua-nginx-module:
STEP 9
Configure nginx:
STEP 10
Compile and build:
nmake
MSVC Run-Time library
By default nginx uses so called multithread, static version of the run-time library (/MT linker switch). I didn't change this so basically LuaJIT should also be compiled in a static version (as a static library).
Another option is to use multithread, DLL-specific version of the run-time library (/MD linker switch).
This way LuaJIT must be compiled as a .dll library. For this
LIBC
variable in "auto/cc/msvc" file must be changed from "-MT" to "-MD" (though i didn't try this variant).I guess dll-specific version of RTL is more preferable for a general use but it's for OpenResty developers to decide. Anyway these patches are a good starting point for MSVC support.
Thank you for attention and sorry if I made a lot of text :)
[Update] For additional info about how MSVC CRT's work please see:
https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx