psysh: 0.11.21 -> 0.12.3 + build-support/php: update #308608
Conversation
TODO: Is it the best way to do that?
Also update the `composer` derivation
| @@ -62,7 +62,7 @@ let | |||
| runHook postInstallCheck | |||
| ''; | |||
|
|
|||
| composerRepository = phpDrv.mkComposerRepository { | |||
| composerRepository = phpDrv.mkComposerRepository ({ | |||
There was a problem hiding this comment.
I would use previousAttrs.composerRepository or phpDrv.mkComposerRepository …. This looks more like composerRepositoryArgs and having different behaviour when passing it to buildComposerProject and to overrideAttrs would be confusing.
| @@ -23,6 +23,10 @@ composerInstallConfigureHook() { | |||
| cp "$composerLock" composer.lock | |||
| fi | |||
|
|
|||
| chmod +w composer.{json,lock} | |||
| cp ${composerRepository}/composer.{json,lock} . | |||
There was a problem hiding this comment.
This makes the two lock checks below redundant.
| @@ -40,7 +40,7 @@ php.buildComposerProject (finalAttrs: { | |||
| --prefix PATH : ${lib.makeBinPath [ _7zz cacert curl git unzip xz ]} | |||
| ''; | |||
|
|
|||
| vendorHash = "sha256-dNNV9fTyGyRoGeDV/vBjn0aMgkaUMsrKQv5AOoiYokQ="; | |||
| vendorHash = "sha256-4s912b04PTPbK9+suni9uJ2Cx4qDb8CkMLy6y25a1eQ="; | |||
There was a problem hiding this comment.
Yeah, this is bad. If we want to not break BC for outside consumers, we would need to introduce mkComposerRepository2 and buildComposerProject2, and deprecate the old one.
| setComposeRootVersion | ||
| composer config platform.php 7.4 | ||
| composer require --no-update symfony/polyfill-iconv symfony/polyfill-mbstring | ||
| composer require --no-update --dev roave/security-advisories:dev-latest |
There was a problem hiding this comment.
Actually, this is not reproducible, is it? The hash would change over time since the lockfile is included in the output.
There was a problem hiding this comment.
No, this is fully reproducible actually, tested with --rebuild.
There was a problem hiding this comment.
What if roave/security-advisories releases a new version?
| vendorHash = ""; | ||
|
|
||
| composerRepository = { | ||
| preBuild = '' |
There was a problem hiding this comment.
Hmm, maybe we should use make build/psysh then?
There was a problem hiding this comment.
make ? What does make has anything to do in here?
There was a problem hiding this comment.
You are mentioning it in the commit message.
| @@ -16,7 +16,18 @@ php.buildComposerProject (finalAttrs: { | |||
| url = "https://github.com/bobthecow/psysh/releases/download/v${finalAttrs.version}/composer-v${finalAttrs.version}.lock"; | |||
| hash = "sha256-ur6mzla3uXeFL6aEHAPdpxGdvcgzOgTLW/CKPbNqeCg="; | |||
| }; | |||
| vendorHash = ""; | |||
|
|
|||
| composerRepository = { | |||
There was a problem hiding this comment.
Yeah, this is confusing. I would expect this attribute to be passed the result of mkcomposerRepository.
If you apply the suggested change to buildComposerProjectOverride, you should be able to use composerRepository = previousAttrs.composerRepository.overrideAttrs { preBuild = …; };
There was a problem hiding this comment.
Yeah I'm not yet at ease with this.... I will do the change.
|
superseded by #308059 |
Some projects in PHP are running
composercommands on their CI before building a release.Even if the project's
composer.lockgets shipped along the PHAR release, runningcomposer validatereturns and error (bobthecow/psysh#796).This PR allows users to run a
preBuildhook so that extra composer commands can be triggered just before building the composer repository (017b29d)The downside of this is that all the PHP hashes needs to be regenerated. I don't know if there's a better way to do this yet.
Description of changes
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.