Move ext/tidy to pkg-config

[NattyNarwhal]

It seems most distributions i.e. Debian/Gentoo/Fedora have all moved over to html-tidy as the tidy library they ship. It ships a pkg-config file, so we can just simply check for that and omit the various alternatives to check for.

I'm not sure if the checks for library functionality are needed anymore if we assume the html5 tidy fork either.

[NattyNarwhal]

CI issue seems related, but I can't seem to reproduce it locally on macOS w/ MacPorts and Alpine.

[petk]

This HAVE_TIDYBUFFIO_H macro is also used elsewhere in the code.

And tidy headers should be included like this then:

index f012d8466f..aa58281e56 100644
--- a/ext/tidy/tidy.c
+++ b/ext/tidy/tidy.c
@@ -26,8 +26,8 @@
 #include "php_ini.h"
 #include "ext/standard/info.h"
 
-#include "tidy.h"
-#include "tidybuffio.h"
+#include <tidy/tidy.h>
+#include <tidy/tidybuffio.h>
 
 #include "tidy_arginfo.h"

I have no idea where are the tidy library sources for Windows. Some sort of magic installation it seems so I'm not sure how to sync these two.

Edit: Windows library sources are these: https://github.com/winlibs/libtidy (GitHub can't seem to search for "tidy" keyword among the repositories obviously).

[NattyNarwhal]

The bit about tidy/ is a little confusing to me, because at least the distributions I’ve been testing with seem to put it in the root of the package manager’s include directory (MacPorts shown):

% ls -d /opt/local/include/tidy*

/opt/local/include/tidy.h               /opt/local/include/tidyenum.h
/opt/local/include/tidybuffio.h         /opt/local/include/tidyplatform.h

Alpine is the same way.

That said, I checked Debian’s package layout and it does seem to put it in a subdirectory. That’s a little annoying to deal with.

[petk]

Indeed. The Debian package doesn't update the .pc file: https://salsa.debian.org/debian/tidy-html5/-/blob/master/debian/libtidy-dev.install?ref_type=heads and the headers are installed in the tidy namespace/subdirectory.

Otherwise the headers are intended to be included like <tidy.h> according to the documentation https://api.html-tidy.org/tidy/tidylib_api_5.8.0/libtidy_04.html

So, then I think that this include directory will need to be adjusted based on whether there is a tidy subdirectory or not.

[petk]

I'd suggest to move this perhaps to the version after PHP-8.4. I think it will be a bit too big of a change at once to:

• remove support for tidyp fork (which uses the obsolete buffio.h header)
• use pkg-config exclusively instead of the DIR argument
• remove the obsolete buffio.h header

FreeBSD still has tidyp package, and also PHP is installing the tidyp in .cirrus.yml file for FreeBSD. Although could be replaced with tidy-html5 package on current FreeBSD versions.

🤔

[NattyNarwhal]

Yeah, I agree that this turned out to be more ugly than expected. I'll check what tidyp is doing.

[petk]

Otherwise, these can be adjusted to use pkg-config also at some point: gmp, tidy, ldap, snmp. In dba extension (cdb, lmdb, TokyoCabinet, and QDBM). And ACL library in FPM.

[NattyNarwhal]

I had posted in the PHP Foundation Slack with what I found, and it mostly overlaps yours (didn't know gmp shipped one), though some caveats.

• LDAP is nasty because it seems it can rely on libraries other than OpenLDAP i.e. a Solaris system one, and an Oracle one (!?). OpenLDAP also only started shipping a pkg-config file recently-ish (later in 2.5 cycle?), so there might be some time for it to shake out.
• readline actually does ship a pkg-config file nowadays (libedit did and we already use that), but it also uses ncurses and we'll have to discover that the traditional way.
• bzip2 one day, when the 1.1 branch ships.
• firebird, gettext, MM (used by session, seems unmaintained?) don't ship them.

[petk]

The readline should be removed from php-src #13184 because its license is not compatible with PHP.

Yes, that ldap is quite complex one. According to my checks, the Oracle's LDAP should be removed also from php-src because it doesn't work anyhow and cannot be built. It lacks a lot of features that was integrated in ldap extension unconditionally with OpenLDAP in mind only.

MM library integration I think is also targeted for removal in PHP 9: #14019

[petk]

Debian tidy-html5 issue about the missing include flag (-I/usr/include/tidy) has been reported at https://salsa.debian.org/debian/debpkg-metadata/-/issues/2