cc @remicollet

It's fatally broken if you actually do try to set $PKG_CONFIG (to something other than a fully-qualified path), instead, the extremely well-tested pkg.m4 macro provided by pkg-config itself, which forms the autotools standard, should be used.

Please use #3632 instead. Apologies for not submitting it much, much sooner.

Using the pkg-config macros is also preferable, because it will add ./configure --help hints for the PKG_CONFIG* variables available to use, and additionally it will allow using FREETYPE2_CFLAGS/FREETYPE2_LIBS to override pkg-config detection (and hint about that too).

@eli-schwartz Sure, but there are several other cases where pkg-config is used like this, so I would rather suggest to separate these two issues and have a quick fix for freetype2 and then fix pkg-config throughout the whole source tree.

I could work on porting the other uses of pkg-config over. :)
(A proper fix for freetype which pulls in pkg.m4 does not need to state anything about the rest of the tree, so I see no reason to separate this.)

I started on curl, pushed to a new branch to avoid forcing it in my PR, see https://github.com/eli-schwartz/php-src/commits/pkg-config-everywhere

Also your PR leaves the configure help slightly confusing, e.g. what does --with-freetype-dir do when pkg-config is in use?

Also your PR leaves the configure help slightly confusing, e.g. what does --with-freetype-dir do when pkg-config is in use?

Well, the patch could be rewritten so that an explicitly given --with-freetype-dir would be used like before (i.e. no pkg-config involvement).

--with-freetype-dir should be preferred, if passed. That allows to link to a custom lib version even the system one is installed and registered to pkg-config. Otherwise users will be forced to uninstall the system lib and some unrelated packages might depend on it. It gives also more flexibility for developers on adopting new lib versions. IMO that's the only point the current patch should take care about.

The migration to the pkg-config macro makes sense but probably should be done as a separate effort. When the macro is added and all the relevant places are migrated, new PRs can use it.

Thanks.

Usually, it’s better (and enough) to pass correct directory in PKG_CONFIG_PATH for a custom installation.

Would the path order in PKG_CONFIG_PATH be respected? Fe a lib is in both system and a custom prefix, would export PKG_CONFIG_PATH=/my/lib/prefix:$PKG_CONFIG_PATH mean pkg-config would check in /my/lib/prefix first?

Thanks.

If --with-freetype-dir would ignore its argument, the option should better be changed to --enable-freetype – otherwise it would rather be confusing.

I've merged the variant at #3632 for master. Do we maybe still want to land this PR (which is less intrusive and essentially backwards compatible) for older versions?

IMO fine for PHP-7.3 (at least), iff we resolve the issue that --with-freetype-dir ignores the passed directory (so either rename to --enable-freetype or prefer the passed directory over pkg-config).