Locally stored patches in packages

[jurgenhaas]

We have a private package with a composer.json like this:

{
  "require": {
    "phpoffice/phpspreadsheet": "^1.11",
  },
  "extra": {
    "patches": {
      "phpoffice/phpspreadsheet": {
        "Output only from column D onwards": "patches/column-d.patch"
      }
    }
  }
}

The package contains a local file patches/column-d.patch and when composer update runs, it finds the patch file and applies it.

Now, when we require that private package in a project with "enable-patching": true in the extra section, that work well with all the patches that have a fully qualified url (e.g. https://www.drupal.org/files/issues/2019-02-15/3033341-2-domain-pattern.patch) but it doesn't find the local patch file.

With composer 1 we got this working by specifying the patch file name in the package as a relative path from the future root project, but that was (a) not safe and (b) does no longer work with composer 2.

Any idea how we could do that properly?

[cweagans]

I'm surprised that it worked to specify a relative path from the future root project. I'm also surprised that doesn't work anymore.

IMO, the best way to do this is to put the patch somewhere public (private gist maybe?).

If you can track down where the problem is and open a PR, I'd happily take a look. Stuff that worked in Composer 1 should also work in Composer 2, so anything that doesn't is a regression (probably in this plugin).

[jurgenhaas]

@cweagans indeed I can't reproduce how that worked before but I worked out a simple solution on how to support patch files that are stored locally. I'm willing to provide a PR but I'm struggling, because the master branch here is completely different from the 1.7 release that gets installed and which I used to develop my PR. Can you please explain how I should proceed?

[cweagans]

1.x is probably where you want to go. master is the upcoming 2.0 release.

[jurgenhaas]

Submitted a PR for this, is it any good?

[jurgenhaas]

@cweagans any chance of moving this forward?

[ctrl-f5]

We are also running into this issue, we want to patch codeception, and this patch is distributed in a package that also uses codeception in its own tests.

I tested the changes in PR #340 and it works as expected!

[robincee]

Did some testing / reviewed lots of code, this is where i landed. (1.x, didn't really look at 2.x yet, TODO)

Long story really short: When a project contains a locally stored patch for itself composer-patches tries to apply the patch before the project is downloaded and installed. Voila: issue.

Don't know how this works with composer1, but composer2 fetches all metadata before starting to install packages. Patches are resolved, but if the extension is not installed yet, so the resolving fails to add the correct path without informing the user and the relative path (folder/file.patch) is returned - and used - and that obviously fails, cause if the file does not exist, composer-patches attempts to download it - and that also fails. Leading to the error like this, 329 and alike.

I believe this should also be fixed / related: https://github.com/cweagans/composer-patches/issues/329

But the real issue is: composer-patches does not re-fetch patch info (and thus does not add the correct path) when the extension is installed - it fetches data once.

(And this is a problem anyway, because how does composer-patches know a extension is installed and contains patches for some other extension - so data should be updated / composer should download these extensions first - but that's not happening - so the unresolved local patch locations are used)

In my case i tried to patch extension A from extension B (patch in extension B repo) and this failed because when installing extension A, extension B is not downloaded / installed yet - and the patch path resolving fails silently / just returns the relative path specified in extension B's composer.json file (without informing the user).

With the patch from #340 this changes: during the initial composer install it still fails, but the second time it finds the patch and applies it (from the previously downloaded version that's already installed). This is obviously wrong. Composer-patches should fail on the initial run just like how it fails on the second run - and using patches from the previous codebase should never occur (this could also be a nice note in the readme for local patches).

When testing this, make sure you use a clean environment without anything installed.

I've also tested relative local patches (relative to the root folder) and all patches inside the project's repo - both work fine. Relative: because the resolving always finds the patch. Project repo: because this is the first thing that's downloaded and installed and patches using a relative path are resolved from the project root, so this works-ish.

Moving everything to a webserver / private gist made my case work again (because then the patch is downloaded at the moment the extension is installed).

My suggestions: The local patches should get a note in the readme: Only use them in the root project composer.json / beware local patches + existing codebase. And the error message could be improved, so people don't waste lots of time on this. (but please focus on 2.x :-) )

[kanevbg]

This feature is a must for enhancing the development workflow, as it allows you to create your own package which requires another package and being able to fix it with local patch.

So from security and BC perspective I see no issues. Conditions to be met in order for package "my" to patch package "theirs" should be:

1. my requires "theirs"
2. my defines relative filepath (to the package by starting with ./) or URL Thus composer-patches verifies that package "my" requires "theirs" in order to allow patching "theirs" package with local patch file relative to "my" package.

@cweagans thoughts?

[cweagans]

main doesn't resolve patches from dependencies at all and I don't intend to update 1.x with this functionality. Sorry.