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

Failure when attempting to make patch #229

Closed
kf6kjg opened this issue May 1, 2020 · 6 comments · Fixed by #271
Closed

Failure when attempting to make patch #229

kf6kjg opened this issue May 1, 2020 · 6 comments · Fixed by #271

Comments

@kf6kjg
Copy link
Contributor

kf6kjg commented May 1, 2020

$ npx patch-package serverless-webpack
patch-package 6.2.2
• Creating temporary folder
• Installing serverless-webpack@5.3.1 with npm

{
  status: 243,
  signal: null,
  output: [ null, <Buffer >, <Buffer > ],
  pid: 29938,
  stdout: <Buffer >,
  stderr: <Buffer >,
  error: null
}
undefined

Same problem occurs no matter which package I attempt to make a patch for.

Ubuntu 18.04.3 LTS
Node and NPM installed via

snap install --classic node --channel=14/stable

resulting in

$ snap list
Name               Version                     Rev   Tracking       Publisher     Notes
...redacted...
node               14.1.0                      2687  14/stable      nodesource✓   classic
...redacted...
@kf6kjg
Copy link
Contributor Author

kf6kjg commented May 18, 2020

I've spent the better part of this morning digging into the code and it all comes back to something in the intersection of npm and node's spawnSync. Hazarding a guess I suspect it's a buffering issue similar to what I met in serverless-heaven/serverless-webpack#584, and described in nodejs/node#2926.

Oh, and my current environment has changed to:
Ubuntu 20.04 LTS
npm 6.14.5
node v12.16.3
still installed via the snap package.
I downgraded node due to another project I'm working on.

I also forgot to mention that my .npmrc contains the following due to another problem I've fought with before, though I doubt this changes anything relevant:

prefix=~/.local/share/npm-packages/

@kf6kjg
Copy link
Contributor Author

kf6kjg commented May 18, 2020

Sure enough the same Linux-only hack made it work.

Here's the hac.. ahem... patch that got things rolling again for me:

diff --git a/node_modules/patch-package/dist/makePatch.js b/node_modules/patch-package/dist/makePatch.js
index 84275f2..10e9996 100644
--- a/node_modules/patch-package/dist/makePatch.js
+++ b/node_modules/patch-package/dist/makePatch.js
@@ -85,7 +85,7 @@ function makePatch(_a) {
             try {
                 // try first without ignoring scripts in case they are required
                 // this works in 99.99% of cases
-                spawnSafe_1.spawnSafeSync("npm", ["i"], {
+                spawnSafe_1.spawnSafeSync("script", ["-q", "/dev/null", "-c", "npm i", "/dev/null"], {
                     cwd: tmpRepoNpmRoot,
                     logStdErrOnError: false,
                 });
@@ -93,7 +93,7 @@ function makePatch(_a) {
             catch (e) {
                 // try again while ignoring scripts in case the script depends on
                 // an implicit context which we havn't reproduced
-                spawnSafe_1.spawnSafeSync("npm", ["i", "--ignore-scripts"], {
+                spawnSafe_1.spawnSafeSync("script", ["-q", "/dev/null", "-c", "npm i --ignore-scripts", "/dev/null"], {
                     cwd: tmpRepoNpmRoot,
                 });
             }

@kf6kjg
Copy link
Contributor Author

kf6kjg commented May 18, 2020

After digging even deeper there's another way to "fix" this that's fully cross-platform:

diff --git a/node_modules/patch-package/dist/makePatch.js b/node_modules/patch-package/dist/makePatch.js
index 84275f2..a2547a6 100644
--- a/node_modules/patch-package/dist/makePatch.js
+++ b/node_modules/patch-package/dist/makePatch.js
@@ -88,6 +88,7 @@ function makePatch(_a) {
                 spawnSafe_1.spawnSafeSync("npm", ["i"], {
                     cwd: tmpRepoNpmRoot,
                     logStdErrOnError: false,
+                    stdio: 'ignore',
                 });
             }
             catch (e) {
@@ -95,6 +96,7 @@ function makePatch(_a) {
                 // an implicit context which we havn't reproduced
                 spawnSafe_1.spawnSafeSync("npm", ["i", "--ignore-scripts"], {
                     cwd: tmpRepoNpmRoot,
+                    stdio: 'ignore',
                 });
             }
         }

Since STDERR and STDOUT were not able to be read on my system anyway this works well.

Here's where I dug up that solution: nodejs/node-v0.x-archive#9057

@landsman
Copy link

I have same problem.

Environment:

  • Ubuntu 20.04 LTS
  • node version: v10.21.0
  • npm version: 6.14.6
$ snap list | grep node
node                    10.21.0                     2800  10/stable        nodesource*      classic

@kf6kjg
Copy link
Contributor Author

kf6kjg commented Jul 21, 2020

@landsman add the above patch to your patches folder with the file name patch-package+6.2.2.patch and you'll be able to create patches for other packages and patch-package will auto patch itself on every new install.

kf6kjg added a commit to kf6kjg/patch-package that referenced this issue Oct 8, 2020
Fixes ds300#229 using the working patch defined there.
@ds300 ds300 closed this as completed in #271 Mar 8, 2021
@jim-alexander
Copy link

The irony

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.

3 participants