Revert using hardcoded php-intl meta package

[MichaIng]

By submitting this pull request, I confirm the following:

• I have read and understood the contributors guide, as well as this entire template.
• I have made only one major change in my proposed changes.
• I have commented my proposed changes within the code.
• I have tested my proposed changes, and have included unit tests where possible.
• I am willing to help maintain this change if there are issues with it later.
• I give this submission freely and claim no ownership.
• It is compatible with the EUPL 1.2 license
• I have squashed any insignificant commits. (git rebase)

Signed-off-by: MichaIng micha@dietpi.com

---

What does this PR aim to accomplish?:
Using the meta package causes several issues:

• Install on Debian prior to Stretch and Ubuntu prior to Xenial is broken, since those do not serve the meta packages but php5-* packages instead.
• If $phpVer != "php", then multiple conflicting PHP versions can be installed.
• If "${phpVer}-intl" does not pull the correct package, then inherently "${phpVer}-xml" etc are wrong, too. This is theoretically possible, e.g. if PHP7.4 was installed while the webserver uses a concurrently installed PHP7.3 instance. Then the "php" shell command output can differ from what the webserver uses. This theoretical issue would need a different approach to derive $phpVer, not based on the shell command output but by asking the webserver somehow in the first place. But using $phpVer for some modules and hardcoded meta for the others can only lead to inconsistencies and issues.

How does this PR accomplish the above?:

• Revert to $phpVer for PHP intl module

[dschaper]

Debian prior to Jesse is not supported.

We tried using this way and it didn't fix anything. See the notes for the PR that caused this change.

[MichaIng]

@dschaper
Please read the new posts in the originating issue: pi-hole/web#1148 (comment)
Using phpVar was never the reason for missing intl module (on DietPi webserver install is skipped, hence PHP module installs as well, PHP intl package has been added for DietPi v6.29), and when looking into the package details it simply cannot be. But please read through the possible serious issues that hardcoded meta package bears.

Debian prior to Jesse is not supported.

Whoops, I means Debian prior to Stretch, hence including Jessie. Otherwise the whole php5 package check could be removed 😉.

[dschaper]

https://docs.pi-hole.net/main/prerequesites/

[dschaper]

And yes, I've read the issues. Our fix works everywhere but, apparently, DietPi? And DietPi uses some "magic" to import versions of PHP that are not native to the underlying distribution. Thus I feel the problem is with trying to force those PHP packages in from a version that they are not meant to run on.

[MichaIng]

@dschaper
Ah okay, no Jessie support officially, then we can remove the PHP5 package checks. However this does not mean that the meta packages can always be used. If there is already any PHP version installed, the matching module packages must be pulled, else havok 😄.

[MichaIng]

@dschaper

Thus I feel the problem is with trying to force those PHP packages in from a version that they are not meant to run on.

Since PHP7.0 lost support by a bunch of web applications (ownCloud, Nextcloud just to mention two), it is a pretty well known solution to use Ondrejs (official Debian PHP maintainer) PHP repo https://deb.sury.org/ to install newer PHP versions on Debian Stretch. And there are several other reasons why one might find multiple PHP versions available in the repos. Blindly taking the meta package that is pulled in by current policies, when there is a different PHP version installed already, will break things for many users.

And you already HAVE the code there to check for matching PHP packages. Even when it would be the decision to only allow standard Debian repo and then use meta packages, then it is inconsistent to use those for intl module only while keeping the phpVar in place for all other modules. Either hard-code it for all of them or for none, else users will run into multiple PHP instances.

And as for DietPi: This has no influence at all on DietPi since we skip the webserver install together with the PHP modules, but install them prior to Pi-hole installer.

Our fix works everywhere

This was never a fix, it simply had no effect for most users as the exact same PHP intl package is installed. And where it has an effect, it is a negative one. The whole assumption for the hardcode was based on one report where DietPi was used which skips the module installs all together, regardless of phpVar or meta package being used 😉.

