Closed rt2zz closed 6 years ago
Thanks for the report! Hope you don't mind answering a couple more questions:
Since the patch always applied fine locally, unfortunately I did not have the constitution to debug further on travis. My local git is 2.15
Thanks for those details. I'm going to look into this over the next week or so. Probably going to end up doing pure-JS patch application if I can. git-apply
has been giving me a lot of grief -_-
cool, I love this package btw! thank you.
FWIW v3 is working great atm
I think I'm having the same issue, but downgrading to v3 doesn't fix the problem for me:
I get the following error after 'yarn':
**ERROR** Failed to apply patch for package react-native-maps
This error was caused because Git cannot apply the following patch file:
patches/react-native-maps+0.19.0.patch
This is usually caused by inconsistent whitespace in the patch file.
If I manually try to apply the patch with "git apply -v --ignore-whitespace patches/react-native-maps+0.19.0.patch":
Checking patch node_modules/react-native-maps/lib/components/MapView.js.orig...
Checking patch node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMap.m...
Checking patch node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMapManager.m...
error: while searching for:
@interface AIRGoogleMapManager() <GMSMapViewDelegate>
{
BOOL didCallOnMapReady;
}
@end
error: patch failed: node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMapManager.m:32
error: node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMapManager.m: patch does not apply
If I check AIRGoogleMapManager.m:32 the content it's looking for seems to be there, but it's still failing somehow... If I apply using GNU patch with --ignore-white-space/-l set it does apply (ignore me forgetting to specify it's a unified diff ;-)):
Hmm... Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|patch-package
|new file mode 100644
|--- /dev/null
|+++ b/node_modules/react-native-maps/lib/components/MapView.js.orig
--------------------------
Patching file node_modules/react-native-maps/lib/components/MapView.js.orig using Plan A...
Hunk #1 succeeded at 1.
Hmm... The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- a/node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMap.m
|+++ b/node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMap.m
--------------------------
Patching file node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMap.m using Plan A...
Hunk #1 succeeded at 34.
Hunk #2 succeeded at 50.
Hunk #3 succeeded at 152.
Hmm... The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- a/node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMapManager.m
|+++ b/node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMapManager.m
--------------------------
Patching file node_modules/react-native-maps/lib/ios/AirGoogleMaps/AIRGoogleMapManager.m using Plan A...
Hunk #1 succeeded at 32.
Hunk #2 succeeded at 318.
done
Thanks for this @mauritsd — I should have some time this weekend to take a look at making the patch application js-based rather than using Git, which seems to have quite flaky behaviour across platforms and versions.
I looked into doing pure-js patch application and I think it would be a lot of work and there would be a bunch of edge cases to deal with, some of which I'm probably not going to anticipate before publishing.
Something I could do to mitigate this problem in the meantime would be to use patch
to apply patches on *nix machines and fall back to git apply
on windows. For some reason patch
seems to be a lot less flaky than git apply
That'd be fine; I'm also on macOS.
Is there a definitive diagnosis as to why 'git apply' fails? I'll have a look at the patch file and the destination file with a hex editor...
I've figured out why it fails in my particular instance. When patch-package generates the diff it specifies the --ignore-space-at-eol option to git-diff. Through testing I've found out that this, when coupled with indented lines without content (e.g. " \n"), will cause git-diff to generate patch files which are not accepted by git-apply, even with --ignore-whitespace and friends. If I remove the --ignore-space-at-eol option from patch-package the patch file is a bit less clean but does apply.
Maybe an idea to allow the user to specify which git-diff options to use?
Thanks for the investigation! Maybe I should just not use the --ignore-space-at-eol flag? Allowing configuration of the git command would let people solve these issues on their own, but man I'd love to have something that just works for everyone. Feels like that should exist too.
On Tue, 30 Jan 2018, 15:52 Maurits Dijkstra, notifications@github.com wrote:
I've figured out why it fails in my particular instance. When patch-package generates the diff it specifies the --ignore-space-at-eol option to git-diff. Through testing I've found out that this, when coupled with indented lines without content (e.g. " \n"), will cause git-diff to generate patch files which are not accepted by git-apply to fail, even with --ignore-whitespace and friends. If I remove the --ignore-space-at-eol option from patch-package the patch file is a bit less clean but does apply.
Maybe an idea to allow the user to specify which git-diff options to use?
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/ds300/patch-package/issues/36#issuecomment-361637048, or mute the thread https://github.com/notifications/unsubscribe-auth/ABL1qdeMznUVIzjdwFoeG7peBX7TVN5Xks5tPzqfgaJpZM4Q-Wpv .
I think the 'right' course of action is to fix git-apply, but that's more of a long term solution...
By the way, git-merge does have an option to ignore spaces at EOL (-Xignore-space-at-eol).
Same issue here with v5 and CircleCI. Patches appear flaky from one build to another (e.g. it succeeds if I install packages on my local Mac using Yarn with --frozen-lockfile
, but the same command fails in Circle). Is there a suggested workaround? Thx
EDIT: the problem reappears even when I regenerate the patch (either via patch-package
and diff -u
) on a local Linux system.
Moreover, downgrading to v5.0.0 (which has been working for me for an entire month) does fix the problem. So it seems there might have been some regression in the last 2 releases (5.1.0/5.1.1).
Thanks for the report. Weird that for you 5.1 is flaky but 5.0 is not. I didn't change the way git apply
is invoked except to potentially re-order the cli options. It would be funny if that's causing the issue.
As a temporary workaround you could call patch
instead of patch-package in your prepare hook. See the README for an example. I'm planning to make patch-package use it by default soon.
Hmm, I've just had this problem with 5.0 as well. Updated to 5.1.1 and now it works again! Not sure why it was misbehaving earlier - will update if anything changes again. Thanks
😂 😩 Must find some time to fix this soon. Thanks for the update.
I think it has something to do with caching of node_modules
(that is what I use in CircleCI at least). If yarn.lock
hasn't changed, my config will re-use the cached versions of all packages. But if a package was updated, even though I run yarn --frozen-lockfile
in CI, it may still update that package without an error. Not sure how this would lead to inconsistency with the applied patch (apart from direct incompatibility with the new version of the package, though I do check that it works on my local system before CI). But purging the cache seems to help (not always though, it seems).
I'm having this bug. I have 4 old patches that work fine, but the 5th doesn't:
patch-package: Applying patches...
react-native@0.54.0 ✔
**ERROR** Failed to apply patch for package react-native-camera
This error was caused because Git cannot apply the following patch file:
patches/react-native-camera+1.0.2.patch
This is usually caused by inconsistent whitespace in the patch file.
macOS 10.13.3 nodejs 8.9.0 npm 5.7.1 patch-package 5.1.1 react-native 0.54.0 react-native-camera 1.0.2 (pull request)
You can compare with the pull request and see some numbers are off.
I made the changes again on the file above, this time it decided to work 🤔 The difference between the new and old patches are indeed only spaces:
How to prevent this from happening again?
I was also having this issue. I tried downgrading versions and all kinds of things which didn't work. Removing the cache key and all the lines it included from travis.yml resolved the problem.
Here is the full list of steps I took before reaching this solution:
I think the step to delete the original patch may not have been necessary, but I am not entirely sure. Definitely the step for patch #2 was not useful. I hope any of this helps anyone else having this issue.
FWIW, not sure if the same could be done in Travis, but in CircleCI it's easy enough to update the cache key by just adding a number to it, like so:
- restore_cache:
keys:
- cache-{{ checksum "yarn.lock" }}-1
- cache-
One can always just increment the number if the cache key needs to be busted again. Hope that helps.
I'll keep that in mind next time around, thanks for the tip @dinvlad.
I hope this issue finds a solution, this is an incredibly useful module. @ds300 , thanks so much for the great work on this!
Thanks for the detailed tips Liisa!
Update for this thread: Last night I made a lot of progress working on custom pure-TypeScript patch application. That will make these errors go away soon.
On Thu, 22 Mar 2018, 08:28 Liisa Duerig-Laitinen, notifications@github.com wrote:
I'll keep that in mind next time around, thanks for the tip @dinvlad https://github.com/dinvlad.
I hope this issue finds a solution, this is an incredibly useful module. @ds300 https://github.com/ds300 , thanks so much for the great work on this!
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/ds300/patch-package/issues/36#issuecomment-375216238, or mute the thread https://github.com/notifications/unsubscribe-auth/ABL1qQEqCsTc1vKjkVYxBta0f7MbmAKmks5tg2CXgaJpZM4Q-Wpv .
Another update for this thread: You can try out the 6.0.0-X pre-release builds, which use the new TS-only patch application (yay! no more git apply
🎉). It should fix these errors in 99.9% of cases. I have a few more bits to iron out before making a full release. You can follow progress in #45
Oh no. Apparently the publish didn't go through last night. Should work now.
npm i --save-dev patch-package@beta
or
yarn add --dev patch-package@beta
@ds300 I was having the same issue and for now, I can confirm the beta works :)
@ds300 The @beta is pointing to this version (and I guess the good one is v6.0.0-3):
https://github.com/ds300/patch-package/releases/tag/v3.5.3-0
Weird, for me it's pointing to the correct one, v6.0.0-3
Are you using yarn or npm?
Also this:
@ds300 Maybe it's because we are behind a private npm repo but we (both of us) only have the chance to install up to 5.1.1. Please, ignore this if we are the only ones reporting the problem.
Update: indeed it was, fixed now by our internal IT team =) Update2: v6.0.0-3 fixed the white space problem. Thanks!
Is this fixed and can it be closed?
Yes! It is fixed in patch-package@beta
. Need to find some time to push out a full release soon. Might as well close this now though, since it's not tied to a specific issue but a kind of genre of issues that I don't have to worry about any more :)
i'm getting the same with a patch that was supposedly working for over a month, nothing updated that i know of...
v5.1.1
This problem also occurs when someone creates a patch with npm/npx and his colleague tries to apply the patch with Yarn..
Maybe we can implement a notice for such a human problems in the fail message?
I get the following
when CI tries to apply the patch that I created locally (OSX) with patch package 5. Downgrading to v3 resolves the issue.