composer-patches icon indicating copy to clipboard operation
composer-patches copied to clipboard

Requires GNU patch 2.7 for correct behavior, but `composer-patches` does not warn the user about this!

Open wimleers opened this issue 3 years ago • 3 comments

@cweagans holy shit, the need for brew install gpatch took me many hours to figure out, because for a particular project TravisCI builds were succeeding but local builds were failing. 😬 Can we make cweagans/composer-patches detect the patch version, and explicitly warn the user that any version below 2.7 may result in silent failures?

Originally posted by @wimleers in https://github.com/cweagans/composer-patches/issues/172#issuecomment-674860594

The solution:

brew install gpatch (you'll need Homebrew installed for that to work (https://brew.sh))

Originally posted by @cweagans in https://github.com/cweagans/composer-patches/issues/172#issuecomment-587147692

wimleers avatar Aug 17 '20 12:08 wimleers

Warning could lead to faq wiki page instead of patch build detection

andypost avatar Aug 17 '20 12:08 andypost

Definitely not opposed to a warning here if there's a reliable way to detect GNU Patch vs whatever macOS ships with.

cweagans avatar Aug 17 '20 22:08 cweagans

This is still a problem. A colleague of mine wasted another hour on this.

To answer your question about detecting GNU patch:

$ which patch
/usr/local/bin/patch
$ /usr/bin/patch --version
patch 2.5.8
Copyright (C) 1988 Larry Wall
Copyright (C) 2002 Free Software Foundation, Inc.

This program comes with NO WARRANTY, to the extent permitted by law.
You may redistribute copies of this program
under the terms of the GNU General Public License.
For more information about these matters, see the file named COPYING.

written by Larry Wall and Paul Eggert
$ /usr/local/bin/patch --version
GNU patch 2.7.6
Copyright (C) 2003, 2009-2012 Free Software Foundation, Inc.
Copyright (C) 1988 Larry Wall

License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Larry Wall and Paul Eggert

(/usr/bin/patch ships with macOS, /usr/local/bin/patch was installed by brew install gpatch.)

→ I am pretty sure that you could do something like preg_match('/^GNU patch/', …) on the output of patch --version.

wimleers avatar May 04 '22 09:05 wimleers

Some colleagues are using brew ( I use Ubuntu mostly )

Here's the easy solution for Macintosh brewers that need an upgraded version of gpatch/patch to allow advanced patch capabilities and prevent composer failures:

brew install gpatch;
brew link gpatch --force --overwrite;

Then restart terminal or reboot, try again, patch version should show up as 2.7.6

This resolves the 'Only garbage found' when applying certain types of patches specifically ones that move files around.

joejoseph00 avatar Nov 18 '22 23:11 joejoseph00

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

marclaporte avatar Jan 26 '23 03:01 marclaporte

Please review the PR @ #402 where we try to get the non-GNU version of patch working

mfb avatar Jan 27 '23 01:01 mfb

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.

danepowell avatar Jan 27 '23 16:01 danepowell

Let's just close this. #447 has explicit support and detection for different versions of patch. The behavior in 2.x is going to be pretty different: the goal is to get the patch applied, and the mechanism for actually performing that work doesn't matter as much. As long as there is a usable mechanism for applying a patch, the patch should be applied.

cweagans avatar Feb 06 '23 01:02 cweagans

@cweagans

Let's just close this. #447 has explicit support and detection for different versions of patch. The behavior in 2.x is going to be pretty different: the goal is to get the patch applied, and the mechanism for actually performing that work doesn't matter as much. As long as there is a usable mechanism for applying a patch, the patch should be applied.

What I've noticed is colleagues using MacintApples are using brew and on the MacintApple the version of patch is an older version that has problems applying patches that move files around. Test this by writing a patch that moves the same file from one folder to another.

Example:

git mv README.txt src/.
git diff HEAD > MacintAppleChokesByDefault.patch;

include this patch with your project and ask a MacintApple person to run the composer install.

Unless they upgrade to use gpatch version 1.7.6 their system will fail to execute a move file(s) patch .

joejoseph00 avatar Feb 06 '23 04:02 joejoseph00

before patch version: patch --version;

output might look something like 2.0-12u11-Apple this version is unable to move files

To fix MacintApple brew environments use the following commands:

brew install gpatch;
brew link gpatch --force --overwrite;

after upgrading the command patch --version should output 2.7.6

This version of patch has no problem moving files around.

joejoseph00 avatar Feb 06 '23 04:02 joejoseph00

In that scenario (even in 1.x), Git should be able to apply the patch properly before it ever gets to patch, right? I've made a note to add a test for this scenario.

cweagans avatar Feb 06 '23 05:02 cweagans

cweagans, yes that is likely true that git apply could be a better cross platform option but I'm not sure how to make git behave like patch , where patch applies to the current folder but git applies to the root respository which is not predictable. Not only that, composer can include some git repos and other projects that are not git repos.

joejoseph00 avatar Feb 06 '23 05:02 joejoseph00

Do you have "config": {"preferred-install": "source"}, in your composer.json? I recommend that in the readme specifically so that there is a higher chance of being able to use git for patching. Even so, my intent with 2.x is that the specific mechanism for applying patches shouldn't matter. If there aren't any suitable mechanisms available, the plugin will make noise about not being able to apply the patch.

One other idea that I've been kicking around is to distribute a version of patch compiled like this so that there is a valid patcher no matter what. That might not be 2.0.0 though (and may not even be in the core plugin).

cweagans avatar Feb 06 '23 06:02 cweagans

@cweagans no I don't have that entry

I have this instead:

  "config": {
      "allow-plugins": {
        "composer/installers": true,
        "cweagans/composer-patches": true,
        "oomphinc/composer-installers-extender": true,
        "drupal/console-extend-plugin": true,
        "drupal/core-composer-scaffold": true,
        "dealerdirect/phpcodesniffer-composer-installer": true
      }
  }

joejoseph00 avatar Feb 06 '23 06:02 joejoseph00

ok I'll add that config option and try it out. We're using cweagans version 1.x

  "config": {
    "preferred-install": "source"
  },

joejoseph00 avatar Feb 06 '23 07:02 joejoseph00

After adding it will look like this.

  "config": {
      "allow-plugins": {
        "preferred-install": "source",
        "composer/installers": true,
        "cweagans/composer-patches": true,
        "oomphinc/composer-installers-extender": true,
        "drupal/console-extend-plugin": true,
        "drupal/core-composer-scaffold": true,
        "dealerdirect/phpcodesniffer-composer-installer": true
      }
  }

joejoseph00 avatar Feb 06 '23 07:02 joejoseph00

@cweagans wow facinating link you shared https://justine.lol/ape.html

this would be really cool to have a single executable do everything cross platform. With that said, windows filesystems have a real tough time with Linux/BSD-like symlinks but most things would work. It'd be good to actually fix the MacintApple brew problem this way though.

joejoseph00 avatar Feb 06 '23 07:02 joejoseph00

After adding it will look like this.


  "config": {

      "allow-plugins": {

        "preferred-install": "source",

        "composer/installers": true,

        "cweagans/composer-patches": true,

        "oomphinc/composer-installers-extender": true,

        "drupal/console-extend-plugin": true,

        "drupal/core-composer-scaffold": true,

        "dealerdirect/phpcodesniffer-composer-installer": true

      }

  }

Preferred-install should be one level up (a sibling of allow-plugins).

cweagans avatar Feb 06 '23 17:02 cweagans

Just got bitten by this. Thanks for posting the brew solution

weitzman avatar Mar 22 '23 22:03 weitzman

brew link gnu-getopt --force solved

mr-m0nst3r avatar May 07 '23 02:05 mr-m0nst3r