[dschaper]

pi-hole/web#1148 (comment)

If the contributor that provided the instruction would like to respond.

[dschaper]

On the side, is there any way you can identify DietPi so that we can easily see during a debug run that your OS is being used? I think os-release just shows the underlying distribution. It would save a lot of time for our support volunteers to be able to find out very quickly that DietPi is the OS.

[MichaIng]

@dschaper

If the contributor that provided the instruction would like to respond.

pi-hole/web#1148 (comment) (and below)

That php7.3-intl got installed was not true but a misunderstanding as he watched the linked code instead of what packages got really installed. As he uses DietPi, no PHP module was installed at all by the Pi-hole installer by design.

On the side, is there any way you can identify DietPi

2020-03-10 20:17:38 root@VM-Buster:~# ls /boot/dietpi.txt
/boot/dietpi.txt

However the originating reason for the misunderstanding and wrong fix should checked as well:

2020-03-10 20:18:59 root@VM-Buster:~# grep 'INSTALL_WEB_SERVER' /etc/pihole/setupVars.conf
INSTALL_WEB_SERVER=true

Probably it makes sense to document the consequence of this option, the required PHP modules and that those need to be installed manually, when using the option, what you think?

Another idea:

• PHP modules and webserver mostly are not related to each other.
• It make sense to install php-cgi only together with the webserver, as if users have a different one installed already, most likely a different handler module as well (php-fpm or mod-php). But the other PHP modules are required in any case, independent of the webserver.

So the idea would be to split:

WEB_INTERFACE_DEPS="${phpVer}-xml ${phpVer}-${phpSqlite} ${phpVer}-intl"
WEB_SERVER_DEPS="lighttpd ${phpVer}-cgi"

php-common is pulled by php-cgi btw, hence not necessarily required as dedicated package here, however would not hurt.
What you think?

[dschaper]

That php7.3-intl got installed was not true but a misunderstanding as he watched the linked code instead of what packages got really installed. As he uses DietPi, no PHP module was installed at all by the Pi-hole installer by design.

If we could get a definitive answer from @mahowi and not third party interpretations of what we think was said or meant.

Probably it makes sense to document the consequence of this option, the required PHP modules and that those need to be installed manually, when using the option, what you think?

Consequences of what option? The consequences of not installing a webserver? I think that's rather obvious. Admin page doesn't work.

What you think?

If no web server is installed then no PHP modules should be either.

[dschaper]

I'd still like to see some evidence of this being an issue with non DietPi installs however.

[dschaper]

@jfb-pihole When faced with a DietPi install having issues with the web interface I think our default response is to not install Pi-hole via DietPi-Software and to uninstall and instead use the Pi-hole curl installation process?

[MichaIng]

@dschaper

Consequences of what option? The consequences of not installing a webserver? I think that's rather obvious. Admin page doesn't work.

This option was added by Fourdee that time to allow installing other webserver than Lighttpd or with the faster php-fpm handler. By this Pi-hole can be used with Nginx, Apache2 and php-fpm. So of course a webserver is required when the web interface is wanted, but not necessarily Lighttpd with php-cgi.

Of course this option is rarely used I guess, however since it exists with reason (that I hope you still agree with) it would make sense to either document it well and/or reduce unwanted impact to a minimum.

[dschaper]

Well, it seems that DietPi changes our installer or our files. There's a lot of sed and patches to make things work. Our install process seems to be functioning well, I don't have a lot of open issues with regards to non-DietPi issues. I think the solution would be to modify your patching to work well.

[MichaIng]

@dschaper

I'd still like to see some evidence of this being an issue with non DietPi installs however.

Which issue you mean? Installing php7.3-intl on any DietPi system will fix any potential issue. Of course this is missing with current version since this was not a dependency prior to Pi-hole v5, which is still in beta, like our fix.

[dschaper]

Installing php7.3-intl on any DietPi system will fix any potential issue.

Okay, then pin/patch/modify your PHP configuration to lock php-intl to php7.3?

