cweagans / composer-patches

Simple patches plugin for Composer
https://www.cweagans.net/project/composer-patches
BSD 3-Clause "New" or "Revised" License
1.52k stars 239 forks source link

Can't patch after update to MacOS Ventura #423

Closed amokato closed 1 year ago

amokato commented 1 year ago

I am not sure if this is actually an issue of this package or if it is caused somewhere else. Does anyone else has the problem, that while applying a patch (on MacOs Ventura) the process runs into a timeout? This happens only since the recent update.

After hanging on "File to patch" the following error is thrown:

File to patch: 
   Could not apply patch! Skipping. The error was: The process "patch '-p1' --no-backup-if-mismatch -d '/Users/mrl/Repositories/m2-composer-base/vendor/amasty/module-gift-card-account' < '/Users/mrl/Repositories/m2-composer-base/patches/composer/Amasty_GiftCardAccount_Fixes.patch'" exceeded the timeout of 300 seconds.
Downloading https://packagist.org/downloads/
[201] https://packagist.org/downloads/

  [Exception]                                                                                                         
  Cannot apply patch Fixed amasty broken namespaces (AMO-783) (patches/composer/Amasty_GiftCardAccount_Fixes.patch)!  
mikeohara commented 1 year ago

I have had this exact issue as well. I found by accident spamming the enter key while it runs works. So it seems like there is a user input that may be being suppressed.

I have found that adding the -n (--no-interaction) flag allows patches to apply just fine.

mikeohara commented 1 year ago

I also tried with the -v (--verbose) flag, and it didn't stall then either. Seems almost as if specifying any flag will "just make it work".

As I was writing this, I decided to try a couple of things. I rm -rf'd my web/core and web/contrib folders and kept re-running the composer install to see where it fell down. In my case, it started to fail on applying patches to drupal/core. Where it would stall and ask for the "File to patch":

image

and trying to step through it with the enter key results like so:

image

It seems like something has changed with perhaps the patch binary being used and currently rolled patches that is now causing an issue with it no longer being able to find the patch file locations in some cases. Example of a now-failing patch: https://www.drupal.org/files/issues/2022-07-24/2949017-99.patch

For reference, I'm including a screenshot of the successful patch action during composer run, if useful.

Screenshot 2022-11-01 at 11 42 31 AM

mikeohara commented 1 year ago

Last followup on this, this can be corrected fully by ensuring you are using the patchLevel directive in the extras section of your composer.json:

  "extra": {
    "patchLevel": {
      "drupal/core": "-p2"
    }
  }
}

Adding that corrected the stall, and correctly applied core and contrib patches.

dpi commented 1 year ago

Seeing one patch fail on core on my host. Running in an alpine container remains fine.,

"drupal/core": "-p2" set long ago.

Patch is https://www.drupal.org/files/issues/2019-11-11/2831233-83.patch, it has about 40 other patches its applying with just for core. So could either be new conflict behaviour, maybe the patch is corrupt and cached?...

amokato commented 1 year ago

In our case it happens with Magento. Spamming the enter key let's me finish too. The patch file can't be found in all the failing cases.

devetus commented 1 year ago

Same here, after Ventura update composer asks for "File to path:" in -vvv mode and is stuck otherwise.

Executing command (CWD): patch '-p2' --no-backup-if-mismatch -d '/Users/xyz/workspace/docker/vendor/some/module' < '/Users/xyz/workspace/patches/composer/some/module/some.patch'
 File to patch: 
amokato commented 1 year ago

Maybe this information is of any help. My Big Sur patch version is 2.5.8 and on Ventura it's 2.0-12u11-Apple.

mikeohara commented 1 year ago

Maybe this information is of any help. My Big Sur patch version is 2.5.8 and on Ventura it's 2.0-12u11-Apple.

On that note, what seemed to fix this for me with additional testing was installing the gnu patch via homebrew. (https://formulae.brew.sh/formula/gpatch) brew install gpatch which installs patch GNU patch 2.7.6

After which, no issues with patches applying, including the one that failed for @dpi

image

claudiu-cristea commented 1 year ago

@mikeohara solution from https://github.com/cweagans/composer-patches/issues/423#issuecomment-1301026697 is fixing the issue. Thank you!

shawnachieve commented 1 year ago

I can confirm that installing gpatch resolved this issue for me as well.

passioneight commented 1 year ago

For me, installing gpatch is not an option, so I tried all patch levels found in https://github.com/cweagans/composer-patches/blob/55c428957e791404ecfbe63c2bd9b43f57721550/src/Plugin/Patches.php#L111

and added them like so:

Last followup on this, this can be corrected fully by ensuring you are using the patchLevel directive in the extras section of your composer.json:

  "extra": {
    "patchLevel": {
      "drupal/core": "-p2"
    }
  }
}

