Code review comment for ~athos-ribeiro/ubuntu/+source/apache2:postinst-triggered-mm

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

On Wednesday, March 20 2024, Athos Ribeiro wrote:

> It would be worth discussing this with with the SRU team (after an upload?). I'd be OK with waitin ga bit longer to see if we get any feedback from Debian on this.

ACK.

>> I'm specifically worried about the "behaviour change" side of this fix.
>
> AFAICT, PHP should be the only package affected. If the concern is
> about "we are adding a feature to a stable release", I do believe that
> on the PHP end this should be considered a bug (which has been there
> for a long while) instead. However, this may indeed be seen as a
> feature in the apache2 side.

I understand where you're coming from. This is a bug on apache2 because
PHP expects apache2 to react to triggers. But it can also be said that
maybe apache2 *shouldn't* react to those triggers, because it doesn't
want to be restarted at random times.

> That said, an alternative approach could be:
>
> - keep the noble change, adding support for triggers in apache2.
> - change the php's libapache2-mod-php postinst script to reload or restart apache2 without the apache2-maintscript-helper.

You mean that you could implement either one or the other, right?

Honestly, they're equivalent and don't change the underlying problem of
restarting apache2 "behind the scenes" after installing a PHP module
(but maybe I'm misunderstanding what you're saying).

> Still, given that (most likely) the only known behavioral change is in php, the approach presented here could still be acceptable, although the alternative presented above looks safer from a SRU perspective.

Let me backtrack a little bit and ask you this: if you only reload
apache2 (instead of restarting it), does it work the same? Because if
it does, then that would be less intrusive indeed.

--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14

« Back to merge proposal