| 5 May 2024 |
Pol | The overriding part is:
composerRepository = finalAttrs.composerRepository.overrideAttrs (oldAttrs: {
preBuild = ''
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
'';
});
| 14:38:07 |
Pol | When I remove it, I don't have the infinite recursion. something must be wrong somewhere, I can't figure it out. | 14:38:33 |
Jan Tojnar | Pol: right, you finalAttrs.composerRepository in physh ends up pointing to the composerRepository attribute in buildComposerProjectOverride, and the previousAttrs.composerRepository in that points to the composerRepository in physh | 14:52:20 |
Jan Tojnar | * Pol: right, the finalAttrs.composerRepository in physh ends up pointing to the composerRepository attribute in buildComposerProjectOverride, and the previousAttrs.composerRepository in that points to the composerRepository in physh | 14:52:30 |
Pol | Let me re-read that slowly. | 14:52:43 |
Pol | OK got it. | 14:53:04 |
Pol | What should we do to fix that? | 14:53:13 |
Pol | This is the part where I'm totally insecure in the PHP Composer Builder | 14:55:27 |
Jan Tojnar | I guess the cleanest solution would be just using composerRepository = mkComposerRepositoryNext { … }; when more advanced usage is needed | 15:01:06 |
Pol | Argh | 15:02:03 |
Pol | In reply to @drupol:matrix.org
The overriding part is:
composerRepository = finalAttrs.composerRepository.overrideAttrs (oldAttrs: {
preBuild = ''
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's no way to override it cleanly like that? | 15:02:31 |
Jan Tojnar | You could do (php.buildComposerProjectNext (finalAttrs: {})).overrideAttrs (_finalAttrs: prevAttrs: {composerRepository = prevAttrs.composerRepository.overrideAttrs (oldAttrs: { }); } but that is ugly too | 15:04:33 |
Pol | Oh boy yeah... | 15:05:02 |
Jan Tojnar | the issue is that the attribute set passed to buildComposerProjectNext is level 0, and overrides can only refer to either level n - 1 or the final level | 15:05:54 |
Jan Tojnar | * the issue is that the attribute set passed to buildComposerProjectNext is level 0, and overrides can only refer to either level n - 1 or the final level | 15:06:04 |
Jan Tojnar | and final level will not work since that would be a cycle | 15:06:17 |
Pol | Can we do some nice things in the new version of the PHP Composer Builder ? I would like to sort those things. | 15:06:56 |
Jan Tojnar | you could maybe add extra composerRepositoryArgs argument that could be passed in level 0 but there are issues with that as well | 15:08:10 |
Pol | Indeed, I've been thinking about that... but yeah it's ugly :( | 15:08:29 |
Pol | I will check other builders how they do | 15:10:06 |
Jan Tojnar |
- it would be confusing if you added that and e.g.
composerNoDev – which should take precedence, especially with multiple levels of overrides
- you would have to filter it out of arguments passed to
mkDerivation since attribute sets are not allowed, but then it would disappear from the fix point (maybe it would work in passthru, not sure)
| 15:10:09 |
Jan Tojnar | other builders did not really bother to support or predated the finalAttrs pattern, buildComposerProject is unique here | 15:11:24 |
Pol | Yeah I noticed :S | 15:11:39 |
Pol | hence, the added complexity to override some stuff | 15:11:49 |
Jan Tojnar | well, there is no shame in leaving advanced use cases to explicit mkComposerRepository | 15:13:11 |
Pol | I will see the things to do to override the preBuild step. | 15:13:56 |
Jan Tojnar | btw, might be a good idea to clone the old infrastructure first and then apply the changes in a new commit so the differences are easier to review. Also maybe use 2 suffix instead of Next so that further evolution has space to grow into | 15:14:56 |
Pol | Okidocky | 15:16:26 |
Pol | Have you seen, that version get rid of the composer plugin :) | 15:19:47 |
Jan Tojnar | I was to lazy to diff it against the old one, hence the request for a separate commit :) | 15:25:14 |