Adding that corrected the stall, and correctly applied core and contrib patches.

Turns out, for me, -p4 is working (using Pimcore/Symfony as framework).

Some projects, though, have quite some patches. So, it's simply not feasible to add them all via the patchLevel directive. I figured, I'd give this a shot:

I have had this exact issue as well. I found by accident spamming the enter key while it runs works. So it seems like there is a user input that may be being suppressed.

I have found that adding the -n (--no-interaction) flag allows patches to apply just fine.

But it turns out, that the -n actually means --normal (and did not work):

image

Ultimately, this SO answer had me use -f (as in --force), which will avoid the interaction, an thus, loop through all available patch levels.

image

So, it works, because it does what we want:

It assumes the following: skip patches for which a file to patch cannot be found; [...]

But, simply adding -f in this repo may cause some unwanted side-effects, because -f does more than what we want:

It assumes the following: [...] patch files even though they have the wrong version for the "Prereq": line in the patch; and assume that patches are not reversed even if they look like they are. [...]

Any idea on how to proceed? Maybe the potential side-effects are negligible? After all, I'd expect developers to know what they are doing.

vijaycs85 commented 1 year ago

Thanks @mikeohara for https://github.com/cweagans/composer-patches/issues/423#issuecomment-1301026697 it worked for me.

aaronbauman commented 1 year ago

patch -p2 worked, but i don't really understand why this needs to be changed.

I can't update all my project composer files just so they work on my local - I need them to work in a docker / CI environment as well.

back-2-95 commented 1 year ago

For me the brew install gpatch solved all patching errors after updating to macOS Ventura

mladenrtl commented 1 year ago

I tried both solutions:

  1. brew install gpatch
  2. 
    "patchLevel": {
      "drupal/core": "-p2"
    }

The first option works just fine, and it's quick, and for the second one I have to list all components that have to be patched

wimleers commented 1 year ago

Related: https://github.com/cweagans/composer-patches/issues/326 — so you need brew install gpatch already anyway.

PR to check it: https://github.com/cweagans/composer-patches/pull/426

marclaporte commented 1 year ago

Related: https://github.com/cweagans/composer-patches/issues/262

mfb commented 1 year ago

Please review the PR @ #402 where we try to get the non-GNU version of patch working. I've successfully tested it on FreeBSD, but not MacOS, so it's possible some more logic is needed as far as looking at the PHP_OS_FAMILY constant. And since GNU patch could be installed on any operating system, perhaps it's better to check whether or not patch is GNU patch, rather than looking at operating system family.

arcanumbridge commented 1 year ago

I've tried to create a version that sniffs the patch version PR https://github.com/cweagans/composer-patches/pull/444

danepowell commented 1 year ago

Can someone please provide a minimum failing example for this issue (i.e. a composer.json with failing patch)? I use the stock patch utility with Ventura and have not had any problems applying patches. I only see problems if I specify the wrong patch level, which is to be expected.

joelpittet commented 1 year ago

@danepowell without gpatch installed this seems to fail everytime on my mac and no problem inside ddev container (same version of composer 2.5.1)

{
  "patches": {
    "drupal/core": {
      "Switching on aggregation generates fatal SQL error. Issue #2815881": "https://www.drupal.org/files/issues/2022-11-11/2815881-162.patch"
    }
  }
}

I'm on Ventura 13.2, on an M1 MBP, and this is reproducible on an older intel MBP that my colleague has, and not on Windows WSL2 (because ubuntu)

