{"action":"created","issue":{"url":"https://api.github.com/repos/drupal-composer/drupal-project/issues/393","repository_url":"https://api.github.com/repos/drupal-composer/drupal-project","labels_url":"https://api.github.com/repos/drupal-composer/drupal-project/issues/393/labels{/name}","comments_url":"https://api.github.com/repos/drupal-composer/drupal-project/issues/393/comments","events_url":"https://api.github.com/repos/drupal-composer/drupal-project/issues/393/events","html_url":"https://github.com/drupal-composer/drupal-project/issues/393","id":323276093,"number":393,"title":"Replace cweagans/composer-patches with vaimo/composer-patches","user":{"login":"boobaa","id":294106,"avatar_url":"https://avatars3.githubusercontent.com/u/294106?v=4","gravatar_id":"","url":"https://api.github.com/users/boobaa","html_url":"https://github.com/boobaa","followers_url":"https://api.github.com/users/boobaa/followers","following_url":"https://api.github.com/users/boobaa/following{/other_user}","gists_url":"https://api.github.com/users/boobaa/gists{/gist_id}","starred_url":"https://api.github.com/users/boobaa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/boobaa/subscriptions","organizations_url":"https://api.github.com/users/boobaa/orgs","repos_url":"https://api.github.com/users/boobaa/repos","events_url":"https://api.github.com/users/boobaa/events{/privacy}","received_events_url":"https://api.github.com/users/boobaa/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-05-15T15:44:32Z","updated_at":"2018-05-17T06:42:30Z","closed_at":null,"author_association":"NONE","body":"The [cweagans/composer-patches](https://github.com/cweagans/composer-patches) project has a list of issues which are basically around one thing: patches that add new files are NOT handled properly (cweagans/composer-patches#43, cweagans/composer-patches#101 and cweagans/composer-patches#178 to name a few).\r\n\r\nI'm using [this core patch](https://www.drupal.org/files/issues/drupal-add-unpublished-nodes-to-taxonomy-index-962664-81.patch) as an example.\r\n\r\nWhile using cweagans/composer-patches, I'm having some artifacts like `$gitroot/web/core/b/core/modules/taxonomy/taxonomy.install` and `$gitroot/web/core/core/modules/taxonomy/taxonomy.install`, which are obviously wrong (correct one would be `$gitroot/web/core/modules/taxonomy/taxonomy.install`, tho I must admit that this file gets also created). Searching for the root reason for this reveals that cweagans/composer-patches tries to invent the `-p` level for the `patch` command by doing some try&error, with a hardcoded order of `-p1`, `-p0`, `-p2` and `-p4` (in this hardcoded, non-configurable order). With this approach, the patches that are creating new files will just apply cleanly in just about either case. Seemingly cweagans/composer-patches managed to pick `-p2`, but doesn't clear up the mess it created. The author/maintainer @cweagans says the upcoming 2.x release will/does have a configuration option for each patch that would allow specifying the `-p` level, but we're just not there yet.\r\n\r\nThere are some known workarounds to remedy the situation: eg. acquia/blt#2809 simply runs an `rm -rf core/core core/b` during `post-autoload-dump`.\r\n\r\nI did some research on alternatives to cweagans/composer-patches. First I came across [netresearch/composer-patches-plugin](https://github.com/netresearch/composer-patches-plugin) which seems really-really flexible (eg. it would even allow patching a required package from another required package), but it turns out that even when I'm providing the correct `-p2` as the `args` for my patch and it says it's applied, actually it is not. That's a clear showstopper for me.\r\n\r\nThen I came across [vaimo/composer-patches](https://github.com/vaimo/composer-patches). It also allows providing the correct `-p` level manually and does apply the patch when it says so. Its only drawback I'm currently aware of is the inability of patching a package from another (like asking for a core patch within a particular contrib/custom module's `composer.json` file; see vaimo/composer-patches#13 for more details). I'm also tempted to create a PR for it that would copy the lacking `\"enable-patching\": true` functionality from cweagans/composer-patches.\r\n\r\nAs I side note, I think it's also worth mentioning that vaimo/composer-patches seems to be forked from cweagans/composer-patches back in 2016, and seemingly both have some features since then which the other one lacks.\r\n\r\nI'm not creating a PR just yet as I'm interested in your opinion first."},"comment":{"url":"https://api.github.com/repos/drupal-composer/drupal-project/issues/comments/389761688","html_url":"https://github.com/drupal-composer/drupal-project/issues/393#issuecomment-389761688","issue_url":"https://api.github.com/repos/drupal-composer/drupal-project/issues/393","id":389761688,"user":{"login":"boobaa","id":294106,"avatar_url":"https://avatars3.githubusercontent.com/u/294106?v=4","gravatar_id":"","url":"https://api.github.com/users/boobaa","html_url":"https://github.com/boobaa","followers_url":"https://api.github.com/users/boobaa/followers","following_url":"https://api.github.com/users/boobaa/following{/other_user}","gists_url":"https://api.github.com/users/boobaa/gists{/gist_id}","starred_url":"https://api.github.com/users/boobaa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/boobaa/subscriptions","organizations_url":"https://api.github.com/users/boobaa/orgs","repos_url":"https://api.github.com/users/boobaa/repos","events_url":"https://api.github.com/users/boobaa/events{/privacy}","received_events_url":"https://api.github.com/users/boobaa/received_events","type":"User","site_admin":false},"created_at":"2018-05-17T06:42:30Z","updated_at":"2018-05-17T06:42:30Z","author_association":"NONE","body":"Did some more homework.\r\n\r\nFor managing a Drupal 8 project's codebase, we're having these requirements:\r\n1. Both core (`-p2`) and contrib (`-p1`) patches should be handled properly.\r\n2. There shouldn't be any \"artifacts\" (eg. like `$gitroot/web/core/b/core/modules/taxonomy/taxonomy.install` for [this](https://www.drupal.org/files/issues/drupal-add-unpublished-nodes-to-taxonomy-index-962664-81.patch) patch) after Composer finishes its job.\r\n3. There should be a way to patch core from a contrib and/or a custom module. (We're reusing some custom modules across projects that need core patches.)\r\n\r\nHere are my findings.\r\n- Latest stable cweagans/composer-patches fails on the first two requirements (okay, they're the effect of the same problem).\r\n- Latest dev of it (or at least one with the `2ec4f00` commit merged in cweagans/composer-patches#189) solves the first two requirements.\r\n- Latest stable vaimo/composer-patches doesn't have a problem with the first two requirements.\r\n- Latest dev cweagans/composer-patches support explicit providing of the `-p` level, but only per-package; latest stable vaimo/composer-patches support explicit providing the `-p` level even per-patch. For core patches,\r\n- To meet the third requirement, wikimedia/composer-merge-plugin is needed in both cases (with `extra/merge-plugin/merge-extra-deep: true`); for vaimo/composer-patches an `extra/enable-patching: true` is needed in either the root composer.json or the custom module's one; this is not needed for cweagans/composer-patches (TBH I must admit IDK why).\r\n- Also did some benchmarking on a D8 project with two custom modules (one needing a core patch), two other core patches, a whole bunch of contrib modules with three of them needing patches, and some PHP and JS libs. The same codebase was built by cweagans/composer-patches in 10 minutes and by vaimo/composer-patches in 6 minutes. (Basically cweagans/composer-patches applies the two patches from the root composer.json during installation of core, then after generating the autoload files it removes core and readds it with all the three patches, which wastes quite a bit of time.)\r\n\r\nSumming it up, there are only two reasons for replacing cweagans/composer-patches with vaimo/composer-patches (after the `2ec4f00` commit in the former):\r\n- vaimo/composer-patches allows explicit providing of the `-p` level per-patch (which might be a good thing if one needs to apply a `-p1` core patch, if there's any).\r\n- vaimo/composer-patches might run faster (by not wasting time on patching core twice in some not-so-corner cases).\r\n- There are some other benefits for vaimo/composer-patches (like it's generally more flexible and [provides some `composer patch` commands](https://github.com/vaimo/composer-patches#patch-command)), but I find those marginal.\r\n- OTOH, as the `extra/patches` syntax is different, this change might cause some minor headaches for folks already using drupal-composer/drupal-project."}}
{"actor":{"display_login":"boobaa"}}