Skip to content
Draft
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
21 changes: 21 additions & 0 deletions 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

---

### `allow-dependency-patches`

```json
{
[...],
"extra": {
"composer-patches": {
"allow-dependency-patches": [
"some/package",
]
}
}
}
```

**Default value**: null

`allow-dependency-patches` allows you to explicitly apply patches defined by only the listed dependencies. For instance, if your project requires `some/packageA` and `some/packageB`, and both packages define patches, listing `some/packageA` in `allow-dependency-patches` would cause that only `some/packageA` will be evaluated while `some/packageB` will be ignored. If set `allow-dependency-patches: []`, no package will be evaluated. 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.

---

### `ignore-dependency-patches`

```json
Expand Down
4 changes: 4 additions & 0 deletions src/Plugin/Patches.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ public function activate(Composer $composer, IOInterface $io): void
'type' => 'string',
'default' => 'patches.json',
],
"allow-dependency-patches" => [
'type' => 'list',
'default' => null,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably default to []

This comment was marked as outdated.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There aren't existing projects that are using 2.x with this functionality, as this functionality does not yet exist.

2.x uses explicit configuration options and very little is implied by other options. If you don't want dependency patch resolution at all, then you simply disable that resolver. This option will be for changing the behavior, not disabling it.

],
"ignore-dependency-patches" => [
'type' => 'list',
'default' => [],
Expand Down
42 changes: 33 additions & 9 deletions src/Resolver/Dependencies.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,42 @@ public function resolve(PatchCollection $collection): void
return;
}

$this->io->write(' - <info>Resolving patches from dependencies.</info>');

// Using both config keys, if $allowed_dependencies is set, it takes precedence
$allowed_dependencies = $this->plugin->getConfig('allow-dependency-patches');
$ignored_dependencies = $this->plugin->getConfig('ignore-dependency-patches');

// First check, if we do allow dependency patches at all.
if ($allowed_dependencies === []) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe empty($allowed_dependencies) in light of defaulting the value to []?

$this->io->write(' - <info>No patches from dependencies are allowed.</info>');
return;
}

$this->io->write(' - <info>Resolving patches from dependencies.</info>');

$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;
$allowed = in_array($p['name'], $allowed_dependencies ?? []);
$ignored = in_array($p['name'], $ignored_dependencies);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should simplify this a bit and disallow setting both allowed/ignored dependencies. You can either say "I only want patches from these listed deps" or "I want patches from all deps except these specific ones"

It doesn't make sense to say "I want patches from all deps except these specific ones, but only from these listed deps"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have 4 possibilities, if we remain the ignored dependencies possibility:
a) Current behavior: i have no allow-dependency-patches, and no ignored-dependency-patches, i wanted to preserve that all dependent patches will be used. (Case null)
b) Current behavior: I have no allow-dependency-patches, and some ignored-dependency-patches, all dependent patches should be installed except those listed in ignored. (Case null)
c) New behavior: I do not want any dependency patches, so allow-dependency should be a empty list. (Case [] )
d) New behavior: I want do allow some packages, but because of composer-merge i forgot, i have forbitten in another file package x/y to dependent patches. So this will not break existing installations but is really an enhancement without breaking.

If its okay to break current behavior, we could do differently. But i wanted to keep existing behavior, and wanted to keep expected behavior, like an empty list of allowed dependency implicit none is allowed, and if i ignore something, it will be ignored.


// Allowed dependencies is not set in composer.json, and we're not
// supposed to skip gathering patches from this dependency
if (is_null($allowed_dependencies) && !$ignored) {
$this->lookForPatches($p, $collection);
}

// Find patches in the composer.json for dependencies.
if (!isset($p['extra']) || !isset($p['extra']['patches'])) {
continue;
// Allowed dependencies are set, act only on allowed, if they're not
// ignored also.
if ($allowed && !$ignored) {
$this->lookForPatches($p, $collection);
}
}
}

private function lookForPatches(array $p, PatchCollection $collection): void
{

// Find patches in the composer.json for dependencies.
if (isset($p['extra']['patches'])) {
foreach ($this->findPatchesInJson($p['extra']['patches']) as $package => $patches) {
foreach ($patches as $patch) {
$patch->extra['provenance'] = "dependency:" . $package;
Expand All @@ -47,8 +68,11 @@ public function resolve(PatchCollection $collection): void
$collection->addPatch($patch);
}
}
}

// TODO: Also find patches in a configured patches.json for the dependency.
// Find patches in a patch-file for dependencies.
if (isset($p['extra']['composer-patches']['patches-file'])) {
// @todo Handle patch files.
}
}
}