Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PHP pkg-config (pkgconf) .pc metadata file(s) #13755

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

petk
Copy link
Member

@petk petk commented Mar 19, 2024

Since PHP *nix Autotools build system already extensively relies on
pkgconf (pkg-config) command-line tool to find dependencies it should
also provide its .pc metadata file(s). 1

This adds an initial php.pc.in and php-embed.pc.in templates that are
created during the configuration and build phase. They are installed in
the provided system pkgconfig directory. For example:

/usr/lib/pkgconfig/php.pc
/usr/lib/pkgconfig/php-embed.pc
/usr/lib/pkgconfig/php-embed8.pc
/usr/lib/pkgconfig/php-embed8.4.pc
/usr/lib/x86_64-linux-gnu/pkgconfig/php.pc

Pkgconf 2 is a maintained continuation of the initial Freedesktop's
pkg-config project 3.

Extensions and applications embedding the PHP Embed SAPI can then
use also pkgconf to get the required PHP CFLAGS and/or LIBS.

For PHP extensions:

pkgconf --cflags php
pkgconf --libs php
pkgconf --modversion php
pkgconf --cflags php8.4

Applications embedding the PHP Embed SAPI:

pkgconf --libs php-embed
pkgconf --cflags php-embed
pkgconf --modversion php-embed
pkgconf --cflags php-embed8.4

Mapping of php-config and pkg-config usage:

php-config option pkg-config
--prefix pkg-config --variable=prefix php
--includes pkg-config --cflags-only-I php
pkg-config --cflags php
--ldflags pkg-config --libs-only-L php
--libs pkg-config --libs-only-l php
pkg-config --libs-only-other php
pkg-config --libs php
--extension-dir pkg-config --variable=php_extension_dir php
--include-dir pkg-config --variable=includedir php
--lib-dir pkg-config --variable=libdir php-embed
--lib-embed N/A
pkg-config --libs php-embed (--shared)
pkg-config --libs php-embed --static
--man-dir N/A
--php-binary N/A
--php-sapis N/A
--ini-path pkg-config --variable=php_ini_path php
--ini-dir pkg-config --variable=php_ini_dir php
--configure-options N/A
--version pkg-config --modversion php
--vernum pkg-config --variable=php_vernum php
pkg-config --variable=php_zts php
pkg-config --variable=php_debug php

@petk petk force-pushed the patch-pkgconf-pc-in branch from 7456c0b to e0b2ae9 Compare March 20, 2024 07:14
@petk petk force-pushed the patch-pkgconf-pc-in branch 2 times, most recently from e0230ba to c8d2e55 Compare July 4, 2024 15:53
@morrisonlevi
Copy link
Contributor

morrisonlevi commented Jul 5, 2024

I think we should add variables php_debug and php_zts as well. Extensions need to know this information to order to know what ABIs to build for.

@petk
Copy link
Member Author

petk commented Jul 5, 2024

I think we should add variables php_debug and php_zts as well. Extensions need to know this information to order to know what ABIs to build for.

Yes, this list of variables needs to be expanded further. I've added php_debug=yes|no and php_zts=yes|no variables.

@morrisonlevi
Copy link
Contributor

I think it needs the PHP_VERSION_ID as well. The php-config calls this --vernum. I may try building against this today to see if I can get it to work.

@petk
Copy link
Member Author

petk commented Jul 8, 2024

I think it needs the PHP_VERSION_ID as well. The php-config calls this --vernum. I may try building against this today to see if I can get it to work.

This one is added as php_vernum. php_vernum=@PHP_VERSION_ID@.

@morrisonlevi
Copy link
Contributor

I just built from source and this one didn't get expanded:

Cflags.private: @PHP_CFLAGS_PRIVATE@

Also, is it expected that it goes into ${prefix}/lib/php/pkgconf instead of ${prefix}/lib/pkgconf?

@petk petk force-pushed the patch-pkgconf-pc-in branch from 8db6dd8 to 6586486 Compare July 8, 2024 21:56
@petk
Copy link
Member Author