composer.json

    "name": "drupal/recommended-project",
    "description": "Project template for Drupal 9 projects with a relocated document root",
    "type": "project",
    "repositories": [
        {
            "type": "composer",
            "url": "https://packages.drupal.org/8"
        }
    ],
    "require": {
        "cweagans/composer-patches": "^1.0",
        "drupal/core-recommended": "^9.5"
    },
    "conflict": {
        "drupal/drupal": "*"
    },
    "minimum-stability": "dev",
    "prefer-stable": true,
    "config": {
        "sort-packages": true,
        "allow-plugins": {
            "composer/installers": true,
            "cweagans/composer-patches": true,
            "dealerdirect/phpcodesniffer-composer-installer": true,
            "drupal/core-composer-scaffold": true
        }
    },
    "extra": {
        "drupal-scaffold": {
            "locations": {
                "web-root": "public/"
            }
        },
        "installer-paths": {
            "public/core": [
                "type:drupal-core"
            ],
            "public/libraries/{$name}": [
                "type:drupal-library"
            ],
            "public/modules/contrib/{$name}": [
                "type:drupal-module"
            ],
            "public/profiles/contrib/{$name}": [
                "type:drupal-profile"
            ],
            "public/themes/contrib/{$name}": [
                "type:drupal-theme"
            ],
            "drush/Commands/contrib/{$name}": [
                "type:drupal-drush"
            ]
        },
        "composer-exit-on-patch-failure": true,
        "patchLevel": {
            "drupal/core": "-p2"
        },
        "patches-file": "composer.patches.json"
    }
}
danepowell commented 1 year ago

Okay, here's a minimum failing example that I can confirm works in Ubuntu but errors in Ventura without gpatch:

{
    "require": {
        "drupal/core": "^9.5",
        "cweagans/composer-patches": "^1"
    },
    "config": {
        "allow-plugins": {
            "cweagans/composer-patches": true
        }
    },
    "extra": {
        "patchLevel": {
            "drupal/core": "-p2"
        },
        "patches": {
            "drupal/core": {
                "Switching on aggregation generates fatal SQL error. Issue #2815881": "https://www.drupal.org/files/issues/2022-11-11/2815881-162.patch"
            }
        }
    }
}

Changing patchlevel to p1 causes the process to hang rather than erroring.

Installing gpatch causes p2 to work and p1 to error immediately, as expected.

mfb commented 1 year ago

We were curious if #444 fixes things for MacOS in addition to FreeBSD, can someone check? As possibly MacOS has a BSD version of patch (although maybe an older more dysfunctional version than FreeBSD..)

joelpittet commented 1 year ago

@mfb I manually applied it to 1.x and it would work if the condition was !== false because those options are for GNU. I've commented on the code here https://github.com/cweagans/composer-patches/pull/444#pullrequestreview-1280050423

danepowell commented 1 year ago

It appears this is fixed in master, but I haven't been able to determine in precisely which commit. I wasn't able to replicate the problem in master using the test case above.

So it seems like we either need to backport a fix or have a 2.x release. It looks like @cweagans is actively committing tonight, not sure if this is in the cards 😄

Edit: nevermind, see comment below... this is equally broken in 1.x and master, just obscured in master by Git patching.

cweagans commented 1 year ago

I'm inclined to just not support BSD patch, but not sure on that. 2.x is def not ready for release yet though.

danepowell commented 1 year ago

Ah... debugging reveals that the reason this works in master is because patching with Git succeeds, so it never even attempts to use the patch binary. Whereas Git patching fails in 1.x and it falls back to patch.

Hacking master to skip Git patching results in the exact same bug as in 1.x. So... ignore my comment above, this is equally broken in 1.x and master, though it might suggest a workaround if anyone wants to figure how to get Git patching working more reliably.

mfb commented 1 year ago

I'm inclined to just not support BSD patch, but not sure on that. 2.x is def not ready for release yet though.

re: FreeBSD support, #402 should be ready to commit, it's a minimal fix for bug in the existing BSD logic

arcanumbridge commented 1 year ago

@mfb @danepowell @cweagans Are we ok with the latest update for PR #444 ?

danepowell commented 1 year ago

Neither #402 nor #444 fix this issue in master if I force Composer Patches to actually use the patch logic.

I'm inclined to just not support BSD patch, but not sure on that. 2.x is def not ready for release yet though.

Is it an option to only support patching via Git? I don't know how robust that is, but it's certainly more robust in master than 1.x, maybe we can backport it. I think the cyclomatic complexity of multiple patch methods just makes it a nightmare to support and debug.

danepowell commented 1 year ago

Alright, I think I've got a solid grasp on the issue here and have a PR to fix it: https://github.com/cweagans/composer-patches/pull/445

However this still fails on the test case above because the Ventura version of patch seems to be more strict than gpatch, and that particular Drupal patch works fine with gpatch while failing with patch. I don't know if that's something we need to address or not.

Also in master, Composer Patches completely ignores the defined patch version and loops through them iteratively. Not sure if that's by design or not.

mfb commented 1 year ago

I tested https://www.drupal.org/files/issues/2022-11-11/2815881-162.patch on FreeBSD and yes, patch fails on just one hunk of the yml file. I'm by no means an expert on patch files but taking a quick look, I'd consider it an advanced-level patch file as it moves some text between files and then adds another patch on the same file.