[MichaIng]

Well, it seems that DietPi changes our installer or our files. There's a lot of sed and patches to make things work. Our install process seems to be functioning well, I don't have a lot of open issues with regards to non-DietPi issues. I think the solution would be to modify your patching to work well.

Where you get this from? We run the installer untouched. There was one single current issue realted to the web interface with all Pi-hole beta installs and reported across the forum. Then you added PHP intl as fix to the installer, to a part of code which is not executed when using the --no-install-webserver option, hence when using this option the module must be installed manually (which is not an issue at all). As a result I added this to our code. I don't see why DietPi has anything to do with the topic of this PR.

Okay, then pin/patch/modify your PHP configuration to lock php-intl to php7.3?

This is impossible. It depends on the repo on which version this meta package depends on. Also I don't see why you en-question your own code with the phpVer variable now, which was added with good reason to avoid exactly the potential issues I want to re-resolve with this PR.

And again, the whole code, phpVer, the PHP intl module and the commit that added static meta package (that I want to revert here) have anything to do with DietPi, so please keep this out of discussion here and concentrate on the question if the initial idea to use phpVer variable was with reason or you want to have this removed. I hope you conclude that phpVer was with reason and hence there is then no reason to use it for all PHP modules but not for intl only.

[dschaper]

Okay, let's get to the bottom of the issue.

Why do you want to make this change? What evidence can we impartially review that shows this change is needed? What does the person that proposed the change think is the proper action?

[dschaper]

What is your output from the same commands?

dschaper@Mariner-10:~$ apt-cache policy php-intl
php-intl:
  Installed: (none)
  Candidate: 1:7.2+60ubuntu1
  Version table:
     1:7.2+60ubuntu1 500
        500 http://archive.ubuntu.com/ubuntu bionic/universe amd64 Packages

dschaper@Mariner-10:~$ cat /etc/os-release
NAME="Ubuntu"
VERSION="18.04.4 LTS (Bionic Beaver)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 18.04.4 LTS"
VERSION_ID="18.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=bionic
UBUNTU_CODENAME=bionic

dschaper@Mariner-10:~$ apt-cache search php-intl
php-intl - Internationalisation module for PHP [default]
php7.2-intl - Internationalisation module for PHP

[dschaper]

Lastly:

dschaper@Mariner-10:~$ sudo apt install --no-install-recommends --dry-run php-intl
Reading package lists... Done
Building dependency tree
Reading state information... Done
The following additional packages will be installed:
  php7.2-intl
The following NEW packages will be installed:
  php-intl php7.2-intl
0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded.
Inst php7.2-intl (7.2.24-0ubuntu0.18.04.3 Ubuntu:18.04/bionic-updates, Ubuntu:18.04/bionic-security [amd64])
Inst php-intl (1:7.2+60ubuntu1 Ubuntu:18.04/bionic [all])
Conf php7.2-intl (7.2.24-0ubuntu0.18.04.3 Ubuntu:18.04/bionic-updates, Ubuntu:18.04/bionic-security [amd64])
Conf php-intl (1:7.2+60ubuntu1 Ubuntu:18.04/bionic [all])

[MichaIng]

@dschaper
Okay lets start from zero, I think discussion got a bid heated, distracting us from the important questions.

Btw just in case, I know that my writing style can read rude in cases (which is not how it is meant) and when I am sure that I am right, I can be an insisting smart alec, sorry for that 😅.

Okay lets go though it in chronological order:

• With Pi-hole v5 beta, the following issue appeared: https://discourse.pi-hole.net/t/whitelist-and-black-list-failure/28084
• To fix this, the PHP intl module requirement has been added, just the same way as all other PHP modules: https://discourse.pi-hole.net/t/database-error-after-update/27966/6
• On the forum there was provided the solution to install php-intl (as this works generic on Stretch + Buster + Ubuntu etc), which indeed did not work for one DietPi Stretch user, because it pulled the PHP7.4 module, hence he needed to install php7.3-intl: https://discourse.pi-hole.net/t/whitelist-and-black-list-failure/28084/23?u=michaing
  I am sorry for the additional support loop this did cost.
  In all other cases I found, it worked just fine the same way for DietPi users, since, mostly RPi users, we provide Raspbian Buster-based images for a long time already.
