(MODULES-10899) Handle PHP8 MOD package naming convention changes #2121

sanfrancrisko · 2021-02-08T10:16:50Z

This PR incorporates the changes from #2119 and #2124 along with a few other additions:

Change range of versioncmp to < 8 to reduce churn in previous conventions for 7.x PHP versions
Enable and update acceptance test expectations for PHP mod packages on SLES 15. In hindsight, I'm not sure this was required after all, but I wanted to catch any potential issues around the changes being performed given SLES is a bit special with it's MOD PHP package names.
Update test expectations for OSs using PHP >= 7 based on the new naming conventions in

puppet-community-rangefinder · 2021-02-08T10:16:58Z

apache::mod::php is a class

Breaking changes to this file WILL impact these 49 modules (exact match):

Breaking changes to this file MAY impact these 36 modules (near match):

This module is declared in 175 of 576 indexed public `Puppetfiles`.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

codecov-io · 2021-02-08T10:55:45Z

Codecov Report

Merging #2121 (16a4d4b) into main (13f55bd) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2121   +/-   ##
=======================================
  Coverage   56.36%   56.36%           
=======================================
  Files          12       12           
  Lines         220      220           
=======================================
  Hits          124      124           
  Misses         96       96

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1a830b...16a4d4b. Read the comment docs.

gguillotte

@sanfrancrisko LGTM

beatchristen · 2021-02-12T17:00:57Z

I've tried out this branch with debian10 and libapache2-mod-php8.0 and the module name in /etc/apache2/mods-enabled/php.load now points at nothing:

Syntax error on line 1 of /etc/apache2/mods-enabled/php.load: Cannot load /usr/lib/apache2/modules/libphp.so into server: /usr/lib/apache2/modules/libphp.so: cannot open shared object file: No such file or directory

