phpbench: init at 1.2.14

[drupol]

Description of changes

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[drupol]

@dantleech Would you be OK to let the composer.lock file live in the phpbench repository ?

[dantleech]

In theory I'm OK with it, the build might need to be updated

[drupol]

In theory I'm OK with it, the build might need to be updated

Excellent news ! I'll wait for you to publish the new update, so I don't push to git files that will be removed at a later point... and put this PR in draft then.

[dantleech]

Well, it probably won't happen soon, as I should probably automate the generation of the lock file as otherwise it's "works on my machine"? (I.e. you'll pin the PHP version to something, but that won't always correlate to the versionvthta I generate the lock file). But PHPBench has a PHAR - is that a worse option?

Also another consideration is that PHPBench will spawn a PHP process to benchmark the code, but that PHP process should be the one in the developers env. (I.e. wjth same versuon, extensions, etc). so unsure how this will work with NixOS.

[etu]

@dantleech Ideally I'd say that each release should come with a composer.lock that is considered a blessed state of that release. Otherwise there's no guarantees that anyone that downloads the tag of that release can reproduce the same composer dependencies. Not providing a composer.lock is like telling everyone to run composer update to get the deps. Then you may get incompatible dependencies depending on upstream, what if they broke something in a minor or patch version?

For the other consideration, this means that we'd have to provide phpbench for each PHP version, with extensions and all. This is something we easily can do. We just have to move it from the phpbench attribute to a php.packages.phpbench attribute because then the user can select which php version to use and also use a build environment for that to customize it with extensions and configs.

[dantleech]

Otherwise there's no guarantees that anyone that downloads the tag of that release can reproduce the same composer dependencies

The PHAR file already achieves this and is auto-published on release. It's the same difference but if I want a composer.lock then I'll need to have the CI build commit the lock file back to the repo (or build the lock file on my local machine where it is possible that I introduce environment-specific constraints, or I just plain forget to update it or similar).

we'd have to provide phpbench for each PHP version, with extensions and all

I'm sceptical of this approach vs. letting composer handle PHP tooling for the PHP project though (if necessary using composer-bin plugin or similar) - but I haven't quite gotten round to using NixOS 100% for local dev envs, I guess if you can configure f.e. a flake.nix for your project and specify the develop env, and customise the PHPBench install etc it can be pretty useful (and possibly allow you to have multiple PHPBench installs with different environments?).

[etu]

Otherwise there's no guarantees that anyone that downloads the tag of that release can reproduce the same composer dependencies

The PHAR file already achieves this and is auto-published on release.

Correct, however, nixpkgs always opt for building packages from source. Using the phar isn't building it from source. And since we now have our new fancy builder for composer projects pioneered by @drupol. We kinda want to migrate to using that since it follows best practices for how packages are built.

It's the same difference but if I want a composer.lock then I'll need to have the CI build commit the lock file back to the repo (or build the lock file on my local machine where it is possible that I introduce environment-specific constraints, or I just plain forget to update it or similar).

I would say it's best practices to have it commited to the repository at all times, how else would you synchronize exact versions of dependencies between different contributors to the project or do you expect to just use "latest within version constraint"? That's not reproducible and error-prone due to reasons I've already mentioned.

This is not about CI.

However, this is up to you and I'm not here to run your project for you 🙂

we'd have to provide phpbench for each PHP version, with extensions and all

I'm sceptical of this approach vs. letting composer handle PHP tooling for the PHP project though (if necessary using composer-bin plugin or similar) - but I haven't quite gotten round to using NixOS 100% for local dev envs,

I'm not talking about composer dependencies, I'm talking about PHP extensions, such as pdo, pdo_mysql, imagick, xdebug and so on.

The only way to manage extensions for PHP in nix is to use our buildEnv (withExtensions is a shorthand for just extensions), it's documented in full here: https://nixos.org/manual/nixpkgs/unstable/#sec-php

Here's a short example:

PHP in nix doesn't come by default with imagick, if you want imagick you can get it by having this extension of PHP:

{
  # For specific versions
  php81 = php81.withExtensions ({ enabled, all }: enabled ++ [ all.imagick ]);
  php82 = php82.withExtensions ({ enabled, all }: enabled ++ [ all.imagick ]);
  php83 = php83.withExtensions ({ enabled, all }: enabled ++ [ all.imagick ]);

  # For default version (currently 8.2)
  php = php.withExtensions ({ enabled, all }: enabled ++ [ all.imagick ]);

  # Get composer with imagick and redis installed for PHP 8.1
  composer-imagick-redis = (php81.withExtensions ({ enabled, all }:
    enabled ++ [ all.imagick all.redis ])).packages.composer;
}

