nic-6443 commented on code in PR #14070:
URL: https://github.com/apache/apisix/pull/14070#discussion_r4356477356
##########
.github/workflows/build.yml:
##########
@@ -183,7 +204,8 @@ jobs:
- name: Linux Script
env:
TEST_FILE_SUB_DIR: ${{ matrix.test_dir }}
- run: sudo -E ./ci/${{ matrix.os_name }}_runner.sh script
+ # sudo-rs (Ubuntu 26.04) ignores -E, so pass what the script needs
explicitly
+ run: sudo -E TEST_FILE_SUB_DIR="$TEST_FILE_SUB_DIR" ./ci/${{
matrix.os_name }}_runner.sh script
Review Comment:
The `VAR=value` form was fine here (the runner's sudoers rule is `ALL`, and
it ran green on both 24.04 and 26.04), but there's no reason to depend on that.
Switched to `sudo -E env TEST_FILE_SUB_DIR=...` in ef5508669, which works the
same under sudo and sudo-rs.
##########
utils/install-dependencies.sh:
##########
@@ -63,25 +63,33 @@ function install_dependencies_with_yum() {
function install_dependencies_with_apt() {
# add OpenResty source
sudo apt-get update
- sudo apt-get -y install software-properties-common wget lsb-release gnupg
patch
- wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y install wget lsb-release gnupg patch
+ sudo mkdir -p /etc/apt/keyrings
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo gpg --dearmor
--yes -o /etc/apt/keyrings/openresty.gpg
Review Comment:
Fair point, with a restrictive umask `_apt` wouldn't be able to read it.
Added an explicit `chmod 0644` right after writing the keyring in ef5508669.
##########
utils/install-dependencies.sh:
##########
@@ -63,25 +63,33 @@ function install_dependencies_with_yum() {
function install_dependencies_with_apt() {
# add OpenResty source
sudo apt-get update
- sudo apt-get -y install software-properties-common wget lsb-release gnupg
patch
- wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y install wget lsb-release gnupg patch
+ sudo mkdir -p /etc/apt/keyrings
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo gpg --dearmor
--yes -o /etc/apt/keyrings/openresty.gpg
arch=$(uname -m | tr '[:upper:]' '[:lower:]')
arch_path=""
if [[ $arch == "arm64" ]] || [[ $arch == "aarch64" ]]; then
arch_path="arm64/"
fi
- if [[ "${1}" == "ubuntu" ]]; then
- sudo add-apt-repository -y "deb
http://openresty.org/package/${arch_path}ubuntu $(lsb_release -sc) main"
- elif [[ "${1}" == "debian" ]]; then
- # add-apt-repository on Debian 12 writes an empty list file for a plain
- # deb line, so the repository never makes it into apt
- echo "deb http://openresty.org/package/${arch_path}debian
$(lsb_release -sc) openresty" \
- | sudo tee /etc/apt/sources.list.d/openresty.list
+ codename=$(lsb_release -sc)
+ repo_url="http://openresty.org/package/${arch_path}${1}";
Review Comment:
Agreed. openresty.org serves the repositories over https (checked noble,
arm64/noble and bookworm), so `repo_url` is https now in ef5508669. That covers
both the `--spider` check and the apt source line.
##########
utils/install-dependencies.sh:
##########
@@ -63,25 +63,33 @@ function install_dependencies_with_yum() {
function install_dependencies_with_apt() {
# add OpenResty source
sudo apt-get update
- sudo apt-get -y install software-properties-common wget lsb-release gnupg
patch
- wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y install wget lsb-release gnupg patch
+ sudo mkdir -p /etc/apt/keyrings
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo gpg --dearmor
--yes -o /etc/apt/keyrings/openresty.gpg
arch=$(uname -m | tr '[:upper:]' '[:lower:]')
arch_path=""
if [[ $arch == "arm64" ]] || [[ $arch == "aarch64" ]]; then
arch_path="arm64/"
fi
- if [[ "${1}" == "ubuntu" ]]; then
- sudo add-apt-repository -y "deb
http://openresty.org/package/${arch_path}ubuntu $(lsb_release -sc) main"
- elif [[ "${1}" == "debian" ]]; then
- # add-apt-repository on Debian 12 writes an empty list file for a plain
- # deb line, so the repository never makes it into apt
- echo "deb http://openresty.org/package/${arch_path}debian
$(lsb_release -sc) openresty" \
- | sudo tee /etc/apt/sources.list.d/openresty.list
+ codename=$(lsb_release -sc)
+ repo_url="http://openresty.org/package/${arch_path}${1}";
+ # OpenResty publishes packages for a new Ubuntu release some time after it
+ # comes out (e.g. 26.04). Only openresty-zlib-dev and openresty-pcre-dev
are
+ # needed from it, and they are self-contained under /usr/local/openresty,
+ # so fall back to the latest LTS repository it has.
+ if [[ "${1}" == "ubuntu" ]] && ! wget -q --spider
"${repo_url}/dists/${codename}/Release"; then
+ codename=noble
fi
+ component=main
+ if [[ "${1}" == "debian" ]]; then
+ component=openresty
+ fi
+ echo "deb [signed-by=/etc/apt/keyrings/openresty.gpg] ${repo_url}
${codename} ${component}" \
Review Comment:
Same fix as the thread above: `repo_url` is https now (ef5508669), and this
line is built from it.
##########
utils/linux-install-luarocks.sh:
##########
@@ -72,3 +72,9 @@ luarocks config variables.OPENSSL_DIR ${OPENSSL_PREFIX}
luarocks config variables.OPENSSL_LIBDIR ${OPENSSL_PREFIX}/lib
luarocks config variables.OPENSSL_INCDIR ${OPENSSL_PREFIX}/include
luarocks config variables.YAML_DIR /usr
+
+# lrexlib-pcre needs PCRE1, which newer distributions (Ubuntu 26.04, Debian 13)
+# no longer ship; use the copy bundled with APISIX runtime instead.
+if [ -f ${OPENRESTY_PREFIX}/pcre/include/pcre.h ]; then
+ luarocks config variables.PCRE_DIR ${OPENRESTY_PREFIX}/pcre
+fi
Review Comment:
Quoted both uses in the lines this PR adds (ef5508669). The rest of the
script was already unquoted before this change, so I left it alone to keep the
diff focused.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