On FreeBSD (and it sounds like on Mac too), GNU patch is installed by the package system as gpatch, so maybe the logic should be: Try using git to patch, if that's not found then try gpatch, and if that's not found then try patch? The non-GNU patch should work for most patch files out there - but apparently there are some edge cases (and, one hopes they'll eventually fix these incompatibilities?)

danepowell commented 1 year ago

Brew links gpatch as just patch in the PATH (higher than macOS patch). While we might be able to bypass PATH and look for gpatch directly, I don't think it would be a good idea.

mfb commented 1 year ago

Well then, I guess a fallback would be to try executing gpatch if patch fails. I'm personally not too worried about these rare edge cases though, as I'd probably just reroll the patch if I hit one. I was mainly focused on fixing the command-line flags for FreeBSD.

cweagans commented 1 year ago

I think main now handles this properly. Let us know if that's not the case and we can reopen!

joelpittet commented 1 year ago

@cweagans There may be changes needed to get things working with main, I tried the MVC from https://github.com/cweagans/composer-patches/issues/423#issuecomment-1412444347 with main and it just says

  - Installing drupal/core (9.5.3): Extracting archive
  - Patching drupal/core
      - Found cached patch at /Users/pittet/Library/Caches/composer/patches/3c7513e90ce29f656f2bbe5b4b2f7a734bbf9bf208f6a0a33e5ef2a33174c767.patch

In Patches.php line 270:

  No available patcher was able to apply patch https://www.drupal.org/files/issues/2022-11-11/2815881-162.patch to drupal/core
cweagans commented 1 year ago

@joelpittet roger that. I'll take a look.

cweagans commented 1 year ago

I've been thinking: it might be better to just sidestep the entire issue and rely strongly on git for applying patches (#471 has a way to do that, even for dependencies that are not installed through a git clone) and then have an escape hatch for people who want/need to do something special (#472 allows you to run an arbitrary command/script to apply a patch).

cweagans commented 1 year ago

I ended up merging #472, so I'm going to close this again. In main, we don't use patch anymore at all. It's all just git + an escape hatch that you can use to apply patches however you'd like. Feedback on this is welcome - we can always revert all or part of that PR if this is a dealbreaker.

nchhantbar commented 1 year ago

For me the brew install gpatch solved all patching errors after updating to macOS Ventura

preciado04 commented 1 year ago

July 4 2023, and I still have issues when I try to patch a module. Some patches are asking me a file path when I run "composer install -vvv"

cweagans commented 1 year ago

Please try the 2.0.0 beta and see if it works for you. You can also install GNU patch (brew install gpatch).

You're welcome to debug further and open a pull request if needed as well. Contributions are welcome.

preciado04 commented 1 year ago

Please try the 2.0.0 beta and see if it works for you. You can also install GNU patch (brew install gpatch).

You're welcome to debug further and open a pull request if needed as well. Contributions are welcome.

Thanks for your fast answer.

Finally, it works! I used the URL to the patch (https://www.drupal.org/files/issues/2023-04-13/quick-node-clone-3352168-groups-v2_0.patch)

Here my JSON: "drupal/quick_node_clone": { "Fix bug for conflict between group and quick node clone": "https://www.drupal.org/files/issues/2023-04-13/quick-node-clone-3352168-groups-v2_0.patch" }

But when I was having issues, my JSON was this: "drupal/quick_node_clone": { "Fix bug for conflict between group and quick node clone": "patches/quick-node-clone-3352168-groups-v2_0.patch" }

Do you know why this doesn't work with local files?

This is my composer version: Composer version 2.5.8 2023-06-09 17:13:21

This is my OS version: macOS Ventura Version 13.2.1

cweagans commented 1 year ago

It definitely works with local files. The path is relative to the root of your project. So you'd need a patches dir as a sibling to composer.json and friends.

SebastienGicquel commented 1 year ago

brew install gpatch solved all patching errors on MacOS Ventura

damienmckenna commented 11 months ago

👍 for brew install gpatch, thank you all for the tip.

Dave2084 commented 10 months ago

Just to add to this, I just moved from a soon to be retired iMac that was stuck on Big Sur (with patch 2.5.8) to Sonoma (with patch 2.0-12u11-Apple) and when setting up a load of sites patches were not working.

Initially I though it was because I'd moved from Valet to Herd so reinstalled valet but then found this issue report and brew install gpatch did the business! 👍🏻