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

Patch fails when created with v6.0.0-5 #53

Closed
gonzaloserrano opened this issue May 10, 2018 · 11 comments
Closed

Patch fails when created with v6.0.0-5 #53

gonzaloserrano opened this issue May 10, 2018 · 11 comments

Comments

@gonzaloserrano
Copy link

Hi,

First, congrats for the project, I think helps a lot the Developer Experience (DX).

So, after running into this whitespace issue #36, I moved to the beta npm channel but got another one:

The patch is created OK:

❯ cat patches/localforage+1.7.1.patch
diff --git node_modules/localforage/src/drivers/indexeddb.js node_modules/localforage/src/drivers/indexeddb.js
index c77786f..f54a612 100644
--- node_modules/localforage/src/drivers/indexeddb.js
+++ node_modules/localforage/src/drivers/indexeddb.js
@@ -664,7 +664,12 @@ function setItem(key, value, callback) {

                             resolve(value);
                         };
-                        transaction.onabort = transaction.onerror = function() {
+
+                        transaction.onerror = function() {
+                            reject(req.error);
+                        };
+
+                        transaction.onabort = function() {
                             var err = req.error
                                 ? req.error
                                 : req.transaction.error;

But yarn patch-package fails with:

❯ yarn patch-package
yarn run v1.5.1
$ /Users/gonzalo/dev/lernin/game-wrapper-app/node_modules/.bin/patch-package
patch-package: Applying patches...

**ERROR** Failed to apply patch for package localforage

  This error was caused because patch-package cannot apply the following patch file:

    patches/localforage+1.7.1.patch

  If removing node_modules and trying again doesn't fix this, maybe there was
  an accidental change made to the patch file? If not, then it's probably a bug
  in patch-package, so please submit a bug report. Thanks!

    https://github.com/ds300/patch-package/issues


error An unexpected error occurred: "Command failed.
Exit code: 1
Command: sh
Arguments: -c /Users/gonzalo/dev/lernin/game-wrapper-app/node_modules/.bin/patch-package
Directory: /Users/gonzalo/dev/lernin/game-wrapper-app
Output:
".
info If you think this is a bug, please open a bug report with the information provided in "/Users/gonzalo/dev/lernin/game-wrapper-app/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Aplying the patch manually works:

❯ patch -p0 < patches/localforage+1.7.1.patch
patching file node_modules/localforage/src/drivers/indexeddb.js

Thank you.

@ds300
Copy link
Owner

ds300 commented May 10, 2018

Hi! 👋! Thanks for the report!

There seems to be an issue with the way the patch file was generated on your machine. It is missing the a/ andb/ prefixes for the file paths. i.e. it should look like this:

❯ cat patches/localforage+1.7.1.patch
diff --git a/node_modules/localforage/src/drivers/indexeddb.js b/node_modules/localforage/src/drivers/indexeddb.js
index c77786f..f54a612 100644
--- a/node_modules/localforage/src/drivers/indexeddb.js
+++ b/node_modules/localforage/src/drivers/indexeddb.js
@@ -664,7 +664,12 @@ function setItem(key, value, callback) {

                             resolve(value);
                         };
-                        transaction.onabort = transaction.onerror = function() {
+
+                        transaction.onerror = function() {
+                            reject(req.error);
+                        };
+
+                        transaction.onabort = function() {
                             var err = req.error
                                 ? req.error
                                 : req.transaction.error;

I think this is supposed to be part of the git spec: https://git-scm.com/docs/git-diff#_generating_patches_with_p but maybe there are certain environmental factors which could prevent these prefixes from being included. Are you aware of anything?

Otherwise, it would be great to know which OS and git version you're working with. As well as any custom global git configuration you can share.

@gonzaloserrano
Copy link
Author

Good to know!

@gonzaloserrano
Copy link
Author

Digged a little and probably is because I'm using custom diff stuff to colorize etc, so maybe adding a flag here

tmpExec("git", ["diff", "--cached", "--name-only"])
like git diff --no-ext-diff could work (haven't tried).

@ds300
Copy link
Owner

ds300 commented May 10, 2018

I think this is the culprit:

https://github.com/gonzaloserrano/configs/blob/0de92c77268bfec42e4d82cab6442e4fb2e474d1/gitconfig#L40

I didn't realise this was an option until now :) Out of interest, why do you have that option set?

@gonzaloserrano
Copy link
Author

Thank you! That must be it.

Probably it was done when I tried this tool https://github.com/so-fancy/diff-so-fancy.

I will get rid of it and try again.

@AlexHuicu
Copy link

AlexHuicu commented Jul 23, 2018

This is also happening to us.
I tried to use this version in order to fix the "inconsistent white spaces" problem, but run into this one.

It's happening when someone updates/creates a patch on mac and that patch is applied on a windows machine.
This is the log:
2018-07-23T10_17_25_952Z-debug.log

This is the smallest project in which the problem occure. I've included an old patch that was working and the new one, which throws the error above, with my node modules:
https://drive.google.com/open?id=1xnNnOMWR6VJ-x6QC3BbZsS0jT3odgHXN

This is the project without node modules:
patch-test.zip

yarn version:1.7.0
node version: 8.11.3
npm version: 5.1.0
lerna version:2.11.0 (i belive not necessary)
Windows 10 Pro, version 10.0.17134 Build 17134

The patch can by applyed by running "lerna bootstrap" in the main folder, or by runnint "git apply.." in the "first" forder. The result is the same

If you need more details, I m happy to help.

@AlexHuicu
Copy link

I tried to bypass this error by reverting the patch to the old one, making the saves myself and running "patch-package".
It applyed it correctly and the app worked. The first time.

Unfortunately, the second time I run yarn, the patch-package failed with the "inconsistent whitespace" problem.
My workaround is to delete the patched node module every time I run yarn.

This is the newly created patch, on the windows machine(I changed it's extension to be able to upload it here):

react-native-charts-wrapper+0.4.3.txt

@AlexHuicu
Copy link

Edit:
The workaround did not work. I'm not able to build for a few hours know

@ds300
Copy link
Owner

ds300 commented Jul 23, 2018

Hi @alexhuiculescu ! 👋 Thanks for the report!

I just got back from a long holiday and have a lot of issues to work through over the next few days. Rest assured this one is a high priority. Thanks for your patience. 🙏

Which cli environment are you using on Windows?

@AlexHuicu
Copy link

I'm using VSCode with cmd as default terminal

@AlexHuicu
Copy link

Edit: using the built in VSCode command to replace LF with CRLF on the commited files did the trick. I've just made the changes myself while standardizing the end lines with Windows end-line style

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

No branches or pull requests

3 participants