phpro / grumphp-shim

This repository provides easy way to install GrumPHP without the risk of conflicting dependencies.
MIT License
22 stars 3 forks source link

chore: fix `phar.composer.lock` #24

Closed drupol closed 11 months ago

drupol commented 11 months ago

Hello,

This PR relate to #23 and fix the composer.lock validation. There's no more discrepancies between composer.json and composer.lock now.

Built with PHP 8.2.11 and Composer 2.6.5

veewee commented 11 months ago

The file is (and should be) autogenerated. As mentioned in the issue, I think the issue there is that the composer.json of the grumphp directory you are building should contain the configuration for php 8.1:

composer config platform.php 8.1'

I don't think the plugin-api-version will cause issues, cause it's the version of composer that built the lock file. But if that's an issue, we'll have to pin the composer version somehow during builds (which I'm not really looking forward to).

drupol commented 11 months ago

Ok will try with that.

drupol commented 11 months ago

Looks like it's still creating issues.

❯ cp phar.composer.lock composer.lock
❯ composer config platform.php 8.1
~/C/t/grumphp > v2.x +4 -1 [!?] > php@8.2.11 ❯ php ^C                                                                                                                   192.168.2.188 |  | ❄
❯ composer validate
./composer.json is valid but your composer.lock has some errors
# Lock file errors
- The lock file is not up to date with the latest changes in composer.json, it is recommended that you run `composer update` or `composer update <package name>`.
❯ composer update --lock
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Writing lock file
Installing dependencies from lock file (including require-dev)
Nothing to install, update or remove
Generating autoload files
75 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
> GrumPHP\Composer\DevelopmentIntegrator::integrate
Watch out! GrumPHP is sniffing your commits!

Found 1 security vulnerability advisory affecting 1 package.
Run "composer audit" for a full list of advisories.
~/C/t/grumphp > v2.x +4 -1 [!?] > php@8.2.11 ❯ cp ^C                                                                                                          192.168.2.188 |  | 1s245ms | ❄
❯ diff phar.composer.lock composer.lock
7c7
<     "content-hash": "8a069c630e6ddbc4475db9a992430539",
---
>     "content-hash": "0474062650b24a22c63007631cf35f1e",
7093c7093
<     "plugin-api-version": "2.3.0"
---
>     "plugin-api-version": "2.6.0"
~/C/t/grumphp > v2.x +4 -1 [!?] > php@8.2.11 ✘  

But OK, I'm a bit out of ideas on how to get this things working right now, Grumphp 2 will wait a bit before being packaged in Nix. I pushed this PR, we'll see what the other PHP maintainers will think.

veewee commented 11 months ago

As you can see in the logs, only the composer API version differs now. Meaning you'll have to use the exact same composer version to validate the file that infused to build the PHAR or somehow disable that specific line during the validate command.

Not sure either how we can do this tbh

drupol commented 11 months ago

I could also just wait for the next point release, hopefully your composer version will be updated :D

veewee commented 11 months ago

It might or it might not be :) In any case, this way it won't be reproducible in time either. Can imagine you'll bump into similar issues on other packages you are trying to ship as well.

It might make sense to ask the composer people how to deal with it?

drupol commented 11 months ago

It might or it might not be :)

OK ! But what's preventing you to automatize generation of the composer.lock using the latest Composer version in your workflow?

We are always trying to use the latest stable version of Composer, we cannot use an arbitrary version of composer and/or maintain all the versions in the repository. This is not only specific to Nix, but in every distributions. Composer being the most important tool here.

In any case, this way it won't be reproducible in time either. Can imagine you'll bump into similar issues on other packages you are trying to ship as well.

So far this problem only happen with Grumphp. I doubt there will be similar issues with others... (yet?).

I could fix the issue by providing patches, but I would have preferred fixing the problem within this repo than polluting nixpkgs with patches that would be removed at some point.

veewee commented 11 months ago

I get that. From my point of view , there is nothing wrong with the provided lock file though.

I could upgrade composer before building the phar, but if you build your package a week later - and a new composer version bumps the API version in the lock file, your build will fail again.

I'm rather suggesting looking into a more stable build process from your side, since that's the more time-proof solution.

drupol commented 11 months ago

From my point of view , there is nothing wrong with the provided lock file though.

True if you use a version of Composer < 2.6.0. With the current version of Composer, the validation is failing. So to me, there's an issue with older versions of composer. Of course this is not critical, and we can still run composer install it will work, but in automated processes where we would like to integrate a composer validate in between, this is an issue. That's the reason why I started to work on a PR where we could skip this validation process at https://github.com/NixOS/nixpkgs/pull/262388.

I could upgrade composer before building the phar, but if you build your package a week later - and a new composer version bumps the API version in the lock file, your build will fail again.

No, we can use different Composer version, but if the attribute composer-plugin-api change we will have issues. It has been changed 4 months ago, at the release of Composer 2.6.0...

But allez, no worries, I will just wait an update of Grumphp and in the meantime still use 1.15 or just provide my own composer.lock.

As always, thanks for your availability, even if we haven't found the best compromise.

drupol commented 11 months ago

I fixed the issue in Nix by providing 2 very small patches, find it here: https://github.com/NixOS/nixpkgs/pull/262558

veewee commented 11 months ago

Allright nice.

In any case, I'll update the script in order to download the latest composer before building. I doubt it will solve all issues in the long run, but we'll see what kind of lemons life will throw at us afterwards :)

Probably something for next friday. I'll keep you posted.

drupol commented 11 months ago

Thank you mate, ping me when you need me to check it out.