diff --git a/docs/api/patches-lock-json.md b/docs/api/patches-lock-json.md index a6cdd5c6..8e0fbee7 100644 --- a/docs/api/patches-lock-json.md +++ b/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). diff --git a/docs/usage/configuration.md b/docs/usage/configuration.md index 55c511b7..087c07de 100644 --- a/docs/usage/configuration.md +++ b/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" ] } } diff --git a/docs/usage/defining-patches.md b/docs/usage/defining-patches.md index c30c7b3b..a135a437 100644 --- a/docs/usage/defining-patches.md +++ b/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: diff --git a/src/Capability/Resolver/CoreResolverProvider.php b/src/Capability/Resolver/CoreResolverProvider.php index e8c0c6ad..3f9e5836 100644 --- a/src/Capability/Resolver/CoreResolverProvider.php +++ b/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), ]; } } diff --git a/src/Downloader.php b/src/Downloader.php index a2b23baf..0d798a25 100644 --- a/src/Downloader.php +++ b/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( - ' - Skipping downloader ' . get_class($downloader) . '', + ' - Skipping downloader ' . $class . '', true, IOInterface::VERBOSE ); diff --git a/src/Patch.php b/src/Patch.php index 0c299bfa..7beb82f1 100644 --- a/src/Patch.php +++ b/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. * diff --git a/src/Patcher.php b/src/Patcher.php index ce7114a5..35f4fd58 100644 --- a/src/Patcher.php +++ b/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( - ' - Skipping patcher ' . get_class($patcher) . '', + ' - Skipping patcher ' . $class . '', true, IOInterface::VERBOSE ); diff --git a/src/Plugin/Patches.php b/src/Plugin/Patches.php index c6ec4b03..5b74ca19 100644 --- a/src/Plugin/Patches.php +++ b/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'); } diff --git a/src/Resolver.php b/src/Resolver.php index b1d96309..18243039 100644 --- a/src/Resolver.php +++ b/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( - ' - Skipping resolver ' . get_class($resolver) . '', + ' - Skipping resolver ' . $class . '', true, IOInterface::VERBOSE ); diff --git a/src/Resolver/Dependencies.php b/src/Resolver/Dependencies.php new file mode 100644 index 00000000..e7ee8350 --- /dev/null +++ b/src/Resolver/Dependencies.php @@ -0,0 +1,54 @@ +composer->getLocker(); + if (!$locker->isLocked()) { + $this->io->write(' - Composer lock file does not exist.'); + $this->io->write(' - Patches defined in dependencies will not be resolved.'); + return; + } + + $this->io->write(' - Resolving patches from dependencies.'); + + $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. + } + } +} diff --git a/src/Resolver/PatchesFile.php b/src/Resolver/PatchesFile.php index cf80383c..c78a0457 100644 --- a/src/Resolver/PatchesFile.php +++ b/src/Resolver/PatchesFile.php @@ -18,8 +18,8 @@ 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) { @@ -27,11 +27,12 @@ public function resolve(PatchCollection $collection): void } $this->io->write(' - Resolving patches from patches file.'); - $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); } } diff --git a/src/Resolver/RootComposer.php b/src/Resolver/RootComposer.php index c0efc851..e677ea6e 100644 --- a/src/Resolver/RootComposer.php +++ b/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); } } diff --git a/tests/_data/dep-test-package/composer.json b/tests/_data/dep-test-package/composer.json index c1ad968a..b4f142ff 100644 --- a/tests/_data/dep-test-package/composer.json +++ b/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" + } } + } } diff --git a/tests/_data/fixtures/dependency-patches-duplicate-patch/composer.json b/tests/_data/fixtures/dependency-patches-duplicate-patch/composer.json new file mode 100644 index 00000000..3da81c53 --- /dev/null +++ b/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" + } + } + } +} diff --git a/tests/_data/fixtures/dependency-patches/composer.json b/tests/_data/fixtures/dependency-patches/composer.json new file mode 100644 index 00000000..87550e75 --- /dev/null +++ b/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 + } + } +} diff --git a/tests/_data/fixtures/disable-dependency-patches/composer.json b/tests/_data/fixtures/disable-dependency-patches/composer.json new file mode 100644 index 00000000..7c13947b --- /dev/null +++ b/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" + ] + } + } +} diff --git a/tests/_data/fixtures/no-patchers-available/composer.json b/tests/_data/fixtures/no-patchers-available/composer.json index c0555c9f..2b68989f 100644 --- a/tests/_data/fixtures/no-patchers-available/composer.json +++ b/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": { diff --git a/tests/acceptance/CommandsCept.php b/tests/acceptance/CommandsCept.php index 166d05cd..0ec8433f 100644 --- a/tests/acceptance/CommandsCept.php +++ b/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'); diff --git a/tests/acceptance/CustomComposerJsonFilenameCept.php b/tests/acceptance/CustomComposerJsonFilenameCept.php index a1eeecd3..0aa64dca 100644 --- a/tests/acceptance/CustomComposerJsonFilenameCept.php +++ b/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'); diff --git a/tests/acceptance/DependencyPatchesCept.php b/tests/acceptance/DependencyPatchesCept.php new file mode 100644 index 00000000..ee707a27 --- /dev/null +++ b/tests/acceptance/DependencyPatchesCept.php @@ -0,0 +1,17 @@ +wantTo('know that the plugin will find patches defined in dependencies'); +$I->amInPath(codecept_data_dir('fixtures/dependency-patches')); +$I->runComposerCommand('install', ['-vvv']); +$I->canSeeInComposerOutput('Resolving patches from dependencies'); +$I->canSeeInComposerOutput('Patching cweagans/composer-patches-testrepo'); +$I->seeFileFound('OneMoreTest.php', 'vendor/cweagans/composer-patches-testrepo/src'); +$I->openFile('patches.lock.json'); +$I->seeInThisFile('5f539e947d097d79e97006f958916c362df0fd19ba7011b1b54b69f02d5f9958'); diff --git a/tests/acceptance/DependencyPatchesDuplicatePatchCept.php b/tests/acceptance/DependencyPatchesDuplicatePatchCept.php new file mode 100644 index 00000000..a71bae2d --- /dev/null +++ b/tests/acceptance/DependencyPatchesDuplicatePatchCept.php @@ -0,0 +1,18 @@ +wantTo('know that the plugin will handle duplicate patches appropriately'); +$I->amInPath(codecept_data_dir('fixtures/dependency-patches-duplicate-patch')); +$I->runComposerCommand('install', ['-vvv']); +$I->canSeeInComposerOutput('Resolving patches from root package'); +$I->canSeeInComposerOutput('Resolving patches from dependencies'); +$I->canSeeInComposerOutput('Patching cweagans/composer-patches-testrepo'); +$I->seeFileFound('OneMoreTest.php', 'vendor/cweagans/composer-patches-testrepo/src'); +$I->openFile('patches.lock.json'); +$I->seeInThisFile('4dc9f5061770f76d203942a3a7f211fe6bbcbde58a185605afc038002f538c9f'); diff --git a/tests/acceptance/DisableDependencyPatchesCept.php b/tests/acceptance/DisableDependencyPatchesCept.php new file mode 100644 index 00000000..8e309323 --- /dev/null +++ b/tests/acceptance/DisableDependencyPatchesCept.php @@ -0,0 +1,17 @@ +wantTo('disable resolving patches from dependencies (and test disabling resolvers in general)'); +$I->amInPath(codecept_data_dir('fixtures/disable-dependency-patches')); +$I->runComposerCommand('install', ['-vvv']); +$I->dontSeeInComposerOutput('Resolving patches from dependencies'); +$I->dontSeeInComposerOutput('Patching cweagans/commposer-patches-testrepo'); +$I->dontSeeFileFound('OneMoreTest.php', 'vendor/cweagans/composer-patches-testrepo/src'); +$I->openFile('patches.lock.json'); +$I->seeInThisFile('6142bfcb78f54dfbf5247ae5e463f25bdb8fff1890806e2e45aa81a59c211653'); diff --git a/tests/unit/CoreResolverProviderTest.php b/tests/unit/CoreResolverProviderTest.php index 913e8040..68a6fcb5 100644 --- a/tests/unit/CoreResolverProviderTest.php +++ b/tests/unit/CoreResolverProviderTest.php @@ -24,7 +24,7 @@ public function testGetResolvers() $resolvers = $resolverProvider->getResolvers(); - $this->assertCount(2, $resolvers); + $this->assertCount(3, $resolvers); $this->assertContainsOnlyInstancesOf(ResolverInterface::class, $resolvers); } }