predis: Using local patches for dev dependencies breaks projects that use cweagans/composer-patches

Describe the bug

Follow-up of https://github.com/predis/predis/issues/632, which didn’t fully address this issue

Adding patches to composer.json for dev dependencies seems incorrect to me, for two reasons:

a) The local patch files won’t be found in the module, AFAIK composer-patches won’t look there b) Even if they would be found, the project is likely to use a different version of phpunit where the patches don’t apply.

To Reproduce Steps to reproduce the behavior:

composer init
composer require cweagans/composer-patches predis/predis
composer config extra.enable-patching true
composer require --dev phpunit/phpunit

  - Applying patches for phpunit/phpunit
    ./tests/phpunit_php7.patch (Fix PHP 7 compatibility)
   Could not apply patch! Skipping. The error was: The "./tests/phpunit_php7.patch" file could not be downloaded: failed to open stream: No such file or directory
    ./tests/phpunit_php8.patch (Fix PHP 8 compatibility)
   Could not apply patch! Skipping. The error was: The "./tests/phpunit_php8.patch" file could not be downloaded: failed to open stream: No such file or directory

Expected behavior A clear and concise description of what you expected to happen.

Versions (please complete the following information):

Predis: 1.1.x
PHP 7.4
Redis Server irrelevant
OS: Ubuntu 20.10

I don’t think it is possible to do this, not without cweagans/composer-patches introducing a patches-dev. My suggestion would be to switch to applying those patches with a script/travis commands (or whatever CI you use)

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 3
Comments: 21 (11 by maintainers)

Most upvoted comments

can confirm that patches-ignore works, but that’s not a solution, forcing everyone else to work around this is really confusing.

@cweagans making the patches public won’t help here, the patches here are for making old, unsupported versions of phpunit php7+ compatible. They won’t apply for anyone who is e.g. using Drupal 8 and most other projects as they are already on newer phpunit versions that don’t need those patches.

Having support for patches-dev would work, then local patches are fine. Without that, yes, just add the patches in CI. not sure about the advantages/disadvantages of that other patches package.

@roborourke I assume the patches are used because 1.x also supports older php versions which in turn aren’t supported by new phpunit versions which is why it uses that hack to test it on both old and new php versions. So it’s not “fixed” in v2, it simply doesn’t need the patches there as the phpunit version is newer.

Berdir on Sep 30, 2020

@smustgrave: Yes, I’ll tag a new release as soon as I get Travis working.

tillkruss on Apr 4, 2021

Local patches really shouldn’t be used by projects that can be pulled in as a dependency in the first place. I recommend either applying those patches in your travis job manually or putting the patches somewhere public where they can be downloaded (maybe Gist? worked pretty well like this: https://github.com/cweagans/mcrypt-polyfill/blob/d261945011d06273fa3221e68cb2b7e75a40b3ee/composer.json#L30).

I’m reasonably sure patches-ignore is working, but if you can reliably reproduce a case where it’s not, definitely open an issue with the info and maybe we can get it working.

cweagans on Sep 30, 2020

@fgm: That looks good.

@cweagans: Do you have any suggestions with this?

tillkruss on Sep 18, 2020