overriding the $path like below fixes it ($php_version = '8.0':

  class { 'apache::mod::php':
    path        => "/usr/lib/apache2/modules/libphp${webserver::php_version}.so",
    php_version => $webserver::php_version,
    require     => Package["libapache2-mod-php${webserver::php_version}"],
  }

gguillotte · 2021-02-12T19:49:24Z

Worth looking into. Notably for reproduction purposes, those php8 packages are only in sid/unstable at the moment.

gguillotte · 2021-02-12T19:56:33Z

Re: https://php.watch/versions/8.0/mod_php-rename, the version number is removed from php_module but not from the libphp filename itself:

- LoadModule php7_module "/usr/lib/apache2/modules/libphp7.4.so"
+ LoadModule php_module "/usr/lib/apache2/modules/libphp8.0.so"

So for:

puppetlabs-apache/manifests/mod/php.pp

Lines 73 to 97 in 721878a

    
           if $::operatingsystem == 'SLES' { 
        
             ::apache::mod { $mod: 
        
               package        => $_package_name, 
        
               package_ensure => $package_ensure, 
        
               lib            => "mod_${mod}.so", 
        
               id             => "php${_php_major}_module", 
        
               path           => "${apache::lib_path}/mod_${mod}.so", 
        
             } 
        
           } elsif (versioncmp($php_version, '8') < 0) { 
        
             ::apache::mod { $mod: 
        
               package        => $_package_name, 
        
               package_ensure => $package_ensure, 
        
               lib            => $_lib, 
        
               id             => "php${_php_major}_module", 
        
               path           => $path, 
        
             } 
        
           } else { 
        
             ::apache::mod { $mod: 
        
                 package        => $_package_name, 
        
                 package_ensure => $package_ensure, 
        
                 lib            => "${libphp_prefix}.so", 
        
                 id             => 'php_module', 
        
                 path           => $path, 
        
             } 
        
           }

L93 may need to use

       lib            => $_lib,

instead of

       lib            => "${libphp_prefix}.so",

sanfrancrisko · 2021-02-15T10:56:15Z

Thanks for highlighting @beatchristen and the feedback and suggestions @gguillotte .

@beatchristen I did a quick test with the module, as it is right now and it appears as though the same problem exists - can you confirm?

@gguillotte L93, that you highlighted, should only be hit in cases where $php_version < 8, which is the legacy behaviour. I'd be concerned that changing that would have knock on effects to existing versions.

For reference, this was the result when using a2enmod, with sid (unstable) at the moment:

/etc/apache2/mods-enabled/php8.0.load:

# Conflicts: php5
# Depends: mpm_prefork
LoadModule php_module /usr/lib/apache2/modules/libphp8.0.so

I'm not sure what Debian are going to do when PHP 8 moves from unstable -> stable in terms of package naming convention.

The original issue reports that this was affecting CentOS 7 and Debian 10 - I was going to suggest we raise this as a separate issue to fix on Debian 10 (using Sid) but if the customer in the original issue is using Debian 10 perhaps we need to address it now @gguillotte ?

beatchristen · 2021-02-15T14:45:04Z

@sanfrancrisko not sure what problem you specifically encountered. IMHO, the module up until 7.x works as expected on Debian10.

I've added two tests and fixes for the problems reported on Debian 10 here:
#2124

I'm not sure what Debian are going to do when PHP 8 moves from unstable -> stable in terms of package naming convention.

There's probably no reason for Debian 11 to change anything about the structure within the file, as it was initiated further upstream with the PHP team itself.
So when SLES get's a PHP 8.0 version, it should work once it's added to apache::version.

sanfrancrisko · 2021-02-16T12:49:49Z

@beatchristen I was referring to the issue of trying to install PHP 8.0 using the sid repo on Debian 10 affecting the module as-is on the Forge right now (i.e. without this change merged).

There's a bit of time pressure on this fix for MODULES-10899 so I was going to get PR merged and then we could continue investigating the issues with PHP 8.0 on Debian 10 on a separate PR.

And yes, to clarify, I can also confirm the module is working fine up to PHP 7.x with Debian 10 👍

Thanks for putting up #2124 - looks good but just had one comment. I'll likely try to land this PR today, as mentioned, to address MODULES-10899 - as far as I can see it will not cause any breaking changes for the module so far, but by all means scream if I'm missing something 😄

PHP8 dropped the conventions of including the major versions in apache modules. This fix makes it so that moving forward the module will no longer add the major in the relevant places. before change: /etc/httpd/conf.modules.d/php8.conf /etc/httpd/conf.modules.d/php8.load ^---Content---^ LoadModule php8_module modules/libphp8.so Apache won't load After change: /etc/httpd/conf.modules.d/php.conf /etc/httpd/conf.modules.d/php.load ^---Content---^ LoadModule php_module modules/libphp.so

Also remove restriction on tests so they execute on SLES platforms

On SLES >= 15 it appears as though the `apache2-mod_php5` package is no longer available in the default repo. This commit ensures that the default value for the $php_version parameter is 7 on SLES >= 15 and the appropriate `apache2-mod_php7` package is used. The spec test expectations have also been updated based on the changes.

This commit introduces the `$_module_id` variable which will be populated with the correct module ID based on the version specified. This makes things a bit more explicit and easier to follow. Also, two spec tests added for the following scenario: - Debian 10 (out of the box) using PHP 7.3 - Debian 10 (experimental) using PHP 8.0

sanfrancrisko requested a review from a team as a code owner February 8, 2021 10:16

sanfrancrisko marked this pull request as draft February 8, 2021 14:53

sanfrancrisko force-pushed the MODULES-10899/main/apache_php8_mod_name branch 3 times, most recently from 55a3e5f to ca2cd64 Compare February 8, 2021 17:53

sanfrancrisko added the bugfix label Feb 8, 2021

sanfrancrisko force-pushed the MODULES-10899/main/apache_php8_mod_name branch from ca2cd64 to a7b20b9 Compare February 8, 2021 18:40

sanfrancrisko changed the title ~~(MODULES-10899) Handle differences in PHP MOD name for PHP 7/8~~ (MODULES-10899) Handle differences in PHP MOD name for PHP 8 / <8 Feb 8, 2021

sanfrancrisko force-pushed the MODULES-10899/main/apache_php8_mod_name branch 2 times, most recently from 4f50735 to 721878a Compare February 10, 2021 17:28

sanfrancrisko marked this pull request as ready for review February 10, 2021 17:34

sanfrancrisko requested a review from gguillotte February 10, 2021 17:34

sanfrancrisko mentioned this pull request Feb 10, 2021

Fix issues when using php8 #2119

Closed

sanfrancrisko changed the title ~~(MODULES-10899) Handle differences in PHP MOD name for PHP 8 / <8~~ (MODULES-10899) Handle PHP8 MOD package naming convention changes Feb 10, 2021

gguillotte approved these changes Feb 11, 2021

View reviewed changes

sanfrancrisko mentioned this pull request Feb 16, 2021

(MODULES-10899) Clean up PHP mod id handling in apache::mod::php #2124

Closed

cdamage and others added 5 commits February 16, 2021 15:56

(MODULES_10899) Use versioncmp function for $php_version

8dee6e1

Also remove restriction on tests so they execute on SLES platforms

(MODULES-10899) Fix double quoted string issues

0d175da

sanfrancrisko force-pushed the MODULES-10899/main/apache_php8_mod_name branch from 721878a to bc3f35c Compare February 16, 2021 16:01

pmcmaw merged commit 4bb4078 into puppetlabs:main Feb 16, 2021

sanfrancrisko deleted the MODULES-10899/main/apache_php8_mod_name branch February 16, 2021 21:03

xchotard-talentsoft mentioned this pull request Aug 4, 2022

PHP module not working on Ubuntu 22.04 / Jammy #2259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

(MODULES-10899) Handle PHP8 MOD package naming convention changes #2121

(MODULES-10899) Handle PHP8 MOD package naming convention changes #2121

Uh oh!

sanfrancrisko commented Feb 8, 2021 •

edited

Loading

Uh oh!

puppet-community-rangefinder bot commented Feb 8, 2021

Uh oh!

codecov-io commented Feb 8, 2021

Uh oh!

gguillotte left a comment

Uh oh!

beatchristen commented Feb 12, 2021

Uh oh!

gguillotte commented Feb 12, 2021

Uh oh!

gguillotte commented Feb 12, 2021

Uh oh!

sanfrancrisko commented Feb 15, 2021

Uh oh!

beatchristen commented Feb 15, 2021

Uh oh!

sanfrancrisko commented Feb 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

(MODULES-10899) Handle PHP8 MOD package naming convention changes #2121

(MODULES-10899) Handle PHP8 MOD package naming convention changes #2121

Uh oh!

Conversation

sanfrancrisko commented Feb 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

puppet-community-rangefinder bot commented Feb 8, 2021

apache::mod::php is a class

This module is declared in 175 of 576 indexed public Puppetfiles.

Uh oh!

codecov-io commented Feb 8, 2021

Codecov Report

Uh oh!

gguillotte left a comment

Choose a reason for hiding this comment

Uh oh!

beatchristen commented Feb 12, 2021

Uh oh!

gguillotte commented Feb 12, 2021

Uh oh!

gguillotte commented Feb 12, 2021

Uh oh!

sanfrancrisko commented Feb 15, 2021

Uh oh!

beatchristen commented Feb 15, 2021

Uh oh!

sanfrancrisko commented Feb 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sanfrancrisko commented Feb 8, 2021 •

edited

Loading

This module is declared in 175 of 576 indexed public `Puppetfiles`.