We went through a great amount of effort a couple of years back to build every built in PHP extension as a separate package so we can binary cache it per package per php version. This makes it possible to have a super minimal PHP version as base and then compose a package with any combination of extensions within seconds since there's no compile time, just a config file that needs to be built.

So I wouldn't worry about having it as a PHP version dependent package, that's how we package PHP programs that are version dependent.

I guess if you can configure f.e. a flake.nix for your project and specify the develop env, and customise the PHPBench install etc it can be pretty useful (and possibly allow you to have multiple PHPBench installs with different environments?).

This is exactly right, that's why we did all this work 🙂

[dantleech]

thanks for the explanations!

I would say it's best practices to have it commited to the repository at all times

the problem becomes how to ensure that the .lock file is generated in an enviornment that guarantees it will work on all supported versions of PHP? if I "just commit the lock file" it will be, by default, based on whatever constriants are in my environment (i.e. my PHP version if nothing else). if I commit the lock file that was updated PHP 8.2 and NixOS packages it with PHP 8.0 that's not necessarily going to work as it could include newer versions of packages that depend on 8.2.

Depending on the maintainer or the individual contributors to remember to generate the lock file correctly is error prone, so it needs to be automated, which is ... probably fine but requires additional effort.

Saying that - I could probably run the test suite with the locked-down version of the package in all PHP versions, but that would increase the build time significantly (it already runs all the tests in multiple versions of PHP with min and max deps). Note that for right or wrong, it's intended that PHPBench supports a wide range of constraints.

In summary - I think it's more effort than just committing the lock file.

This is exactly right, that's why we did all this work

It's amazing stuff 🎉

[etu]

the problem becomes how to ensure that the .lock file is generated in an enviornment that guarantees it will work on all supported versions of PHP?

You as a project will have to decide which versions of PHP you support, this can be defined in your composer.json. This is something that I see that you already have done: https://github.com/phpbench/phpbench/blob/master/composer.json#L21, this is defined to match 7.4.* and 8.. (https://semver.madewithlove.com/?package=php&constraint=%5E7.4+%7C%7C+%5E8.0).

if I "just commit the lock file" it will be, by default, based on whatever constriants are in my environment (i.e. my PHP version if nothing else).

Now we're getting into the weeds here and I'm pretty confident that that's not how it works... but now I'm not 100% sure. I'm pretty sure that a composer update with PHP 8.1 and PHP 8.2 will install the exact same dependencies even if some dependency has newer versions that may require 8.2. But I'm... again. Not a 100% confident here.

if I commit the lock file that was updated PHP 8.2 and NixOS packages it with PHP 8.0 that's not necessarily going to work as it could include newer versions of packages that depend on 8.2.

This is something we can handle in nixpkgs though, and already have cases for.

Sure, we do generate the phpXY.packages.* from the same file so... as a base, every package is automatically added for every supported PHP version. However, we do acknowledge that this may not be the case in practice.

So what we can do is like we do with the blackfire extension that doesn't work with PHP 8.3.0RC1, we just put a condition to exclude it depending on PHP version: https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/php-packages.nix#L228-L230

We can do the same for tools/packages 🙂 So if a tool is only supporting certain versions of PHP we can accommodate for that. 🙂

Depending on the maintainer or the individual contributors to remember to generate the lock file correctly is error prone, so it needs to be automated, which is ... probably fine but requires additional effort.

My experience with composer for the past 10 years and the lock files is that it's fine to use lock files even from older versions (not sure about newer) of composer. I think, like once they have changed the lock-file format and it didn't break use with newer versions of composer. So it's in my experience very reliable. They have changed datetime formats and such as well some times. However it's been minimal and fine.

I'm not here to convince you to change your development practices though.

In summary - I think it's more effort than just committing the lock file.

Do phpbench have some subcommand that can be used to test if it's happy with it's environment it's running in? Like make sure dependencies loads and such?

I haven't used phpbench myself, but I'm thinking if something exists along the lines phpbench diagnose (similar to composer diagnose)?

Because then we, in nixpkgs, could automate the update of our composer.lock when we update to the latest version and we could add a testPhase to the package that gets executed with every build. Then we can ensure the package works with every version that we provide, if it then fails in our CI we can catch it and look into it 🙂

[dantleech]

Now we're getting into the weeds here

So I think it makes sense now. By default composer will use the current PHP interpreters version to resolve packages, but you can override this with by declaring the platform, so in theory that should produce reproducible lock file, in this case I guess I should set it to 7.4 (or give the package a long-overdue bump to PHP 8.0 or 8.1). I should probably also set an upper limit on the supported PHP version (i.e. <8.3)

[drupol]

@dantleech The same kind of request has been made for the Grumphp project, follow the interesting discussion at phpro/grumphp#1095

[drupol]

@dantleech ^^ :)