Skip to content

Dependency patch resolution #547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/api/patches-lock-json.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
24 changes: 23 additions & 1 deletion docs/usage/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
]
}
}
Expand Down
9 changes: 9 additions & 0 deletions docs/usage/defining-patches.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions src/Capability/Resolver/CoreResolverProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
];
}
}
5 changes: 3 additions & 2 deletions src/Downloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
2 changes: 1 addition & 1 deletion src/Patch.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
5 changes: 3 additions & 2 deletions src/Patcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
6 changes: 5 additions & 1 deletion src/Plugin/Patches.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down
6 changes: 4 additions & 2 deletions src/Resolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
54 changes: 54 additions & 0 deletions src/Resolver/Dependencies.php
Original file line number Diff line number Diff line change
@@ -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.
}
}
}
7 changes: 4 additions & 3 deletions src/Resolver/PatchesFile.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/Resolver/RootComposer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
23 changes: 10 additions & 13 deletions tests/_data/dep-test-package/composer.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
26 changes: 26 additions & 0 deletions tests/_data/fixtures/dependency-patches/composer.json
Original file line number Diff line number Diff line change
@@ -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
}
}
}
33 changes: 33 additions & 0 deletions tests/_data/fixtures/disable-dependency-patches/composer.json
Original file line number Diff line number Diff line change
@@ -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"
]
}
}
}
8 changes: 3 additions & 5 deletions tests/_data/fixtures/no-patchers-available/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
4 changes: 2 additions & 2 deletions tests/acceptance/CommandsCept.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions tests/acceptance/CustomComposerJsonFilenameCept.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Loading