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

[20.09] pkgs/uwsgi: fix compiling and linking the php plugin in uwsgi #107792

Merged
merged 4 commits into from Dec 31, 2020

Conversation

gdamjan
Copy link
Contributor

@gdamjan gdamjan commented Dec 28, 2020

Motivation for this change

fixes #97352 - it enables the use of the php plugin of uwsgi. A port to release-20.09, of the PR #107403 that merged the same change to master.

Things done

I tested in a nixos docker container, on ArchLinux host:

$ docker run -ti --rm -v ~/src/nixpkgs:/nixpkgs nixos/nix:latest
# nix-build -E 'with import <nixpkgs> {}; uwsgi.override { plugins=[ "php" ]; }' # fails as specified in the bug 97352

# NIX_PATH=nixpkgs=/nixpkgs nix-build -E 'with import <nixpkgs> {}; uwsgi.override { plugins=[ "php" ]; }' # works
Explanation of the changes

The root of the problem is that uwsgi has a bit of a different build system, and it would expect to find the php-config binary in order to set it's compiler and linker flags. There was an additional problem that the uwsgi-php plugin needs to also link with the session.so php extension - which on NixOS is a separate package¹.

The first problem was solved by adding all the required buildInputs to the uwsgi package so that it can find all the needed header files, libraries and to have php-config in its PATH.

I also had to patch uwsgi to not look for php_session.h with a path like this #include "ext/session/php_session.h", since on NixOS it's just in …php-session-7.4.8-dev/include.

Third, I had to patch the uwsgi build system for the php plugin to explicitly add the session.so library and its rpath to LDFLAGS. In the nix expression that's exported as the UWSGICONFIG_PHP_LDFLAGS environment variable. It's important that these flags are not used for the main uwsgi binary, but just for the php-plugin shared object.

Copy link
Contributor

@rnhmjoj rnhmjoj left a comment

Choose a reason for hiding this comment

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

Thank you for opening the PR.
Backport should be done using cherry-pick -xe, see CONTRIBUTING.md.

@gdamjan gdamjan changed the title pkgs/uwsgi: fix compiling and linking the php plugin in uwsgi [20.09] pkgs/uwsgi: fix compiling and linking the php plugin in uwsgi Dec 28, 2020
gdamjan and others added 4 commits December 29, 2020 22:56
uwsgi needs to run php-config to get the include path. set the
UWSGICONFIG_PHPPATH to the php-config in the php.dev store.

Patch common.h to directly include php_session.h (since it's a separate package on
nixos).

NIX_CFLAGS_LINK was removed since it doesn't seem to be used at all

https://github.com/unbit/uwsgi/blob/master/plugins/php/common.h
(cherry picked from commit 6dd858d)
On NixOS the session.so php extension is a separate package (and not builtin
in libphp.so). But since the uwsgi php plugin uses the session
mechanisms, we need to link the plugin to that library too.

With this change uwsgipluginpy is hacked to take an additional UWSGICONFIG_PHP_LDFLAGS
environment variable and add it to its LDFLAGS, and then in the nix
expression the UWSGICONFIG_PHP_LDFLAGS is set to point to php.extensions.session

(cherry picked from commit 87bed6c)
(cherry picked from commit 06749ba)
Copy link
Contributor

@rnhmjoj rnhmjoj left a comment

Choose a reason for hiding this comment

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

Looks good.

@rnhmjoj rnhmjoj merged commit 55e4ef4 into NixOS:release-20.09 Dec 31, 2020
@gdamjan gdamjan deleted the fix-uwsgi-php-20.09 branch December 31, 2020 13:58
gdamjan added a commit to gdamjan/tt-rss-service that referenced this pull request Jan 21, 2021
gdamjan added a commit to gdamjan/nextcloud-service that referenced this pull request Jan 21, 2021
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.

None yet

2 participants