You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/02/19 03:48:12 UTC
[GitHub] [apisix] tokers opened a new pull request #3603: fix: use openssl111 in openresty dir in precedence
tokers opened a new pull request #3603:
URL: https://github.com/apache/apisix/pull/3603
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
We use luasec since 5399a31b, however, it uses the openssl libs and headers to compile itself, currently, we set two variables `$OPENSSL_LIBDIR` (e.g. `/usr/local/openresty/openssl/lib`) and `$OPENSSL_INCDIR` (e.g. `/usr/local/openresty/openssl/include`).
However, OpenResty 1.17.8 or higher version actually uses `openssl111` as the dir name of openssl. So in this PR, we detect the existence of dir `openssl111` and use it in precedence.
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#discussion_r578921057
##########
File path: doc/install-dependencies.md
##########
@@ -39,6 +39,8 @@
- On some platforms, installing LuaRocks via the package manager will cause Lua to be upgraded to Lua 5.3, so we recommend installing LuaRocks via source code. if you install OpenResty and its OpenSSL develop library (openresty-openssl-devel for rpm and openresty-openssl-dev for deb) via the official repository, then [we provide a script for automatic installation](../utils/linux-install-luarocks.sh). If you compile OpenResty yourself, you can refer to the above script and change the path in it. If you don't specify the OpenSSL library path when you compile, you don't need to configure the OpenSSL variables in LuaRocks, because the system's OpenSSL is used by default. If the OpenSSL library is specified at compile time, then you need to ensure that LuaRocks' OpenSSL configuration is consistent with OpenResty's.
+- If you are using OpenResty/1.17.8 or higher version, please installing openresty-openssl111-devel instead of openresty-openssl-devel.
Review comment:
We should warn for low version instead.
And also update the CI scripts.
And also update the Chinese doc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#issuecomment-782628589
> The change belongs to the dependencies installation, which is covered in the install-dependencies.md, Is there anything should also be change there?
all fine now. thanks for your explaining
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#discussion_r579111272
##########
File path: doc/install-dependencies.md
##########
@@ -39,6 +39,8 @@
- On some platforms, installing LuaRocks via the package manager will cause Lua to be upgraded to Lua 5.3, so we recommend installing LuaRocks via source code. if you install OpenResty and its OpenSSL develop library (openresty-openssl-devel for rpm and openresty-openssl-dev for deb) via the official repository, then [we provide a script for automatic installation](../utils/linux-install-luarocks.sh). If you compile OpenResty yourself, you can refer to the above script and change the path in it. If you don't specify the OpenSSL library path when you compile, you don't need to configure the OpenSSL variables in LuaRocks, because the system's OpenSSL is used by default. If the OpenSSL library is specified at compile time, then you need to ensure that LuaRocks' OpenSSL configuration is consistent with OpenResty's.
+- If you are using OpenResty/1.17.8 or higher version, please installing openresty-openssl111-devel instead of openresty-openssl-devel.
Review comment:
Yes, but we have a transition time, for now, we still need this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#issuecomment-781899370
> more docs need to be updated:
>
> https://github.com/apache/apisix/blob/master/doc/install-dependencies.md#centos-7
>
> 
Do you view the file on my branch? It was updated already.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#issuecomment-781864854
more docs needs to be updated:
https://github.com/apache/apisix/blob/master/doc/install-dependencies.md#centos-7
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#discussion_r579599867
##########
File path: utils/linux-install-openresty.sh
##########
@@ -80,4 +81,23 @@ else
openresty="openresty-debug=$OPENRESTY_VERSION*"
fi
-sudo apt-get install "$openresty" lua5.1 liblua5.1-0-dev openresty-openssl-debug-dev
+verlte() {
+ [ "$1" = "`echo -e "$1\n$2" | sort -V | head -n1`" ]
+}
+
+verlt() {
+ [ "$1" = "$2" ] && return 1 || verlte $1 $2
+}
+
+sudo apt-get install "$openresty" lua5.1 liblua5.1-0-dev
+
+if [ "$OPENRESTY_VERSION" == "default" ]; then
+ sudo apt-get install openresty-openssl111-debug-dev
+else
+ verlt $OPENRESTY_VERSION 1.17.8.1
Review comment:
Fixed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis edited a comment on pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
membphis edited a comment on pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#issuecomment-781864854
more docs need to be updated:
https://github.com/apache/apisix/blob/master/doc/install-dependencies.md#centos-7
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis merged pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #3603:
URL: https://github.com/apache/apisix/pull/3603
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#discussion_r578955556
##########
File path: doc/install-dependencies.md
##########
@@ -39,6 +39,8 @@
- On some platforms, installing LuaRocks via the package manager will cause Lua to be upgraded to Lua 5.3, so we recommend installing LuaRocks via source code. if you install OpenResty and its OpenSSL develop library (openresty-openssl-devel for rpm and openresty-openssl-dev for deb) via the official repository, then [we provide a script for automatic installation](../utils/linux-install-luarocks.sh). If you compile OpenResty yourself, you can refer to the above script and change the path in it. If you don't specify the OpenSSL library path when you compile, you don't need to configure the OpenSSL variables in LuaRocks, because the system's OpenSSL is used by default. If the OpenSSL library is specified at compile time, then you need to ensure that LuaRocks' OpenSSL configuration is consistent with OpenResty's.
+- If you are using OpenResty/1.17.8 or higher version, please installing openresty-openssl111-devel instead of openresty-openssl-devel.
Review comment:
We should install openresty-openssl111-devel by default and install openresty-openssl-devel for OpenResty 1.15.8.
And we need to update the Chinese docs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#discussion_r578921057
##########
File path: doc/install-dependencies.md
##########
@@ -39,6 +39,8 @@
- On some platforms, installing LuaRocks via the package manager will cause Lua to be upgraded to Lua 5.3, so we recommend installing LuaRocks via source code. if you install OpenResty and its OpenSSL develop library (openresty-openssl-devel for rpm and openresty-openssl-dev for deb) via the official repository, then [we provide a script for automatic installation](../utils/linux-install-luarocks.sh). If you compile OpenResty yourself, you can refer to the above script and change the path in it. If you don't specify the OpenSSL library path when you compile, you don't need to configure the OpenSSL variables in LuaRocks, because the system's OpenSSL is used by default. If the OpenSSL library is specified at compile time, then you need to ensure that LuaRocks' OpenSSL configuration is consistent with OpenResty's.
+- If you are using OpenResty/1.17.8 or higher version, please installing openresty-openssl111-devel instead of openresty-openssl-devel.
Review comment:
We should warn for low version instead.
And also update the CI scripts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#discussion_r579591196
##########
File path: utils/linux-install-openresty.sh
##########
@@ -80,4 +81,23 @@ else
openresty="openresty-debug=$OPENRESTY_VERSION*"
fi
-sudo apt-get install "$openresty" lua5.1 liblua5.1-0-dev openresty-openssl-debug-dev
+verlte() {
+ [ "$1" = "`echo -e "$1\n$2" | sort -V | head -n1`" ]
+}
+
+verlt() {
+ [ "$1" = "$2" ] && return 1 || verlte $1 $2
+}
+
+sudo apt-get install "$openresty" lua5.1 liblua5.1-0-dev
+
+if [ "$OPENRESTY_VERSION" == "default" ]; then
+ sudo apt-get install openresty-openssl111-debug-dev
+else
+ verlt $OPENRESTY_VERSION 1.17.8.1
Review comment:
We can simply check if the version is `1.15.8.2` so there is no need to comment out `set -euo pipefail`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#discussion_r578957370
##########
File path: doc/install-dependencies.md
##########
@@ -39,6 +39,8 @@
- On some platforms, installing LuaRocks via the package manager will cause Lua to be upgraded to Lua 5.3, so we recommend installing LuaRocks via source code. if you install OpenResty and its OpenSSL develop library (openresty-openssl-devel for rpm and openresty-openssl-dev for deb) via the official repository, then [we provide a script for automatic installation](../utils/linux-install-luarocks.sh). If you compile OpenResty yourself, you can refer to the above script and change the path in it. If you don't specify the OpenSSL library path when you compile, you don't need to configure the OpenSSL variables in LuaRocks, because the system's OpenSSL is used by default. If the OpenSSL library is specified at compile time, then you need to ensure that LuaRocks' OpenSSL configuration is consistent with OpenResty's.
+- If you are using OpenResty/1.17.8 or higher version, please installing openresty-openssl111-devel instead of openresty-openssl-devel.
Review comment:
if we only support `> openresty 1.17.8.1`, can it be easier?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#discussion_r579650144
##########
File path: utils/linux-install-openresty.sh
##########
@@ -80,4 +80,10 @@ else
openresty="openresty-debug=$OPENRESTY_VERSION*"
fi
-sudo apt-get install "$openresty" lua5.1 liblua5.1-0-dev openresty-openssl-debug-dev
+sudo apt-get install "$openresty" lua5.1 liblua5.1-0-dev
+
+if [ "$OPENRESTY_VERSION" == "1.15.8.2" ]; then
Review comment:
We will not support version 1.15.8.2 soon.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#issuecomment-781949152
> Do you view the file on my branch? It was updated already.
https://github.com/apache/apisix/blob/22827880f46e50cc657371ed36a5ae76f412fa57/README.md#configure-and-installation
We need to update `README.md`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#discussion_r578948584
##########
File path: doc/install-dependencies.md
##########
@@ -39,6 +39,8 @@
- On some platforms, installing LuaRocks via the package manager will cause Lua to be upgraded to Lua 5.3, so we recommend installing LuaRocks via source code. if you install OpenResty and its OpenSSL develop library (openresty-openssl-devel for rpm and openresty-openssl-dev for deb) via the official repository, then [we provide a script for automatic installation](../utils/linux-install-luarocks.sh). If you compile OpenResty yourself, you can refer to the above script and change the path in it. If you don't specify the OpenSSL library path when you compile, you don't need to configure the OpenSSL variables in LuaRocks, because the system's OpenSSL is used by default. If the OpenSSL library is specified at compile time, then you need to ensure that LuaRocks' OpenSSL configuration is consistent with OpenResty's.
+- If you are using OpenResty/1.17.8 or higher version, please installing openresty-openssl111-devel instead of openresty-openssl-devel.
Review comment:
You mean the low version of OpenResty?
Also, Do we still need to update the Chinese?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #3603: fix: use openssl111 in openresty dir in precedence
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #3603:
URL: https://github.com/apache/apisix/pull/3603#issuecomment-782013347
> > Do you view the file on my branch? It was updated already.
>
> https://github.com/apache/apisix/blob/22827880f46e50cc657371ed36a5ae76f412fa57/README.md#configure-and-installation
>
> We need to update `README.md`.
>
> 
The change belongs to the dependencies installation, which is covered in the install-dependencies.md, Is there anything should also be change there?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org