 | Nix PHP | 75 Members |
| A room for PHP developers running on Nix | 21 Servers |
| Nix PHP | 75 Members |
| A room for PHP developers running on Nix | 21 Servers |
| Sender | Message | Time |
|---|---|---|
| 22 May 2023 | ||
 Pol | I've been testing that thing the whole weekend, and I improved it here and there as you can see in the PR. | 06:59:19 |
| Pol | There is only ONE single issue with that method. | 06:59:31 |
| Pol | If by any change you have post-update-cmd in composer.json like in Drupal: https://github.com/drupal/drupal/blob/11.x/composer.json#L111This is going to fail. | 07:00:30 |
| Pol | Why? Because as far as I can see, the update command is expected to be run AFTER the install command and the autoloader needs to be loaded so the update command can run the command properly. | 07:01:14 |
| Pol | But in our case, we are running the update command BEFORE the install command: https://github.com/NixOS/nixpkgs/blob/58fed5f92032c4e51286d08a76d9d2bab5443ecb/pkgs/build-support/php/hooks/composer-install-hook.sh#L52 | 07:02:18 |
| Pol | So today I'm going to test stuff and see how we can get rid of that composer update command before the install. If you have any clue, please let me know. | 07:02:50 |
| Pol | Oh I think I fixed the Drupal issue ! | 08:28:50 |
| Pol | Confirmed! Drupal issue fixed :) | 08:31:55 |
| Pol | The fix has been committed to drupol/composer-local-repo-plugin: https://github.com/drupol/composer-local-repo-plugin/commit/9e1144ba0f3e98442bbfc955edd148be281c4148 and in the PR: https://github.com/NixOS/nixpkgs/compare/58fed5f92032c4e51286d08a76d9d2bab5443ecb..2d49300918ee738fa8f2e9d086647539493bb879 | 08:37:56 |
| Pol | Jan Tojnar: FYI, I think there are rooms for improvements in fossar/composition-c4. I'm trying to propose a PR, but I have the feeling that I need to move this aside or else I won't be doing anything else today... and I'm at work :D | 10:09:39 |
| Pol | Basically, the idea is that Composer is able to download a package from dist or source attribute (see relevant part of the code https://github.com/composer/composer/blob/main/src/Composer/Downloader/DownloadManager.php#L143). In fossar/composition-c4, we only handle source. I guess it would not be complicated to fix this. | 10:42:54 |
| Pol | * Basically, the idea is that Composer is able to download a package from dist or source attribute (see relevant part of the code https://github.com/composer/composer/blob/main/src/Composer/Downloader/DownloadManager.php#L143). In fossar/composition-c4, we only handle source. I guess it would not be complicated to fix this. I just don't have enough knowledge in Nix to do it... I'm still trying :D | 11:45:27 |
| Pol | I created the relevant issue to keep track of this: https://github.com/fossar/composition-c4/pull/6 | 11:57:44 |
| Pol | WDYT of this? I just pushed a commit to refactor the script and add a little bit of documentation: https://github.com/NixOS/nixpkgs/blob/7f2c754d1daeeca5eef173b7a2ec6d305522857d/pkgs/build-support/php/hooks/composer-setup-hook.sh | 14:03:28 |
| Pol | The other hook is also very clean now: https://github.com/NixOS/nixpkgs/blob/7f2c754d1daeeca5eef173b7a2ec6d305522857d/pkgs/build-support/php/hooks/composer-install-hook.sh | 14:05:01 |
| Pol | * WDYT of this? I just pushed a commit to refactor the setup hook script and add a little bit of documentation: https://github.com/NixOS/nixpkgs/blob/7f2c754d1daeeca5eef173b7a2ec6d305522857d/pkgs/build-support/php/hooks/composer-setup-hook.sh | 14:09:41 |
 Jan Tojnar | In reply to @drupol:matrix.orgYeah, I think disabling scripts is proper solution – the only reason we are running composer update is to update the sources in the lockfile. | 16:04:26 |
| Jan Tojnar | In reply to @drupol:matrix.orgI do not see the motivation for https://github.com/drupol/composer-local-repo-plugin/commit/9e1144ba0f3e98442bbfc955edd148be281c4148 | 16:05:34 |
| Jan Tojnar | In reply to @drupol:matrix.org I am only providing a single variant since otherwise I can only use | 16:10:05 |
| Jan Tojnar | alternately, we could provide both source and dist generated from a single source but that is pointless since composer will always fall back on what is available, even when using --prefer-source/--prefer-dist flags | 16:12:28 |
| Pol | In reply to @jtojnar:matrix.orgIn Drupal, you have these kind of things: https://github.com/drupal/drupal/blob/11.x/composer.json#L124 | 16:16:15 |
| Pol | Composer repos inside the drupal sources. | 16:16:24 |
| Pol | Without this change: https://github.com/drupol/composer-local-repo-plugin/commit/9e1144ba0f3e98442bbfc955edd148be281c4148 it simply doesn't work | 16:16:51 |
| Pol | By the way, I just forced pushed on the branch, I made this new improvement: https://github.com/NixOS/nixpkgs/pull/232450/files#diff-d6938cf1d292c64b6f1726b28397358dfa80ce78c9cfaaf54591acf09c36a04eR43 | 16:17:48 |
| Pol | I'm creating a derivation for building the Composer Home directory containing the Composer plugin. | 16:18:04 |
| Pol | The setup hook is even more clean: https://github.com/NixOS/nixpkgs/blob/240a1b5003a5a6d6ef1ad1786033288941c69c63/pkgs/build-support/php/hooks/composer-setup-hook.sh | 16:19:18 |
| Jan Tojnar | In reply to @drupol:matrix.orgone thing I do not like is that the file contains functions both for fetchComposerDeps and buildComposerProject | 16:24:19 |
| Jan Tojnar | I would probably move composerSetupBuildHook directly into fetchComposerDeps nix file, it should not really be used outside anyway | 16:24:48 |
| Jan Tojnar | and we will want to minimize API surface so that it is harder to misuse and easier to change in the future | 16:25:26 |
| Pol | True | 16:28:17 |