phpro / grumphp-shim

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

`composer.lock` validation failing #23

Closed drupol closed 11 months ago

drupol commented 1 year ago

Hello,

I'm trying to update the Nix package with the new Composer builder, and it's failing at validating the composer.lock file.

Here's how to reproduce it locally:

❯ cd ~/Code/tmp/grumphp
❯ git co v2.1.0
Note: switching to 'v2.1.0'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at b190443f 2.1.0 release
❯ wget https://github.com/phpro/grumphp-shim/raw/v2.1.0/phar.composer.lock
--2023-10-05 22:51:16--  https://github.com/phpro/grumphp-shim/raw/v2.1.0/phar.composer.lock
Resolving github.com (github.com)... 140.82.121.3
Connecting to github.com (github.com)|140.82.121.3|:443... connected.
HTTP request sent, awaiting response... 302 Found
Location: https://raw.githubusercontent.com/phpro/grumphp-shim/v2.1.0/phar.composer.lock [following]
--2023-10-05 22:51:16--  https://raw.githubusercontent.com/phpro/grumphp-shim/v2.1.0/phar.composer.lock
Resolving raw.githubusercontent.com (raw.githubusercontent.com)... 185.199.109.133, 185.199.108.133, 185.199.111.133, ...
Connecting to raw.githubusercontent.com (raw.githubusercontent.com)|185.199.109.133|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 255034 (249K) [text/plain]
Saving to: ‘phar.composer.lock’

phar.composer.lock                              100%[====================================================================================================>] 249.06K  --.-KB/s    in 0.06s   

2023-10-05 22:51:16 (4.26 MB/s) - ‘phar.composer.lock’ saved [255034/255034]

❯ mv phar.composer.lock composer.lock
❯ 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>`.
~/C/t/grumphp > b190443@v2.1.0 > php@8.2.11 ✘

Here's the diff of the updated composer.lock:

From 6053048951b7cef38c9af346069a601f023a50ca Mon Sep 17 00:00:00 2001
From: Pol Dellaiera <pol.dellaiera@protonmail.com>
Date: Fri, 6 Oct 2023 08:48:24 +0200
Subject: [PATCH] composer.lock

---
 composer.lock | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/composer.lock b/composer.lock
index 96b692c3..46c38857 100644
--- a/composer.lock
+++ b/composer.lock
@@ -4,7 +4,7 @@
         "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
         "This file is @generated automatically"
     ],
-    "content-hash": "8a069c630e6ddbc4475db9a992430539",
+    "content-hash": "44ce311f8f7cdc1321afb8c4f4dc346f",
     "packages": [
         {
             "name": "amphp/amp",
@@ -7087,8 +7087,5 @@
         "composer-plugin-api": "~2.0"
     },
     "platform-dev": [],
-    "platform-overrides": {
-        "php": "8.1"
-    },
-    "plugin-api-version": "2.3.0"
+    "plugin-api-version": "2.6.0"
 }
-- 
2.42.0

Do you think you could have some time to look at it?

Related PRs:

veewee commented 1 year ago

Oh, I think I accidentally built it with php 8.2...

Will have to look at it when I find more time to see how I can fix this because it's already tagged.

Also I'll have to think about how I can avoid this issue in the future probably :)

drupol commented 1 year ago

Great. Looking forward packaging this in Nix then... Thank you mate!

veewee commented 1 year ago

Hello,

Just took a closer look. It seems like the phar got compiled correctly and the lock file is valid given the way it has been built. Can you give me some more details on how to reproduce the error on my system? A list of commands to execute manually would be nice!

There are 2 things in your diff:

Currently I'm building this stuff locally. It might make sense to either do it in a pipeline with stable dependencies or maybe through a flake to make sure it gets compiled consistently. However, I don't think the issue here is the provided lock file.

drupol commented 1 year ago

Right, let me retry the procedure with PHP 8.1.

drupol commented 1 year ago

I can reproduce the issue with PHP 8.1, see this quick screencast: https://asciinema.org/a/1vlHjh3G6RIiDQAk5nr66KQNY

veewee commented 1 year ago

The validate problem might be related to the platform.php configuration we set when building the phar:

https://github.com/phpro/grumphp-shim/blob/36418a55fe97b1b6b12272b31ae261f1c66a6b48/Makefile#L43

You might need to run this command on the grumphp directory as well, so that the composer.json file has the correct configuration as well. Which results in this lock:

https://github.com/phpro/grumphp-shim/blob/36418a55fe97b1b6b12272b31ae261f1c66a6b48/phar.composer.lock#L7090-L7092

The API version of composer plugins is set because you are using a more recent composer version as I do. Yet I don't think validate complains about this.

veewee commented 1 year ago

@drupol Can you revalidate on tag v2.2.0?

https://github.com/phpro/grumphp-shim/blob/b75325940c3e923d4f47ae0aad7b52136be1ace6/phar.composer.lock#L7090-L7094

drupol commented 1 year ago

Sadly nope :( https://asciinema.org/a/gldqtNIjSwXv1hMJOVB7VRdeJ

Hint 1: How about committing platform.php to the lowest supported PHP version in composer.json? Hint 2: how about just exposing the composer.lock available as a release artifact (so you don't need to commit it anymore) Hint 3: ext-json is not more mandatory in composer.json, it's enabled by default in PHP >= 8.

veewee commented 1 year ago

Can you figure out exactly what is not ok?

Hint 1 is not possible: I cannot set that config on the main repo, cause it's installable on al versions and otherwise ci will start fetching invalid resources maybe you could use the exact same composer command for setting the php config instead of patching?

Hint 2 is possible but doesnt fix the issue right?

Hint 3 not sure if you can install php without Json, but that shouldn't cause any issues right?

I'm lacking time to doing deep dives into this. All the tools you need to build the lock a f phar are in this repo. Feel free to see if you can find a way to do this from within this repo.

veewee commented 11 months ago

Hello @drupol,

Thanks for guiding me through this at SymfonyCon. I'm now shipping the composer.lock file at the GrumPHP repository as well. This PHAR is built based on that lock file (with some additional "optional" dependencies). The resulting phar lock file is still being committed to this repo, so it can be investigated as well.

This should make it reproducable from your end:

https://github.com/phpro/grumphp/releases/tag/v2.4.0

drupol commented 11 months ago

WOW, that's a breakthrough !

Thank you Toon !!!!

veewee commented 11 months ago

Oh yes, forgot to mention:

If you want the phparser task to work, you'll need to bake in composer require 'nikic/php-parser:~4.0' --update-no-dev --no-interaction. It's an optional dependency for grumphp for that task, but it breaks if it's not baked in because of php-scoper.