Only have git patchers and freeform patcher?

[cweagans]

Description

Continuation of #471. If the Git patcher works for everything, why even bother with patch? We could potentially use the git patchers for everything and people can use the FreeformPatcher that this PR adds to run arbitrary commands to apply other patches in some way.

I'm not sure if this is the right thing to do though.

Related tasks

• Documentation has been updated if applicable
• Tests have been added
• Does not break backwards compatibility OR a BC break has been discussed in the related issue(s).

Other notes

[coveralls]

Pull Request Test Coverage Report for Build 4159058115

• 41 of 50 (82.0%) changed or added relevant lines in 5 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+1.3%) to 87.413%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/Patcher/FreeformPatcher.php	19	28	67.86%

Files with Coverage Reduction	New Missed Lines	%
src/Patcher.php	1	88.37%

Totals
Change from base Build 4155475134:	1.3%
Covered Lines:	500
Relevant Lines:	572

---

💛 - Coveralls

[danepowell]

I tested this on our project, it worked the first install (possibly because the library already existed on disk) but after rm -rf vendor && composer install I got this error:

No available patcher was able to apply patch https://patch-diff.githubusercontent.com/raw/consolidation/self-update/pull/21.patch to consolidation/self-update

I'm not sure what that error means exactly. Is there a problem with the patch? It would be pretty bad if patches directly from GitHub can't be applied.

Let me know if you want a separate issue for this.

[cweagans]

@danepowell I cloned your project and gave it a try with composer install -vvv. It looks like the patch is referring to a path that doesn't exist (src/Symfony/Component/Filesystem/Filesystem.php) in vendor/symfony/filesystem. main doesn't guess patch depth anymore, so you have to be explicit about what you want. You can either:

• Change the patch to be relative to the root of vendor/symfony/filesystem
• Change your patch definition to include the depth for that specific patch:

"extra": {
        "patches": {
            "symfony/filesystem": [
                {
                    "description": "patch does something",
                    "url":  "assets/symfony-fs-mirror.patch",
                    "depth": 5
                }
            ],
        },
}

• Set the depth for the package (as you had done previously -- the config lives in a different place now):

    "extra": {
        "patches": {
            "symfony/filesystem": [
                "assets/symfony-fs-mirror.patch"
            ],
        },
        "composer-patches": {
            "package-depths": {
                "symfony/filesystem": 5
            }
        }
    },

---

After getting that patch to apply, it looks like there is another that is failing -- https://patch-diff.githubusercontent.com/raw/consolidation/self-update/pull/21.patch

This one (I think) is a legit failure. That patch was added to your composer.json 5/6/22. The version that composer is downloading includes consolidation/self-update@735ea26, which changes SelfUpdateCommand.php inside of the context that the patch provides, so it doesn't cleanly apply. I'm not seeing a way to tell git to tolerate this, but maybe updating consolidation/self-update#21 would solve that?

I didn't go further than that -- does that help?

[danepowell]

Thanks. For the symfony/filesystem patch, I tried setting the package depth both ways you suggested and no matter what Composer Patches passed -p1 to Git and the patch failed. I pushed a new commit so you can see: acquia/cli@a4d24c3

I'll concede that the consolidation/self-update patch may be borked.

Also this was hard to debug because the errors changed every time I ran composer install until they finally went away, but patching was still failing. Removing patches.lock didn't force Composer Patches to re-attempt patching, I had to rm -rf vendor for that. I think if patching fails, it should fail the same way every time you run composer install.

[danepowell]

Oh I figured it out, Composer Patches was completely ignoring my composer.json changes because patches.lock existed. In the end, removing all of my composer, vendor, and patches.lock files and setting the depth correctly fixed it. But yikes, it shouldn't be that complicated to debug.

Maybe you should store a hash of composer.json inside patches.lock so that you can make sure to pick up changes to composer.json. This is what composer.lock obviously does, and composer.lock and patches.lock are effectively siblings (I understand you created patches.lock just because you couldn't store arbitrary data in composer.lock).

[cweagans]

The plugin works a little differently now -- https://docs.cweagans.net/composer-patches/usage/recommended-workflows/

Not automatically deleting stuff was an explicit goal -- that caused a lot of problems for people in the past. I agree that it should have warned you that patches.lock should be updated though. Opened #479