Skip to content
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

Fix installing libssl on old Alpine versions #902

Merged
merged 3 commits into from
Apr 3, 2024

Conversation

transistive
Copy link
Contributor

@transistive transistive commented Apr 3, 2024

When trying to use the script in PHP 5.6 FPM Alpine, it broke trying to install ^libssl[0-9]+(\.[0-9]+)*$

After closer inspection, it seems that the pattern:

#.....
if test -z "$(apk info 2>/dev/null | grep -E ^libssl)"; then
	buildRequiredPackageLists_libssl='^libssl[0-9]+(\.[0-9]+)*$'
elif
#.......

is not portable. It crashes in older PHP fpm scripts. I assume that older alpine versions don't support regex pattern installations.

This solution leverages apk search and grep -E to achieve the same result.

@mlocati
Copy link
Owner

mlocati commented Apr 3, 2024

Which PHP extension are you trying to install on PHP 5.6 fpm that showcases the problem?

@transistive
Copy link
Contributor Author

I haven't taken the time to isolate the package as I was under the impressions this was a common installation requirement. Here is the list:

install-php-extensions gd \
  apcu \
  amqp \
  memcache \
  sodium \
  redis \
  xdebug \
  mcrypt \
  gmp \
  exif \
  sockets \
  bcmath \
  zip \
  tidy \
  soap \
  mysqli \
  simplexml \
  intl \
  opcache \
  pdo_mysql \
  mongodb \
  @composer

I can narrow it down if you want later, I'm grabbing lunch now

@mlocati
Copy link
Owner

mlocati commented Apr 3, 2024

Nowaways we don't have a libssl-dev package, but one day we may have it.
So, I'd prefer this regex:
^libssl[0-9]+[r0-9.\-]*$

Also, everywhere we simply specify either the complete package name or a regular expression that identifies it: why are you using the grep | cut | uniq approach instead?

@transistive
Copy link
Contributor Author

The problem is when you run :

docker run -it php:5.6-fpm-alpine apk add ^libssl[0-9]+[r0-9.\-]*$ 

it crashes, which gets called indirectly if you run:

docker run php:5.6-fpm-alpine sh -c "
curl -sSLf \
      -o /usr/local/bin/install-php-extensions \
      https://github.com/mlocati/docker-php-extension-installer/releases/latest/download/install-php-extensions
chmod +x /usr/local/bin/install-php-extensions
install-php-extensions mongodb
"

The reason I'm using the funky grep | cut | uniq approach is because if we just use grep it will output something like this:

docker run -it php:5.6-fpm-alpine sh -c 'apk update && apk search | grep -E "^libssl"'

it outputs something like:

libssl1.0-1.0.2u-r0

Which you cannot directly install:

docker run -it php:5.6-fpm-alpine sh -c 'apk add libssl1.0-1.0.2u-r0'

Hence, the cut to only fetch the first part, libssl1.0, which does install:

docker run php:5.6-fpm-alpine sh -c 'apk add libssl1.0'

The uniq part is just me being a bit of a perfectionist but is not required

@mlocati
Copy link
Owner

mlocati commented Apr 3, 2024

You are right, on Alpine we can't use regular expressions to choose apk libraries (I forgot about that).

So, what about

buildRequiredPackageLists_libssl="$(apk search | grep -E '^libssl[0-9]' | cut -d- -f1)"

@transistive
Copy link
Contributor Author

If that works I'm entirely okay with that! I'm currently not in a place where I can double-check. Good stuff 👍

@mlocati
Copy link
Owner

mlocati commented Apr 3, 2024

Even better:

apk search | grep -E '^libssl[0-9]' | head -1 | cut -d- -f1

@transistive
Copy link
Contributor Author

Perfect! Feel free to change it, otherwise I'll do it tomorrow

Test: event, imap, mongo, mongodb, openswoole, relay, stomp, swoole
@mlocati
Copy link
Owner

mlocati commented Apr 3, 2024

Feel free to change it

Done: 3ce069c

@mlocati mlocati changed the title Only install libssl if it exists in apk info Fix installing libssl on old Alpine versions Apr 3, 2024
@mlocati mlocati merged commit 749afd0 into mlocati:master Apr 3, 2024
40 checks passed
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.

None yet

2 participants