php.buildEnv: Make the exported php package overridable, improve handling of currently enabled extensions, etc #85026
Conversation
|
@talyz I appreciate you doing this. My use case for this PR is to override the |
My follow up question is: Has that ever worked? It feels like the way to do that is to do something like this: (pkgs.callPackage <nixos/pkgs/development/interpreters/php> {
apacheHttpd = pkgs.fooApacheHttpd;
# ...
}).php74With this PR I was kinda more expecting it to work with overriding attributes of the For example: php.override {
version = "7.3.2";
sha256 = "0000000000000000000000000000000000000000000000000000";
}But that doesn't seem to work for me: But this to me indicates that @aanderse wish seems to work, because it seems to override the arguments to the I can do for example the following: php.override { writeText = writeScript; }It gives me a new output, but it works the same, because php.override { apacheHttpd = fooDerivation; }But at the same time php.override { config.php.fpm = false; }Seems to work fine, and that's how I thought people usually toggled flags for PHP? But I may be wrong here. And that worked even without this branch. So to conclude: The only thing I don't know how to easily override is version and sha256 to build another version. |
talyz
force-pushed
the
php_buildenv_override
branch
from
1b9f2e0
to
e435658
Compare
|
Ah, sorry for the confusion. My example did not work, because the |
|
It isn't a hard requirement. It's a nice to have. Don't put too much energy into it if it's a difficult task. |
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-linux: 11-100
and removed
10.rebuild-darwin: 0
10.rebuild-linux: 0
labels
talyz
force-pushed
the
php_buildenv_override
branch
from
31d8f5d
to
70cdc24
Compare
|
No worries! It's tricky, but interesting :) With my latest push you should be able to override both packages and configuration flags, since I've made packages arguments of php.override {
version = "7.4.3";
sha256 = "wVF7pJV4+y3MZMc6Ptx21PxQfEp6xjmYFYTMfTtMbRQ=";
fpmSupport = false;
valgrind = valgrind-light;
}(php.withExtensions (e: php.enabledExtensions ++ [ e.imagick ])).override {
fpmSupport = false;
}I also reworked the way currently enabled extensions are handled - they're now an argument of the anonymous function fed to
As a contrived example of what's possible, you can add ImageMagick, then override the version and disable fpm, and lastly remove the ((php.withExtensions (enabled: all: enable ++ [ all.imagick ])).override {
version = "7.4.3";
sha256 = "wVF7pJV4+y3MZMc6Ptx21PxQfEp6xjmYFYTMfTtMbRQ=";
fpmSupport = false;
}).withExtensions (enabled: all: lib.filter (e: e != all.zip) enabled)Note that a large portion of the diff is whitespace changes, so you'll probably want to view it with |
talyz
force-pushed
the
php_buildenv_override
branch
from
70cdc24
to
239ff60
Compare
talyz
force-pushed
the
php_buildenv_override
branch
2 times, most recently
from
bd1dc90
to
055a12a
Compare
|
How to fix this code to enable build php-unit with extensions? |
Well, the short answer is that I've never used The long answer is that it depends: I'm not sure if it's currently possible in any reasonable fashion - if it simply wraps the |
|
I am used this config Modules loading from ini file |
|
@Izorkin Then it's actually very easy, instead of having the And make sure to define let
customPhp = php.withExtensions (e: php.enabledExtensions ++ [
e.apcu e.imagick e.memcached e.event e.protobuf
]);
in customPhp |
|
@etu how to build php-unit with mysqli mysqlnd opcache? |
|
@Izorkin If it can read from a file to find extensions, you should be able to take the config file I pointed at from the default php attributes ( |
|
@etu with this configuration: Error load library With Error: |
This is close to the right solution, I think. The reason this doesn't work is that you're using extensions built for another PHP. Since |
|
@talyz consider the following configuration: I get the this error: Any ideas? |
|
Yes, with my last big update, I changed how So would be written as or, if you want to build one which builds on the previous one and, if you later want that PHP build, but want to add This means that you always get the right extensions for your version of PHP. It even works if you apply an override which changes the version or build options of PHP in between. I still have to update the documentation to reflect this, of course. I realize that it's not very clear from my last update post - sorry about that; I wrote half of it before making this change and the rest after, so yeah.. 😅 |
|
I think I still prefer the single argument function. Having phpPackage =
master.php.withExtensions
(extensions:
master.php.enabledExtensions ++ [ extensions.pdo ]);we can just make the phpPackage =
master.php.withExtensions
(extensions:
extensions.php.enabledExtensions ++ [ extensions.pdo ]);It rolls of a tongue slightly worse than Or even just make the list of enabled extensions available if we do not want to expose phpPackage =
master.php.withExtensions
(extensions:
extensions.currentlyEnabled ++ [ extensions.pdo ]); |
There was a problem hiding this comment.
Great work, thanks a lot! Just tested it locally and it just works!
Apart from the comments below, I have one question:
Quoting your example in the original post:
{
phpWithImagickWithoutFpm743 = phpWithImagick.override {
version = "7.4.3";
sha256 = "wVF7pJV4+y3MZMc6Ptx21PxQfEp6xjmYFYTMfTtMbRQ=";
fpmSupport = false;
};
}Why do we modify derivation's attributes here? Isn't this supposed to happen in .overrideAttrs?
|
If build php74.all not found dev package |
Rework withExtensions / buildEnv to handle currently enabled
extensions better and make them compatible with override. They now
accept a function with the named arguments enabled and all, where
enabled is a list of currently enabled extensions and all is the set
of all extensions. This gives us several nice properties:
- You always get the right version of the list of currently enabled
extensions
- Invocations chain
- It works well with overridden PHP packages - you always get the
correct versions of extensions
As a contrived example of what's possible, you can add ImageMagick,
then override the version and disable fpm, then disable cgi, and
lastly remove the zip extension like this:
{ pkgs ? (import <nixpkgs>) {} }:
with pkgs;
let
phpWithImagick = php74.withExtensions ({ all, enabled }: enabled ++ [ all.imagick ]);
phpWithImagickWithoutFpm743 = phpWithImagick.override {
version = "7.4.3";
sha256 = "wVF7pJV4+y3MZMc6Ptx21PxQfEp6xjmYFYTMfTtMbRQ=";
fpmSupport = false;
};
phpWithImagickWithoutFpmZip743 = phpWithImagickWithoutFpm743.withExtensions (
{ enabled, all }:
lib.filter (e: e != all.zip) enabled);
phpWithImagickWithoutFpmZipCgi743 = phpWithImagickWithoutFpmZip743.override {
cgiSupport = false;
};
in
phpWithImagickWithoutFpmZipCgi743
Since all options controlled by the config.php parameters can now be overridden directly, there's no reason to keep them around.
Some extensions depend on other extensions. Previously, these had to be added manually to the list of included extensions, or we got a cryptic error message pointing to strings-with-deps.nix, which wasn't very helpful. This makes sure all required extensions are included in the set from which textClosureList chooses its snippets.
talyz
force-pushed
the
php_buildenv_override
branch
from
fef1e19
to
f84c101
Compare
|
Exension Need zlib. |
|
Need build php-unit extensions with php-unit package: Package |
|
With this simple changes worked extensions: |
Because we're actually overriding the call to |
|
@Izorkin More extensive changes will have to be made to the |
There was a problem hiding this comment.
I'm very happy with these changes.
I know it breaks earlier use of withExtensions, but I really like the improvements of how it's used so I think it's worth that trade-off.
Unless people have strong opinions against this PR, I plan to merge it by tomorrow night (CEST) around 20:00.
That is about 21 hours from this comment.
I'll also give a go at dropping phpXYbase attributes from being dropped before merge since they aren't actually needed anymore.
Well, the API isn't even on a stable NixOS and the changes and usage are well-discussed and well-documented, so I fully agree here 👍 @talyz thanks a lot! |
This is useful if you need to access the dev output of the unwrapped derivation.
Since the introduction of php.unwrapped there's no real need for the phpXXbase attributes, so let's remove them to lessen potential confusion and clutter. Also update the docs to make it clear how to get hold of an unwrapped PHP if needed.
talyz
force-pushed
the
php_buildenv_override
branch
from
f84c101
to
5ba6ec0
Compare
Instead of using two different php packages in php-packages.nix, one wrapper and one unwrapped, simply use the wrapper and use its "unwrapped" attribute when necessary. Also, get rid of the packages and extensions attributes from the base package, since they're no longer needed.
talyz
force-pushed
the
php_buildenv_override
branch
from
5ba6ec0
to
2535cdf
Compare
ofborg
bot
requested review from
offlinehacker,
CMCDragonkai,
etu and
javaguirre
Motivation for this change
This does a few different, but related things to the PHP packaging:
It implements the override pattern for builds done with
withExtensions/buildEnv, so that we can, for example, writeand get a PHP package with the default extensions enabled, but PHP compiled without fpm support.
It reworks
withExtensions/buildEnvto handle currently enabled extensions better and make them compatible withoverride. They now accept a function with the named argumentsenabledandall, whereenabledis a list of currently enabled extensions andallis the set of all extensions. This gives us several nice properties:You always get the right version of the list of currently enabled extensions
Invocations chain
It works well with overridden PHP packages - you always get the correct versions of extensions
As a contrived example of what's possible, you can add ImageMagick, then override the version and disable fpm, then disable cgi, and lastly remove the
zipextension like this:It automatically includes all needed internal dependencies when specifying extensions. Whereas earlier you had to manually include all extensions your wanted extension depended on:
now specifying the one you want should suffice:
It deprecates the last
config.php.*options, since the affected parameters can now be set directly in the override.It makes the wrapped php build and the
php.iniavailable as attributes of a package built withphp.buildEnv/php.withExtensions.Note that a large portion of the diff is whitespace changes, so you'll probably want to view it with
Hide whitespace changeschecked.cc @aanderse @etu
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)