petk commented Jul 8, 2024

I just built from source and this one didn't get expanded:

Cflags.private: @PHP_CFLAGS_PRIVATE@

Also, is it expected that it goes into ${prefix}/lib/php/pkgconf instead of ${prefix}/lib/pkgconf?

Fixed in the 3rd commit. Installation now puts it to pkgconf directory in libdir. For now, I've removed this Cflags.private part.

@petk petk force-pushed the patch-pkgconf-pc-in branch from 6586486 to c047551 Compare July 15, 2024 19:04
@petk petk force-pushed the patch-pkgconf-pc-in branch from c047551 to 2538bbb Compare August 11, 2024 17:59
@petk petk marked this pull request as ready for review August 11, 2024 17:59
@petk petk changed the title [WIP] Add PHP pkgconf .pc metadata file(s) Add PHP pkgconf .pc metadata file(s) Aug 11, 2024
@petk petk added this to the PHP 8.4 milestone Aug 13, 2024
@petk petk force-pushed the patch-pkgconf-pc-in branch from 2538bbb to 5cd2195 Compare August 13, 2024 23:24
@Girgias
Copy link
Member

Girgias commented Aug 14, 2024

@morrisonlevi could you have another look to see if this behaves like you expect it?

I have no knowledge about build systems/package dependency resolution for system libraries, but I think or we be good to use what seems to be industry standard.

@petk petk force-pushed the patch-pkgconf-pc-in branch from 5cd2195 to 848e975 Compare August 15, 2024 10:33
@petk
Copy link
Member Author

petk commented Aug 15, 2024

I've rebased the branch so it's easier to handle this PR. Here actually everything works but PHP has a bit of weird include flags handling at the moment. This pkg-config template files are (for now) done like the php-config script is done. I'll recheck all the pkg-config variables again what needs to be added.

As far as the PHP-8.4 is concerned I won't touch the include flags yet in this branch. Because they aren't done correctly. Neither the main PHP include directory is done correctly. This also goes for the php-config script. They are for now hard-coded in there as part of the PHP history, but they would need to be dynamic.

@petk petk force-pushed the patch-pkgconf-pc-in branch from 848e975 to 538f6f7 Compare August 17, 2024 13:35
@petk petk force-pushed the patch-pkgconf-pc-in branch from 538f6f7 to d2c4f96 Compare September 5, 2024 10:45
@petk
Copy link
Member Author

petk commented Sep 5, 2024

I've also added a quick table for tracking options between php-config script and possible pkg-config alternatives in the PR's first post. The next appended commit adds some fixes and syncs of the .pc.in templates.

petk added 4 commits September 9, 2024 18:40
Since PHP *nix Autotools build system already extensively relies on
pkgconf (pkg-config) command-line tool to find dependencies it should
also provide its .pc metadata file(s). [1]

This adds an initial php.pc.in and php-embed.pc.in templates that are
created during the configuration and build phase. They are installed in
the provided system pkgconfig directory. For example:

    /usr/lib/pkgconfig/php.pc
    /usr/lib/pkgconfig/php-embed.pc
    /usr/lib/pkgconfig/php-embed8.pc
    /usr/lib/pkgconfig/php-embed8.4.pc
    /usr/lib/x86_64-linux-gnu/pkgconfig/php.pc

Pkgconf [2] is a maintained continuation of the initial Freedesktop's
pkg-config project [3].

Extensions and applications embedding the PHP Embed SAPI can then
use also pkgconf to get the required PHP CFLAGS and/or LIBS.

For PHP extensions:

    pkgconf --cflags php
    pkgconf --libs php
    pkgconf --modversion php
    pkgconf --cflags php8.4

Applications embedding the PHP Embed SAPI:

    pkgconf --libs php-embed
    pkgconf --cflags php-embed
    pkgconf --modversion php-embed
    pkgconf --cflags php-embed8.4

[1]: https://people.freedesktop.org/~dbn/pkg-config-guide.html
[2]: https://github.com/pkgconf/pkgconf
[3]: https://gitlab.freedesktop.org/pkg-config/pkg-config
When customizing the Autoconf's lib directory option with for example:

    ./configure --libdir=--libdir=/usr/lib/x86_64-linux-gnu

This lib directory should be also inserted in the generated *.pc files.
- variables renamed and synced further to match php-config a bit
- Redundant comments removed and refactored
- Typos fixed
- Extension directory template placeholder replaced with @EXTENSION_DIR@
  so it's possible to insert variables directly `$prefix/php/...`
@petk petk force-pushed the patch-pkgconf-pc-in branch from d2c4f96 to 33c26fb Compare September 9, 2024 16:48
@petk petk changed the title Add PHP pkgconf .pc metadata file(s) Add PHP pkg-config (pkgconf) .pc metadata file(s) Sep 9, 2024
The "Requires" inherits all Cflags, Libs and their ".private" fields
and adds it to the output, which simplifies writing templates a bit.
@petk
Copy link
Member Author

petk commented Sep 9, 2024

Yes, this pkg-config stuff is complex, anyone here to test it :D?

Basically it works but the main issue with this will be in scenarios where different PHP versions are installed (where also php-config is prone to errors) or when installing PHP with program prefix/sufix (like php8.4), or when in extreme case both static and shared libphp embed is installed. The PHP Embed library still probably needs one more adjustment to totally match the php-config's php-config --lib-embed behavior. But I think we can adjust these .pc files on the go as needed, either through PHP 8.4 bug fixes or add more fine-tuned features in the next version...

@petk
Copy link
Member Author

petk commented Sep 12, 2024

I got another idea about the php_zts and php_debug variables. Instead of these only a single pkg-config variable like php_features can be added:

--- a/scripts/php.pc.in
+++ b/scripts/php.pc.in
@@ -12,10 +12,7 @@ php_vernum=@PHP_VERSION_ID@
-# Whether PHP is built in debug mode (yes) or not (no).
-php_debug=@ZEND_DEBUG@
-# Whether PHP is built with thread safety (yes) or not (no).
-php_zts=@PHP_THREAD_SAFETY@
+php_features="@PHP_FEATURES@"

This php_features would be a space separated list of enabled features, for example:

php_features="DEBUG ZTS"

Then the php_features can be parsed manually by the build system where needed. And it can be a bit simpler extended in the future with more "features" if needed. For example, curl is using a similar approach and I think it works pretty ok.

Anyhow, I think that this won't be able to land in PHP 8.4 so, let's target next PHP version here then.

@petk petk removed this from the PHP 8.4 milestone Sep 12, 2024
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, this looks like a very good (and overdue) addition (we could get rid of php-config in the long run). Thank you!

I got another idea about the php_zts and php_debug variables. Instead of these only a single pkg-config variable like php_features can be added:

I think this is a good idea. (I would probably go with features only, though.)

└─ ...
├─ scripts/ # php-config, phpize and internal development scripts
├─ scripts/ # php-config, phpize, pkg-config, and internal development scripts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might better be pkg-config metadata file or maybe just .pc, or something like this.

Comment on lines -1470 to +1472
PHP_SUBST([ZEND_EXTRA_LIBS])
PHP_SUBST_OLD([ZEND_EXTRA_LIBS])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, what's the difference between these macros?

php_zts=@PHP_THREAD_SAFETY@

Name: PHP
Description: Build extension for a PHP general-purpose scripting language
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, isn't this supposed to be the description of the package? So maybe instead

Suggested change
Description: Build extension for a PHP general-purpose scripting language
Description: A popular general-purpose scripting language that is especially suited to web development

License: PHP
Version: @PHP_VERSION@
Cflags: -I${includedir} -I${includedir}/main -I${includedir}/TSRM -I${includedir}/Zend -I${includedir}/ext -I${includedir}/ext/date/lib
Libs.private: @EXTRA_LIBS@ @ZEND_EXTRA_LIBS@
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add @PHP_LDFLAGS@ here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants