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

php: fix php-config header file paths #24648

Merged
merged 1 commit into from May 1, 2017
Merged

php: fix php-config header file paths #24648

merged 1 commit into from May 1, 2017

Conversation

asppsa
Copy link
Contributor

@asppsa asppsa commented Apr 5, 2017

Motivation for this change

Split outputs mean that the "include" folder from PHP gets placed into a
"dev" derivation. However php-config is not aware of this, which means
that compiling extensions with phpize fails with an error about being
unable to find header files (see #24357, #24420). This fixes that by:

  1. passing the --includedir flag to configure so that php-config
    gives the correct paths.

  2. moving the phpize and php-config scripts and man pages to the
    dev derivation, to prevent cylic references.

  3. ensuring that the configure script arguments are stripped from
    all binaries, including php-embed, to prevent cyclic references.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@asppsa, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @dezgeg and @globin to be potential reviewers.

@asppsa
Copy link
Contributor Author

asppsa commented Apr 10, 2017

Concerning the point about php-embed, there was a comment in the derivation code that said that uWSGI needed php to contain the configure paths. I have built uwsgi with php using my patch, and am running it without any problems, so perhaps it is no longer the case that uWSGI needs the configure info.

postFixup = ''
mkdir -p $dev/bin $dev/share/man/man1
mv $out/bin/phpize $out/bin/php-config $dev/bin/
mv $out/share/man/man1/phpize.1.gz \
Copy link
Member

@calbrecht calbrecht Apr 14, 2017

Choose a reason for hiding this comment

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

Building php71 on NixOs master, for me there is no $out/share/man/man1/phpize.1.gz but a $out/php/man/man1/phpize.1 likewise for php-config
edit: nevermind -- it is allright

@calbrecht
Copy link
Member

Though extension are building fine with this patch, i dislike the fact somehow that the binaries php-config and phpize will not show up in $PATH like that.

@asppsa
Copy link
Contributor Author

asppsa commented Apr 16, 2017

Concerning the $PATH, in what environment is it that php-config and phpize are not present? I have tried the following:

  • nix-env -f /path/to/nixpkgs --install php; and
  • nix-shell -I nixpkgs=/path/to/nixpkgs -p php

@calbrecht
Copy link
Member

Of course you are right. I was referring to the $PATH of my login-shell.

@schneefux
Copy link
Contributor

Concerning the point about php-embed, there was a comment in the derivation code that said that uWSGI needed php to contain the configure paths. I have built uwsgi with php using my patch, and am running it without any problems, so perhaps it is no longer the case that uWSGI needs the configure info.

That comment was mine - I tried your PR and I get these compile errors (again):

*** uWSGI is ready, launch it with ./uwsgi ***
using profile: buildconf/nixos.ini
detected include path: ['/nix/store/p44wyjqcmb8ldjj3yxjf9v3d4ml6k29x-gcc-5.4.0/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/include', '/nix/store/p44wyjqcmb8ldjj3yxjf9v3d4ml6k29x-gcc-5.4.0/include', '/nix/store/p44wyjqcmb8ldjj3yxjf9v3d4ml6k29x-gcc-5.4.0/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/include-fixed', '/nix/store/hrgvzzs1j58cr0ngb7awjxad1lp2zrqh-glibc-2.25-dev/include']
*** uWSGI building and linking plugin plugins/python ***
[gcc] /nix/store/6d8p7gi1603lqp5ahrly5l08ldzhjix7-uwsgi-2.0.14/lib/uwsgi/python3_plugin.so
build time: 7 seconds
*** python3 plugin built and available in /nix/store/6d8p7gi1603lqp5ahrly5l08ldzhjix7-uwsgi-2.0.14/lib/uwsgi/python3_plugin.so ***
/nix/store/9nr5wrxaiw43f7a6rgj3xxz5gzkf4yvy-php-7.1.2/bin/php-config: line 20: out: command not found
/nix/store/9nr5wrxaiw43f7a6rgj3xxz5gzkf4yvy-php-7.1.2/bin/php-config: line 20: out: command not found
/nix/store/9nr5wrxaiw43f7a6rgj3xxz5gzkf4yvy-php-7.1.2/bin/php-config: line 20: out: command not found
/nix/store/9nr5wrxaiw43f7a6rgj3xxz5gzkf4yvy-php-7.1.2/bin/php-config: line 20: out: command not found
In file included from plugins/php/php_plugin.c:1:0:
plugins/php/common.h:1:17: fatal error: php.h: No such file or directory
compilation terminated.
In file included from plugins/php/session.c:1:0:
plugins/php/common.h:1:17: fatal error: php.h: No such file or directory
compilation terminated.
using profile: buildconf/nixos.ini
detected include path: ['/nix/store/p44wyjqcmb8ldjj3yxjf9v3d4ml6k29x-gcc-5.4.0/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/include', '/nix/store/p44wyjqcmb8ldjj3yxjf9v3d4ml6k29x-gcc-5.4.0/include', '/nix/store/p44wyjqcmb8ldjj3yxjf9v3d4ml6k29x-gcc-5.4.0/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/include-fixed', '/nix/store/hrgvzzs1j58cr0ngb7awjxad1lp2zrqh-glibc-2.25-dev/include']
*** uWSGI building and linking plugin plugins/php ***
[gcc] /nix/store/6d8p7gi1603lqp5ahrly5l08ldzhjix7-uwsgi-2.0.14/lib/uwsgi/php_plugin.so
*** unable to build php plugin ***

I remember that I tracked the issue of the missing php.h down to uwsgi trying to access the libraries PHP was built with and failing because of the paths being unset. I spent a few hours trying to fix and could not find a different solution 😃

@asppsa
Copy link
Contributor Author

asppsa commented Apr 23, 2017

Curse, could have sworn it was working ... I shall take another look!

@asppsa
Copy link
Contributor Author

asppsa commented Apr 23, 2017

@schneefux, so it seems I am still able to run uWSGI+PHP from my PR branch. I am using nix-shell as follows:

  1. I have my php-config-fix2 branch checked out at /home/asp/src/nixpkgs
  2. I have a default.nix file containing the following:
with (import /home/asp/src/nixpkgs {});
let uwsgi = callPackage /home/asp/src/nixpkgs/pkgs/servers/uwsgi {
  plugins = [ "php" ];
};
in stdenv.mkDerivation {
  name = "my-env";
  buildInputs = [ uwsgi ];
}
  1. Then I run nix-shell --run 'uwsgi --plugin php -s :3030 -M' default.nix to start uWSGI.
  2. I'm then connecting to it using Nginx with uwsgi_pass and checking that I can view a phpinfo(); page.

Can you share the way that you are running uWSGI? Maybe I have not done this correctly for some configuration ...

@asppsa
Copy link
Contributor Author

asppsa commented Apr 23, 2017

@calbrecht, I see what you mean now. I think this is actually an issue in NixOS. When I do the nix-env --install on my Archlinux system, running Nix 1.11.7, the bin files in the dev package are added to the path. When I do the same thing in NixOS 17.03 (running Nix 1.11.8), the dev packages are not installed! I think this may be related to issue#24717

@asppsa asppsa closed this Apr 23, 2017
@asppsa asppsa reopened this Apr 23, 2017
@schneefux
Copy link
Contributor

I had a typo and merged the wrong PR actually 😅 Sorry for the confusion.
It seems to work well.
For reference, this is the configuration I use. Thank you for your contribution!

Split outputs mean that the "include" folder from PHP gets placed into a
"dev" derivation. However php-config is not aware of this, which means
that compiling extensions with phpize fails with an error about being
unable to find header files (see #24357, #24420).  This fixes that by:

1. passing the `--includedir` flag to `configure` so that `php-config`
   gives the correct paths.

2. moving the `phpize` and `php-config` scripts and man pages to the
   dev derivation, to prevent cylic references.

3. ensuring that the `configure` script arguments are stripped from
   all binaries, including `php-embed`, to prevent cyclic references.
@asppsa
Copy link
Contributor Author

asppsa commented Apr 24, 2017

@calbrecht, I think I have solved this now. I've added a meta.outputsToInstall entry so that nix-env --install installs both out and dev.

@7c6f434c 7c6f434c merged commit 6008ede into NixOS:master May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants