Merge pull request #547 from cweagans/dependency-patches

Dependency patch resolution

docs/api/patches-lock-json.md

@@ -19,4 +19,4 @@ If the [`COMPOSER`]({{< relref "../usage/configuration.md#composer" >}}) environ
 ```
 * Each patch definition will look like the [expanded format]({{< relref "../usage/defining-patches.md#expanded-format" >}}) that users can put into their `composer.json` or external patches file.
 * No _removals_ or _changes_ will be made to the patch definition object. _Additional_ keys may be created, so any JSON parsing you're doing should be tolerant of new keys.
-* The `extra` object in each patch definition may contain a number of attributes set by other projects or by the user and should be treated as free-form input.
+* The `extra` object in each patch definition may contain a number of attributes set by other projects or by the user and should be treated as free-form input. Currently, Composer Patches uses this attribute to store information about where a patch was defined (in the `provenance` key).

docs/usage/configuration.md

@@ -49,6 +49,27 @@ Technically, this value can be a path to a file that is nested in a deeper direc
 
 ---
 
+### `ignore-dependency-patches`
+
+```json
+{
+    [...],
+    "extra": {
+        "composer-patches": {
+            "ignore-dependency-patches": [
+                "some/package",
+            ]
+        }
+    }
+}
+```
+
+**Default value**: empty
+
+`ignore-dependency-patches` allows you to ignore patches defined by the listed dependencies. For instance, if your project requires `drupal/core` and `some/package`, and `some/package` defines a patch for `drupal/core`, listing `some/package` in `ignore-dependency-patches` would cause that patch to be ignored. This does _not_ affect the _target_ of those patches. For instance, listing `drupal/core` here would not cause patches _to_ `drupal/core` to be ignored.
+
+---
+
 ### `default-patch-depth`
 
 ```json
@@ -81,7 +102,8 @@ You probably don't need to change this value. Instead, consider setting a packag
         "composer-patches": {
             "disable-resolvers": [
                 "\\cweagans\\Composer\\Resolver\\RootComposer",
-                "\\cweagans\\Composer\\Resolver\\PatchesFile"
+                "\\cweagans\\Composer\\Resolver\\PatchesFile",
+                "\\cweagans\\Composer\\Resolver\\Dependencies"
             ]
         }
     }

docs/usage/defining-patches.md

@@ -130,6 +130,15 @@ If you're defining patches in `patches.json` (or some other separate patches fil
 }
 ```
 
+### Dependencies
+
+Packages required by your project can define patches as well. They can do so by defining patches in their root `composer.json` file. Defining patches in a separate `patches.json` in a dependency is currently unsupported.
+
+{{< callout title="Patch paths are always relative to the root of your project" >}}
+Patches defined by a dependency should always use a publicly accessible URL, rather than a local file path. Composer Patches _will not_ attempt to modify file paths so that the patch file can be found within the installed location of a dependency.
+{{< /callout >}}
+
+
 ## Duplicate patches
 
 If the same patch is defined in multiple places, the first one added to the patch collection "wins". Subsequent definitions of the same patch will be ignored without emitting an error. The criteria used for determining whether two patches are the same are:

src/Capability/Resolver/CoreResolverProvider.php

@@ -16,6 +16,7 @@ public function getResolvers(): array
         return [
             new RootComposer($this->composer, $this->io, $this->plugin),
             new PatchesFile($this->composer, $this->io, $this->plugin),
+            new Dependencies($this->composer, $this->io, $this->plugin),
         ];
     }
 }

src/Downloader.php

@@ -52,9 +52,10 @@ public function downloadPatch(Patch $patch)
         }
 
         foreach ($this->getDownloaders() as $downloader) {
-            if (in_array(get_class($downloader), $this->disabledDownloaders, true)) {
+            $class = "\\" . get_class($downloader);
+            if (in_array($class, $this->disabledDownloaders, true)) {
                 $this->io->write(
-                    '<info>  - Skipping downloader ' . get_class($downloader) . '</info>',
+                    '<info>  - Skipping downloader ' . $class . '</info>',
                     true,
                     IOInterface::VERBOSE
                 );

src/Patch.php

@@ -50,7 +50,7 @@ class Patch implements JsonSerializable
     public ?string $localPath;
 
     /**
-     * This is unused in the main plugin, but can be used as a place for other plugins to store data about a patch.
+     * Can be used as a place for other plugins to store data about a patch.
      *
      * This should be treated as an associative array and should contain only scalar values.
      *

src/Patcher.php

@@ -41,9 +41,10 @@ public function __construct(Composer $composer, IOInterface $io, array $disabled
     public function applyPatch(Patch $patch, string $path): bool
     {
         foreach ($this->getPatchers() as $patcher) {
-            if (in_array(get_class($patcher), $this->disabledPatchers, true)) {
+            $class = "\\" . get_class($patcher);
+            if (in_array($class, $this->disabledPatchers, true)) {
                 $this->io->write(
-                    '<info>  - Skipping patcher ' . get_class($patcher) . '</info>',
+                    '<info>  - Skipping patcher ' . $class . '</info>',
                     true,
                     IOInterface::VERBOSE
                 );

src/Plugin/Patches.php

@@ -145,7 +145,11 @@ public function activate(Composer $composer, IOInterface $io): void
             'patches-file' => [
                 'type' => 'string',
                 'default' => 'patches.json',
-            ]
+            ],
+            "ignore-dependency-patches" => [
+                'type' => 'list',
+                'default' => [],
+            ],
         ];
         $this->configure($this->composer->getPackage()->getExtra(), 'composer-patches');
     }

src/Resolver.php

@@ -45,9 +45,11 @@ public function loadFromResolvers(): PatchCollection
         // Let each resolver discover patches and add them to the PatchCollection.
         /** @var ResolverInterface $resolver */
         foreach ($this->getPatchResolvers() as $resolver) {
-            if (in_array(get_class($resolver), $this->disabledResolvers, true)) {
+            $class = "\\" . get_class($resolver);
+
+            if (in_array($class, $this->disabledResolvers, true)) {
                 $this->io->write(
-                    '<info>  - Skipping resolver ' . get_class($resolver) . '</info>',
+                    '<info>  - Skipping resolver ' . $class . '</info>',
                     true,
                     IOInterface::VERBOSE
                 );

src/Resolver/Dependencies.php

@@ -0,0 +1,54 @@
+<?php
+
+/**
+ * @file
+ * Contains \cweagans\Composer\Resolvers\Dependencies.
+ */
+
+namespace cweagans\Composer\Resolver;
+
+use cweagans\Composer\Patch;
+use cweagans\Composer\PatchCollection;
+
+class Dependencies extends ResolverBase
+{
+    /**
+     * {@inheritDoc}
+     */
+    public function resolve(PatchCollection $collection): void
+    {
+        $locker = $this->composer->getLocker();
+        if (!$locker->isLocked()) {
+            $this->io->write('  - <info>Composer lock file does not exist.</info>');
+            $this->io->write('  - <info>Patches defined in dependencies will not be resolved.</info>');
+            return;
+        }
+
+        $this->io->write('  - <info>Resolving patches from dependencies.</info>');
+
+        $ignored_dependencies = $this->plugin->getConfig('ignore-dependency-patches');
+
+        $lockdata = $locker->getLockData();
+        foreach ($lockdata['packages'] as $p) {
+            // If we're supposed to skip gathering patches from a dependency, do that.
+            if (in_array($p['name'], $ignored_dependencies)) {
+                continue;
+            }
+
+            // Find patches in the composer.json for dependencies.
+            if (!isset($p['extra']) || !isset($p['extra']['patches'])) {
+                continue;
+            }
+            foreach ($this->findPatchesInJson($p['extra']['patches']) as $package => $patches) {
+                foreach ($patches as $patch) {
+                    $patch->extra['provenance'] = "dependency:" . $package;
+
+                    /** @var Patch $patch */
+                    $collection->addPatch($patch);
+                }
+            }
+
+            // TODO: Also find patches in a configured patches.json for the dependency.
+        }
+    }
+}

src/Resolver/PatchesFile.php

@@ -18,20 +18,21 @@ class PatchesFile extends ResolverBase
      */
     public function resolve(PatchCollection $collection): void
     {
-        $patches_file = $this->plugin->getConfig('patches-file');
-        $valid_patches_file = file_exists(realpath($patches_file)) && is_readable(realpath($patches_file));
+        $patches_file_path = $this->plugin->getConfig('patches-file');
+        $valid_patches_file = file_exists(realpath($patches_file_path)) && is_readable(realpath($patches_file_path));
 
         // If we don't have a valid patches file, exit early.
         if (!$valid_patches_file) {
             return;
         }
 
         $this->io->write('  - <info>Resolving patches from patches file.</info>');
-        $patches_file = $this->readPatchesFile($patches_file);
+        $patches_file = $this->readPatchesFile($patches_file_path);
 
         foreach ($this->findPatchesInJson($patches_file) as $package => $patches) {
             foreach ($patches as $patch) {
                 /** @var Patch $patch */
+                $patch->extra['provenance'] = "patches-file:" . $patches_file_path;
                 $collection->addPatch($patch);
             }
         }

src/Resolver/RootComposer.php

@@ -27,6 +27,7 @@ public function resolve(PatchCollection $collection): void
         foreach ($this->findPatchesInJson($extra['patches']) as $package => $patches) {
             foreach ($patches as $patch) {
                 /** @var Patch $patch */
+                $patch->extra['provenance'] = "root";
                 $collection->addPatch($patch);
             }
         }

tests/_data/dep-test-package/composer.json

@@ -1,16 +1,13 @@
 {
-    "name": "cweagans/dep-test-package",
-    "description": "Project for use in cweagans/composer-patches acceptance tests.",
-    "type": "project",
-    "license": "BSD-2-Clause",
-    "require": {
-        "drupal/drupal": "8.2.0"
-    },
-    "extra": {
-        "patches": {
-            "drupal/drupal": {
-                "Add a startup config for the PHP web server": "https://www.drupal.org/files/issues/add_a_startup-1543858-58.patch"
-            }
-        }
+  "name": "cweagans/dep-test-package",
+  "description": "Project for use in cweagans/composer-patches acceptance tests.",
+  "type": "project",
+  "license": "BSD-3-Clause",
+  "extra": {
+    "patches": {
+      "cweagans/composer-patches-testrepo": {
+        "Add a file": "https://patch-diff.githubusercontent.com/raw/cweagans/composer-patches-testrepo/pull/1.patch"
+      }
     }
+  }
 }

tests/_data/fixtures/dependency-patches-duplicate-patch/composer.json

@@ -0,0 +1,33 @@
+{
+  "name": "cweagans/composer-patches-test-project",
+  "description": "Project for use in cweagans/composer-patches acceptance tests.",
+  "type": "project",
+  "license": "BSD-3-Clause",
+  "repositories": [
+    {
+      "type": "path",
+      "url": "../../../../"
+    },
+    {
+      "type": "path",
+      "url": "../../dep-test-package"
+    }
+  ],
+  "require": {
+    "cweagans/composer-patches": "*@dev",
+    "cweagans/composer-patches-testrepo": "~1.0",
+    "cweagans/dep-test-package": "*@dev"
+  },
+  "config": {
+    "allow-plugins": {
+      "cweagans/composer-patches": true
+    }
+  },
+  "extra": {
+    "patches": {
+      "cweagans/composer-patches-testrepo": {
+        "Add a file": "https://patch-diff.githubusercontent.com/raw/cweagans/composer-patches-testrepo/pull/1.patch"
+      }
+    }
+  }
+}

tests/_data/fixtures/dependency-patches/composer.json

@@ -0,0 +1,26 @@
+{
+  "name": "cweagans/composer-patches-test-project",
+  "description": "Project for use in cweagans/composer-patches acceptance tests.",
+  "type": "project",
+  "license": "BSD-3-Clause",
+  "repositories": [
+    {
+      "type": "path",
+      "url": "../../../../"
+    },
+    {
+      "type": "path",
+      "url": "../../dep-test-package"
+    }
+  ],
+  "require": {
+    "cweagans/composer-patches": "*@dev",
+    "cweagans/composer-patches-testrepo": "~1.0",
+    "cweagans/dep-test-package": "*@dev"
+  },
+  "config": {
+    "allow-plugins": {
+      "cweagans/composer-patches": true
+    }
+  }
+}

tests/_data/fixtures/disable-dependency-patches/composer.json

@@ -0,0 +1,33 @@
+{
+  "name": "cweagans/composer-patches-test-project",
+  "description": "Project for use in cweagans/composer-patches acceptance tests.",
+  "type": "project",
+  "license": "BSD-3-Clause",
+  "repositories": [
+    {
+      "type": "path",
+      "url": "../../../../"
+    },
+    {
+      "type": "path",
+      "url": "../../dep-test-package"
+    }
+  ],
+  "require": {
+    "cweagans/composer-patches": "*@dev",
+    "cweagans/composer-patches-testrepo": "~1.0",
+    "cweagans/dep-test-package": "*@dev"
+  },
+  "config": {
+    "allow-plugins": {
+      "cweagans/composer-patches": true
+    }
+  },
+  "extra": {
+    "composer-patches": {
+      "disable-resolvers": [
+        "\\cweagans\\Composer\\Resolver\\Dependencies"
+      ]
+    }
+  }
+}

tests/_data/fixtures/no-patchers-available/composer.json

@@ -16,11 +16,9 @@
   "extra": {
     "composer-patches": {
       "disable-patchers": [
-        "cweagans\\Composer\\Patcher\\BsdPatchPatcher",
-        "cweagans\\Composer\\Patcher\\GitPatcher",
-        "cweagans\\Composer\\Patcher\\GitInitPatcher",
-        "cweagans\\Composer\\Patcher\\GnuGPatchPatcher",
-        "cweagans\\Composer\\Patcher\\GnuPatchPatcher"
+        "\\cweagans\\Composer\\Patcher\\FreeformPatcher",
+        "\\cweagans\\Composer\\Patcher\\GitPatcher",
+        "\\cweagans\\Composer\\Patcher\\GitInitPatcher"
       ]
     },
     "patches": {

tests/acceptance/CommandsCept.php

@@ -12,11 +12,11 @@
 $I->runComposerCommand('install', ['-vvv']);
 
 $I->openFile('patches.lock.json');
-$I->seeInThisFile('725f2631cb6a92c8c3ffc2e396e89f73b726869131d4c4d2a5903aae6854a260');
+$I->seeInThisFile('4dc9f5061770f76d203942a3a7f211fe6bbcbde58a185605afc038002f538c9f');
 
 $I->runShellCommand('composer patches-relock');
 $I->openFile('patches.lock.json');
-$I->seeInThisFile('725f2631cb6a92c8c3ffc2e396e89f73b726869131d4c4d2a5903aae6854a260');
+$I->seeInThisFile('4dc9f5061770f76d203942a3a7f211fe6bbcbde58a185605afc038002f538c9f');
 
 $I->runShellCommand('composer patches-repatch 2>&1');
 $I->canSeeInShellOutput('Removing cweagans/composer-patches-testrepo');

tests/acceptance/CustomComposerJsonFilenameCept.php

@@ -16,13 +16,13 @@
 $I->canSeeFileFound('./composer-a-patches.lock.json');
 
 $I->openFile('composer-a-patches.lock.json');
-$I->seeInThisFile('725f2631cb6a92c8c3ffc2e396e89f73b726869131d4c4d2a5903aae6854a260');
+$I->seeInThisFile('4dc9f5061770f76d203942a3a7f211fe6bbcbde58a185605afc038002f538c9f');
 
 $I->deleteFile('composer-a-patches.lock.json');
 $I->runShellCommand('composer patches-relock');
 $I->canSeeFileFound('./composer-a-patches.lock.json');
 $I->openFile('composer-a-patches.lock.json');
-$I->seeInThisFile('725f2631cb6a92c8c3ffc2e396e89f73b726869131d4c4d2a5903aae6854a260');
+$I->seeInThisFile('4dc9f5061770f76d203942a3a7f211fe6bbcbde58a185605afc038002f538c9f');
 
 // Clean up so other tests don't fail.
 putenv('COMPOSER');