patches ignore enhancements

[wilroboly]

First and foremost, thank you for all the hard work done on this plugin. I use it constantly.

Over the course of the last few years, I've come across a few use cases where this plugin has proven to be a challenge when leveraging the patches-ignore of sub-packages. What I mean is, it doesn't. It will load up all the patches and apply them, but unless the patches-ignore are repeated in the main composer.json, the sub-packages patches-ignore are basically ignored.

As such, I have reviewed the code and made a few changes to it to allow for some accommodations that I think might benefit myself and others.

Let me describe a scenario where I find the patch I've proposed quite useful.

I have a starter project composer package which acts like the parent collector for other packages. I want this top level composer.json file to control versions but I do not wish it to contain or maintain patches for other packages. Rather, I would like those specific packages to manage their own requirements. Think of this:

• Project composer.json requires:
  • drupalwxt/wxt -- it has a ton of patches and patches ignore from its requirements
  • my specific drupal profile -- it has its own patches and patches ignore (not necessarily overriding anything in the wxt package, but maybe - shouldn't matter)

I want to do either a composer create-project myproject or a composer up on the very same and should things change upstream on any of those packages I leverage, I want those patches and patches-ignore to be applied.

As such, I've come up with some workarounds and a way to consume the patches in a novel way.

I've introduced some new extra settings enable-patches-ignore-subpackages and patches-ignore-whitelist as well as patches-log. These are further explained in the README.md file updates.

The point was to allow for more flexibility and to make some improvements.

Also, in reviewing the code, I adjust a few things which I found challenging:

1. The Patches.php method arrayMergeRecursiveDistinct() was doing a comparison against patch descriptions which I found wasn't respecting the uniqueness of the patch file and failing if a developer handled the patch incorrectly. Therefore, I switched up the comparison condition and made it work against the patch file instead.
2. I moved around a few variables to add clarity and also added what I thought was a missing protected variable.
3. Clean up a bit of manifest output.
4. Update a few messages for clarity.
5. Hook in some logging for better reviewing of what is being collated and installed as per patches.
6. Introduce a trait which keeps the mod organized and out of the main patches.php class.

I hope this gets tied into the main project as I feel it helps resolve a use case for enterprising solutions using composer patches.

[mxr576]

I totally share your enthusiasm on these topics, great additions! Although I feel that this on PR tries to introduce too many new concepts and features (patches ignore + patch logging, etc) and that plus the size of the PR is going to block or hold it back to be accepted soon. Probably it is worth splitting it into smaller ones.

[wilroboly]

I know what you mean. If we read their note on the first few lines of the README, it actually states they won't accept any new features anyway. My hope is this makes its way into the next version. I know there's a lot here, but the logging is actually to help with the patches ignore update. I found it was useful to see HOW they got applied and whether the request met the outcome. As such, its not a separate function but a part of it.

[mxr576]

I have two Twitter threads for you :)

https://twitter.com/IEMIXER/status/1430504753805602820 TL;DR Consider sponsoring @cweagans https://twitter.com/IEMIXER/status/1430505791665188864 TL;DR Consider if patching is the right solution :)

I have even mentioned this PR in one of these threads.

[wilroboly]

Thanks for the Kudos! Both really good threads. It also gave me a perspective into the OP of the plugin. I know EXACTLY what he means and how he feels.

At the end of the day, I work in Enterprise and Institution markets that leverage Drupal for tons of architectures. We leverage patches all the time and this patch / fork was essential to get around some idiomatic issues. My hope is that they will make their way into the 2.x branch at this point.

[petarbasic]

Unfortunately, this does not seem to work for my team. We replaced composer-patches with https://github.com/wilroboly/composer-patches/tree/wilroboly/patches-ignore-enhancements branch.

Following the instructions there, we added the enable-patches-ignore-subpackages and patches-ignore-whitelist sections in the root composer.json, where we whitelisted one of our sub-packages. In the whitelisted sub-package we add the patches-ignore section where we list a patch added by some other sub-package to be ignored.

Simply, it shows the same error if we didn't do all of this: the composer tries to apply the ignored patch.

Did we miss something or did anyone have a similar issue?

Thanks, and thank you for your effort to implement this feature!

[williamroboly]

Hi there,

The manner in which I got this to work for me is as follows since this is a package download from upstream packagist. In our case, we have a private packagist, so I used my branch, tagged the project as 1.7.1.1 (or whatever you want to tag it as) and then replaced the package source URL with our fork. This allows composer to "require" the cweagans/composer-patches" plugin as though it were the original package BUT also with our own tags.

Now, I know not everyone will have this options, so if you choose to use this code on a local version of the module, you'll have to follow another path. You could use the https://getcomposer.org/doc/05-repositories.md#path approach. Or even try to use the https://getcomposer.org/doc/05-repositories.md#using-private-repositories approach. Both should work. If that doesn't do the trick, then I'm not sure what path can be used, since those are my go to approaches. Maybe a stackoverflow search will yield another way.

Does this help?

[williamroboly]

I recently had a project that had some nested package names in the patches-ignore section which my patch was confused over. I've cleaned up the code and refactored the multidimensional flattening method to better take these possible use-cases. Have Fun!

[cweagans]

2.x doesn't have the concept of patches resolved from dependencies, so ignoring patches won't be as big of a thing. #447 if you're curious. All patches are defined in the root composer.json or in a patches file now (unless you build your own resolver for dependency patch resolution).