• Meanwhile on GitHub the other issue was opened: [release/v5.0] Whitelist shows error box with no error description web#1148
  This indeed is now I guess a 95% DietPi issue, since rarely users use the --disable-install-webserver option or even know about it. However if this option is used, no PHP modules are installed, hence neither the now required intl module.
• However because no one thought about the --disable-install-webserver option, the question appeared why the module was not installed: [release/v5.0] Whitelist shows error box with no error description web#1148 (comment)
  And I hope you don't feel attacked by this, but you made the wrong conclusion:

Because the code is trying to install "${phpVer}-intl" a.k.a php7-intl or php5-intl when I think the package is php-intl?

• ${phpVer}-intl would have been php7.3-intl, hence exactly the same that is pulled by php-intl on this users system.
• In return to your post, @mahowi now tried to install php-intl manually and voila, it worked. But again the conclusion was wrong (sorry for the wording, please don't feel attacked all of you 😉) that ${phpVer}-intl or php7.3-intl would be the wrong package. php-intl did nothing else than pulling php7.3-intl, which was not installed before.
  • Here you see that no intl module is installed: [release/v5.0] Whitelist shows error box with no error description web#1148 (comment)
  • Here you see that DietPi is used: [release/v5.0] Whitelist shows error box with no error description web#1148 (comment)
  • Here you see DietPi-Software using the --disable-install-webserver option: https://github.com/MichaIng/DietPi/blob/master/dietpi/dietpi-software#L4037-L4038
  • And here you see that PHP module deps are not added to the final deps list when this option is used: https://github.com/pi-hole/pi-hole/blob/development/automated%20install/basic-install.sh#L2641-L2644
  • And just for completeness, users can also choose to skip the webserver install during regular installer call without the option: https://github.com/pi-hole/pi-hole/blob/development/automated%20install/basic-install.sh#L1179
    In this case the same issue would appear when migrating from Pi-hole v4 to v5 😉.
• Based on these wrong conclusions, Install php-intl meta package. #3138 has been created and merged.

So I hope the above clarifies that #3138 was based on misunderstanding and wrong conclusions and did not fix anything. Already this might be reason enough to revert it?
Else read the PR description above, where I explain that in certain (probably rare and/or even not officially supported circumstances by Pi-hole) #3138 leads to wrong or multiple PHP instances/modules being installed.
If those rare/unsupported circumstances are no argument, at least internal coding consistency, using the same method to pull all PHP packages, might be?
In the latter case, of course it would be an alternative to use only the meta packages for all PHP modules and skip the phpVer estimation. This breaks above mentioned Debian and Ubuntu versions, but those are not anymore officially supported anyway.

[mahowi]

I can confirm what @MichaIng wrote. As I didn't know about the --disable-install-webserver option DietPi uses for installation I thought installing php-intl had fixed my issue. But after a fresh installation of DietPi I recognized neither php7.3-intl nor php-intl got installed at all. Sorry for the confusion.

[jfb-pihole]

"@jfb-pihole When faced with a DietPi install having issues with the web interface I think our default response is to not install Pi-hole via DietPi-Software and to uninstall and instead use the Pi-hole curl installation process?"

This is correct, since we know the details of how our installer functions and not so much of how the DietPi install is implemented. Sometimes, but not always, this resolves the user problem.

I too would like to see a clear indicator in a debug log that DietPi is the OS. We typically infer that from the DietPi name assigned to a device or to some details in the web config files, but a clear "this is DietPi" somewhere would be helpful.

[MichaIng]

@jfb-pihole
Yeah, not all users follow the convention to report bugs to the distro maintainer in the first place, to evaluate if it's a distro/implementation bug or a software-side bug. I guess this is the same when users run into issues with an installed Debian package, but report it to the upstream devs directly...

IMO better then asking the users to purge and reinstall Pi-hole with official installer, would be to redirect them to https://github.com/MichaIng/DietPi/issues, so we can investigate if this is an issue with DietPi-Software in the first place, if so can fix it, else forward to you.

Since DietPi is a modified Debian/Raspbian, /etc/os-release cannot be used to identify it (as usually on distros). [[ -f /boot/dietpi.txt ]] is the safest way to check for DietPi, since this file must always exist and will never change location.

[dschaper]

We do that with the Arch AUR package when issues come up. But those maintainers make it very clear to their users that it's fully supported by their own team. If a users comes to us to ask how to fix their non-working install, we try to fix it. In the vast majority of the cases an install via the official installer works.

Nevertheless, identifying a DietPi install via the debugger would be the first step towards us sending those users to you for support.

[MichaIng]

@dschaper

But those maintainers make it very clear to their users that it's fully supported by their own team.

How do they make this clear?
I was thinking about how to do that, but the only thing that came to my mind is having an additional prompt at start of DietPi-Software installs or pin it on top of every software online doc thread in our forum, but both seems pretty uncommon, never saw such anywhere. Of course when things are packaged, the package meta infos can contain maintainer info (although regular users check this rarely), but a packaging system is nothing that we can possibly maintain currently. Although I was already thinking to spin up a simple APT server and put a few pre-compiled deb packages inside, we use. More things could be migrated over time... much more ideas than man power...

---

However lets get back to topic. Please read #3204 (comment) which should make clear that this PR simply reverts a mistake that didn't fix something.

[jfb-pihole]

"IMO better then asking the users to purge and reinstall Pi-hole with official installer, would be to redirect them to https://github.com/MichaIng/DietPi/issues, so we can investigate if this is an issue with DietPi-Software in the first place, if so can fix it, else forward to you."

I agree in principle, but in many cases when we ask the user which distro of Pi-hole they are using, they don't know. So, having them install it from the Pi-hole installer puts them on a known install platform, something we can help them with. Note that these help requests are coming to the Pi-hole team, so we work with what we know. If it's clear they are running the DietPi install, we do refer them to your site.

"[[ -f /boot/dietpi.txt ]] is the safest way to check for DietPi, since this file must always exist and will never change location."

This check should be added to the debugger. We don't want to get in the loop of ask/answer with users - "does this file exist, does that file exist, etc." It would be helpful if a debug log clearly showed us what we are dealing with, then we can point the user in the correct direction for help.

[dschaper]

https://aur.archlinux.org/packages/pi-hole-server/#pinned-633053

[MichaIng]

@dschaper
Such a per-package "chat" is indeed nice. I'll think through it and will implement the best idea I got with next release. Current favourite: On first DietPi-Software selection, prompt with an additional whiptail that contains this info + link to GitHub issues + troubleshoot forum.

[DL6ER]

What was the conclusion on this?

[MichaIng]

@DL6ER
From my side the case is clear. It simply cannot fix anything but only creates the risk of multiple PHP instances when using explicit PHP version packages in all but one case. Either the installed PHP version (if present) is checked and modules/packages are consequently installed for the found PHP version in every case, or the meta packages are installed in all cases, leaving the version choice to the repo maintainers. The latter makes it impossible for users to have a different PHP version installed, which could be wanted or not, but the PHP version check was once implemented for reason, which I suggest to review before doing such a decision.

I think the trouble around DietPi causing bug reports with Pi-hole v5 in your forum, and that apt-get install php-intl did not solve it on DietPi Stretch systems (as we by default install PHP7.3 there, to keep support for many modern web applications) masked the root reason and argument of this PR in the discussion above.

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/cant-update-to-5-0-php7-2-intl/31869/33

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-5-1-released/35577/1