Sitelet https://github.com/openresty/openresty/pull/329
Skip to content

feature: added support for compilation of lua-nginx-module with nginx under MSVC - #329

Open
geniuss99 wants to merge 4 commits into
openresty:masterfrom
geniuss99:master
Open

geniuss99 wants to merge 4 commits into
openresty:masterfrom
geniuss99:master

Conversation

@geniuss99

Copy link
Copy Markdown

I hereby granted the copyright of the changes in this pull request to the authors of this openresty project.

This pull request is related to lua-nginx-module feature:
openresty/lua-nginx-module#1222

+ --openssldir="%cd%/openssl/ssl" \
+ $(OPENSSL_OPT)
+
+ if exist ms\do_win64a.bat ( \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing \ is not aligned up vertically.


$OPENSSL/openssl/include/openssl/ssl.h: $NGX_MAKEFILE
- \$(MAKE) -f auto/lib/openssl/makefile.msvc \
+ \$(MAKE) -f auto/lib/openssl/makefile-$NGX_MSVC_TYPE.msvc \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing \ is not aligned up vertically.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot to mention there are also tabs instead of spaces :)
Actually those \ symbols are not aligned in the original file "makefile.msvc" used as a base.

I've aligned them.

+ --openssldir="%cd%/openssl/ssl" \
+ $(OPENSSL_OPT)
+
+ if exist ms\do_nasm.bat ( \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@agentzh

agentzh commented Jan 10, 2018

Copy link
Copy Markdown
Member

@geniuss99 BTW, will you rename your patch to 1.13.6? We have no patches for 1.13.7 and also no plan to use that nginx core (we may directly jump to nginx 1.13.8 or even beyond in the future).

@geniuss99

Copy link
Copy Markdown
Author

Done.

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.

2 participants