Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wimleers opened this issue Aug 17, 2020 · 20 comments

Comments

@wimleers
Copy link

wimleers commented Aug 17, 2020

@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 #172 (comment)

The solution:

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

Originally posted by @cweagans in #172 (comment)

@andypost
Copy link

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

@cweagans
Copy link
Owner

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

@wimleers
Copy link
Author

wimleers commented May 4, 2022

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.

@joejoseph00
Copy link

joejoseph00 commented Nov 18, 2022

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.

alfredbez added a commit to alfredbez/composer-patches that referenced this issue Dec 9, 2022
@marclaporte
Copy link

Related: #262

@mfb
Copy link

mfb commented Jan 27, 2023

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

@danepowell
Copy link
Collaborator

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.

@cweagans
Copy link
Owner

cweagans commented Feb 6, 2023

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 cweagans closed this as completed Feb 6, 2023
@joejoseph00
Copy link

joejoseph00 commented Feb 6, 2023

@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
Copy link

joejoseph00 commented Feb 6, 2023

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.

@cweagans
Copy link
Owner

cweagans commented Feb 6, 2023

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.

@joejoseph00
Copy link

joejoseph00 commented Feb 6, 2023

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.

@cweagans
Copy link
Owner

cweagans commented Feb 6, 2023

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).

@joejoseph00
Copy link

@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
Copy link

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

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

@joejoseph00
Copy link

joejoseph00 commented Feb 6, 2023

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
Copy link

joejoseph00 commented Feb 6, 2023

@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.

@cweagans
Copy link
Owner

cweagans commented Feb 6, 2023

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 cweagans mentioned this issue Feb 7, 2023
31 tasks
@weitzman
Copy link

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

@mr-m0nst3r
Copy link

brew link gnu-getopt --force solved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants