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

I can't create a patch #547

Open
Verzevula241 opened this issue Nov 25, 2024 · 7 comments
Open

I can't create a patch #547

Verzevula241 opened this issue Nov 25, 2024 · 7 comments

Comments

@Verzevula241
Copy link

I can't create a patch via npx patch-package, I see an error about the absence of a local file, even though it is in the packages folder

patch-package 8.0.0
• Creating temporary folder
• Installing @glideapps/[email protected] with yarn
npm WARN tarball tarball data for @glideapps/glide-data-grid@file:packages/glide/glideapps-glide-data-grid-5.2.7.tgz (null) seems to be corrupted. Trying again.
npm WARN tarball tarball data for @glideapps/glide-data-grid@file:packages/glide/glideapps-glide-data-grid-5.2.7.tgz (null) seems to be corrupted. Trying again.
npm ERR! code ENOENT
npm ERR! syscall open
npm ERR! path /tmp/tmp-19391nNb9KAE6LjxX/packages/glide/glideapps-glide-data-grid-5.2.7.tgz
npm ERR! errno -2
npm ERR! enoent ENOENT: no such file or directory, open '/tmp/tmp-19391nNb9KAE6LjxX/packages/glide/glideapps-glide-data-grid-5.2.7.tgz'
npm ERR! enoent This is related to npm not being able to find a file.
npm ERR! enoent 

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/sergey/.npm/_logs/2024-11-25T12_35_39_708Z-debug.log

{
  status: 254,
  signal: null,
  output: [
    null,
    <Buffer >,
    <Buffer 6e 70 6d 20 57 41 52 4e 20 74 61 72 62 61 6c 6c 20 74 61 72 62 61 6c 6c 20 64 61 74 61 20 66 6f 72 20 40 67 6c 69 64 65 61 70 70 73 2f 67 6c 69 64 65 ... 774 more bytes>
  ],
  pid: 19409,
  stdout: <Buffer >,
  stderr: <Buffer 6e 70 6d 20 57 41 52 4e 20 74 61 72 62 61 6c 6c 20 74 61 72 62 61 6c 6c 20 64 61 74 61 20 66 6f 72 20 40 67 6c 69 64 65 61 70 70 73 2f 67 6c 69 64 65 ... 774 more bytes>,
  error: null
}

/home/sergey/git/superset/superset-frontend/node_modules/patch-package/dist/makePatch.js:395
        throw e;
        ^
{
  status: 254,
  signal: null,
  output: [
    null,
    Buffer(0) [Uint8Array] [],
    Buffer(824) [Uint8Array] [
      110, 112, 109,  32,  87,  65,  82,  78,  32, 116,  97, 114,
       98,  97, 108, 108,  32, 116,  97, 114,  98,  97, 108, 108,
       32, 100,  97, 116,  97,  32, 102, 111, 114,  32,  64, 103,
      108, 105, 100, 101,  97, 112, 112, 115,  47, 103, 108, 105,
      100, 101,  45, 100,  97, 116,  97,  45, 103, 114, 105, 100,
       64, 102, 105, 108, 101,  58, 112,  97,  99, 107,  97, 103,
      101, 115,  47, 103, 108, 105, 100, 101,  47, 103, 108, 105,
      100, 101,  97, 112, 112, 115,  45, 103, 108, 105, 100, 101,
       45, 100,  97, 116,
      ... 724 more items
    ]
  ],
  pid: 19409,
  stdout: Buffer(0) [Uint8Array] [],
  stderr: Buffer(824) [Uint8Array] [
    110, 112, 109,  32,  87,  65,  82,  78,  32, 116,  97, 114,
     98,  97, 108, 108,  32, 116,  97, 114,  98,  97, 108, 108,
     32, 100,  97, 116,  97,  32, 102, 111, 114,  32,  64, 103,
    108, 105, 100, 101,  97, 112, 112, 115,  47, 103, 108, 105,
    100, 101,  45, 100,  97, 116,  97,  45, 103, 114, 105, 100,
     64, 102, 105, 108, 101,  58, 112,  97,  99, 107,  97, 103,
    101, 115,  47, 103, 108, 105, 100, 101,  47, 103, 108, 105,
    100, 101,  97, 112, 112, 115,  45, 103, 108, 105, 100, 101,
     45, 100,  97, 116,
    ... 724 more items
  ],
  error: null
}

Node.js v20.2.0
npm v9.6.6
@goququ
Copy link

goququ commented Nov 26, 2024

the same issue

@Grohden
Copy link

Grohden commented Nov 26, 2024

Tried to debug it, here's what I found

  • My global yarn is v1, my project is using v4/v3, patch package was using v1

    • I've tried to fix that by setting my global yarn correctly.. but from the logs bellow I don't think it worked
  • The original error seems to be coming from here

try {
// try first without ignoring scripts in case they are required
// this works in 99.99% of cases
spawnSafeSync(`yarn`, ["install", "--ignore-engines"], {
cwd: tmpRepoNpmRoot,
logStdErrOnError: false,
})
} catch (e) {
// try again while ignoring scripts in case the script depends on
// an implicit context which we haven't reproduced
spawnSafeSync(
`yarn`,
["install", "--ignore-engines", "--ignore-scripts"],
{
cwd: tmpRepoNpmRoot,
},
)
}

Both of them fail, here's the decoded STDs outputs

stdout:

new TextDecoder('utf-8').decode(e.stdout)
// ➤ YN0050: The --ignore-engines option is deprecated; engine checking isn't a core feature anymore

stderr:

