Allow php8

[fezfez]

No description provided.

[fezfez]

@PowerKiKi : error not related

[PowerKiKi]

Thanks for your contribution. I added a few comments that must be resolved before merging.

[mfn]

There is a bit of chicken-egg problem with the current attempt here:

• project has a composer.lock file, currently with phpunit 8.5
• 8.5 is not compatible with PHP8
  at least not yet, see Support PHPUnit 8.5 on PHP 8 sebastianbergmann/phpunit#4533
• hence the tests fails

Thus the initial idea to use ^8 || ^9 for phpunit makes sense but is torpedoed due to the composer.lock, otherwise the composer update step would be able to pick the right version itelf

I rarely see libraries having composer.lock exactly for this kind of reasons. Can it be removed?

It's only relevant for developing this library and is not respected when graphql-upload is used dependency. And in fact this circumstance seems to create a small issue here.

Just an observation, but maybe I got it backwards!

[PowerKiKi]

Thank you for mentioning the future compatibility of PHPUnit 8 and PHP 8. This is great news and will save a few headaches to many libs.

Can it be removed ?

No, it must not. I am aware that some libraries choose not to commit composer.lock file. But IMHO this has huge downsides, such as introducing unpredictability in CI environnements. This is is clearly illustrated in the first version of this PR where PHP-Cs-Fixer "magically" got upgraded and broke the CI.

While some communities are willing and have the resources to track a moving target, I am not and I don't. Keeping the lock is the cheapest solution.

only relevant for developing

As you noted yourself, this has no incidence whatsoever on consuming users, and is only here to help developers and CI not waste time solving "syncing" issue.

Interestingly PHPUnit fails here because somehow composer installs phpunit 4.4.5 (!) 🙄

[fezfez]

Interestingly PHPUnit fails here because somehow composer installs phpunit 4.4.5 (!) roll_eyes

Ok so maybe we need to do a composer update with prefer lowest ? (in an other branch)

[PowerKiKi]

I believe the solution I suggested is the simplest (until sebastianbergmann/phpunit#4533 is released).

[PowerKiKi]

I had a better look and turns out that PHP-Cs-Fixer does not support PHP 8 and prevent us from installing a recent PHPUnit (so we end up with PHPUnit 4.4.5). So until PHP-CS-Fixer/PHP-CS-Fixer#4702 is solved, we need to remove the lock and remove this dependency, but only for PHP 8. Then we can composer install as usual, and the rest will pass.

[fezfez]

@PowerKiKi working and rebased

[mfn]

\o/ ✅

[PowerKiKi]

Much better thank you. The last thing we can improve is always run composer install and never runs composer update. I was going to do it myself, but it seems you did not allow edit on your PR. So I guess you'll have to do this:

diff --git .github/workflows/main.yml .github/workflows/main.yml
index ac52d08..3306e9e 100644
--- .github/workflows/main.yml
+++ .github/workflows/main.yml
@@ -30,16 +30,14 @@ jobs:
           key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
           restore-keys: ${{ runner.os }}-composer-
 
-      - name: Install dependencies
-        run: composer install
-        if: matrix.php-version != '8.0'
-
       - name: Update dependencies
+        if: matrix.php-version == '8.0'
         run: |
           rm composer.lock
           composer remove --dev --no-update friendsofphp/php-cs-fixer
-          composer update
-        if: matrix.php-version == '8.0'
+
+      - name: Install dependencies
+        run: composer install
 
       - name: Setup problem matchers for PHP
         run: echo "::add-matcher::${{ runner.tool_cache }}/php.json"

[fezfez]

Much better thank you. The last thing we can improve is always run composer install and never runs composer update. I was going to do it myself, but it seems you did not allow edit on your PR. So I guess you'll have to do this:

diff --git .github/workflows/main.yml .github/workflows/main.yml
index ac52d08..3306e9e 100644
--- .github/workflows/main.yml
+++ .github/workflows/main.yml
@@ -30,16 +30,14 @@ jobs:
           key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
           restore-keys: ${{ runner.os }}-composer-
 
-      - name: Install dependencies
-        run: composer install
-        if: matrix.php-version != '8.0'
-
       - name: Update dependencies
+        if: matrix.php-version == '8.0'
         run: |
           rm composer.lock
           composer remove --dev --no-update friendsofphp/php-cs-fixer
-          composer update
-        if: matrix.php-version == '8.0'
+
+      - name: Install dependencies
+        run: composer install
 
       - name: Setup problem matchers for PHP
         run: echo "::add-matcher::${{ runner.tool_cache }}/php.json"

phpunit 4 is not compatible with php 8

[PowerKiKi]

Thank you for your contribution, I merged it with the additional modification I mentioned.