new TextDecoder('utf-8').decode(e.stderr)
Debugger listening on ws://127.0.0.1:64859/8d05a664-dbfe-4c01-a6c5-0a64712a48f8
For help, see: https://nodejs.org/en/docs/inspector
Debugger attached.
! The local project doesn't define a 'packageManager' field. Corepack will now add one referencing [email protected]+sha1.ac34549e6aa8e7ead463a7407e1c7390f61a6610.
! For more details about this field, consult the documentation at https://nodejs.org/api/packages.html#packagemanager

Debugger listening on ws://127.0.0.1:64868/a24f621a-dd4d-45a4-a205-d34e53023156
For help, see: https://nodejs.org/en/docs/inspector
Debugger attached.
Waiting for the debugger to disconnect...
Waiting for the debugger to disconnect...

@Grohden
Copy link

Grohden commented Nov 26, 2024

I'll link this issue yarnpkg/berry#5802

But I think that ultimately, the issue is that patch package should copy the "packageManager" field to the temp package.json so that yarn (corepack?) doesn't add yarn v1 to it

@Grohden
Copy link

Grohden commented Nov 26, 2024

Here's a patch that made it work for me

Index: src/makePatch.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/makePatch.ts b/src/makePatch.ts
--- a/src/makePatch.ts	(revision c7c63bf80b3c6b8640b933e20229121b4edfc100)
+++ b/src/makePatch.ts	(date 1732647948973)
@@ -183,6 +183,7 @@
           appPath,
           appPackageJson.resolutions || {},
         ),
+        packageManager: appPackageJson.packageManager
       }),
     )
 
@@ -193,7 +194,7 @@
     // copy .npmrc/.yarnrc in case packages are hosted in private registry
     // copy .yarn directory as well to ensure installations work in yarn 2
     // tslint:disable-next-line:align
-    ;[".npmrc", ".yarnrc", ".yarn"].forEach((rcFile) => {
+    ;[".npmrc", ".yarnrc", ".yarnrc.yml", ".yarn"].forEach((rcFile) => {
       const rcPath = join(appPath, rcFile)
       if (existsSync(rcPath)) {
         copySync(rcPath, join(tmpRepo.name, rcFile), { dereference: true })
@@ -208,7 +209,12 @@
       try {
         // try first without ignoring scripts in case they are required
         // this works in 99.99% of cases
-        spawnSafeSync(`yarn`, ["install", "--ignore-engines"], {
+        // FIXME: yarn --ignore-engines causes yarn to warn on stderr
+        // spawnSafeSync(`yarn`, ["install", "--ignore-engines"], {
+        //   cwd: tmpRepoNpmRoot,
+        //   logStdErrOnError: false,
+        // })
+        spawnSafeSync(`yarn`, ["install"], {
           cwd: tmpRepoNpmRoot,
           logStdErrOnError: false,
         })

I'm not 100% sure bout the need for the packageManager field since I've fixed the yarnrc.yml part after having that fix in place.. but anyway, hope this helps :D

@Verzevula241
Copy link
Author

thanks friend, I'll try the solution and write about the result

@topi-identio
Copy link

topi-identio commented Dec 2, 2024

It feels illegal to use patch-package to patch patch-package but it does work, thanks. :D

I modified the patch a bit for 8.0.0:

diff --git a/node_modules/patch-package/dist/makePatch.js b/node_modules/patch-package/dist/makePatch.js
index d8d0925..9b25269 100644
--- a/node_modules/patch-package/dist/makePatch.js
+++ b/node_modules/patch-package/dist/makePatch.js
@@ -109,7 +109,7 @@ function makePatch({ packagePathSpecifier, appPath, packageManager, includePaths
             resolutions: resolveRelativeFileDependencies_1.resolveRelativeFileDependencies(appPath, appPackageJson.resolutions || {}),
         }));
         const packageVersion = getPackageVersion_1.getPackageVersion(path_1.join(path_1.resolve(packageDetails.path), "package.json"));
-        [".npmrc", ".yarnrc", ".yarn"].forEach((rcFile) => {
+        [".npmrc", ".yarnrc", ".yarnrc.yml", ".yarn"].forEach((rcFile) => {
             const rcPath = path_1.join(appPath, rcFile);
             if (fs_extra_1.existsSync(rcPath)) {
                 fs_extra_1.copySync(rcPath, path_1.join(tmpRepo.name, rcFile), { dereference: true });
@@ -120,7 +120,7 @@ function makePatch({ packagePathSpecifier, appPath, packageManager, includePaths
             try {
                 // try first without ignoring scripts in case they are required
                 // this works in 99.99% of cases
-                spawnSafe_1.spawnSafeSync(`yarn`, ["install", "--ignore-engines"], {
+                spawnSafe_1.spawnSafeSync(`yarn`, ["install"], {
                     cwd: tmpRepoNpmRoot,
                     logStdErrOnError: false,
                 });
@@ -128,7 +128,7 @@ function makePatch({ packagePathSpecifier, appPath, packageManager, includePaths
             catch (e) {
                 // try again while ignoring scripts in case the script depends on
                 // an implicit context which we haven't reproduced
-                spawnSafe_1.spawnSafeSync(`yarn`, ["install", "--ignore-engines", "--ignore-scripts"], {
+                spawnSafe_1.spawnSafeSync(`yarn`, ["install", "--mode=skip-build"], {
                     cwd: tmpRepoNpmRoot,
                 });
             }

@topi-identio
Copy link

topi-identio commented Dec 2, 2024

Seems like there's also a PR open that includes the changes to support Yarn v2+ properly #507

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

4 participants