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

[Bug]: Yarn refuses to build package that is inside of node_modules, saying it needs a yarn.lock file #5804

Closed
1 task
trusktr opened this issue Oct 18, 2023 · 9 comments
Labels
bug Something isn't working stale Issues that didn't get attention waiting for feedback Will autoclose in a while unless more data are provided

Comments

@trusktr
Copy link

trusktr commented Oct 18, 2023

Self-service

  • I'd be willing to implement a fix

Describe the bug

The lume project I've been trying to migrate to Yarn for workspace management depends on deasync. When I yarn install, I get this output:

➤ YN0009: │ deasync@npm:0.1.28 couldn't be built successfully (exit code 1, logs can be found here: C:\Users\trusktr\AppData\Local\Temp\xfs-8901406d\build.log)

and I see this in build.log:

# This file contains the result of Yarn building a package (deasync@npm:0.1.28)
# Script name: install

�[31m�[1mUsage Error�[22m�[39m: The nearest package directory (D:\src\lume+lume\packages\docsifyjs+docsify\node_modules\deasync) doesn't seem to be part of the project declared in D:\src\lume+lume.

- If D:\src\lume+lume isn't intended to be a project, remove any yarn.lock and/or package.json file there.
- If D:\src\lume+lume is intended to be a project, it might be that you forgot to list packages/docsifyjs+docsify/node_modules/deasync in its workspace configuration.
- Finally, if D:\src\lume+lume is fine and you intend packages/docsifyjs+docsify/node_modules/deasync to be treated as a completely separate project (not even a workspace), create an empty yarn.lock file in it.

�[1m$ �[22myarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] <scriptName> ...
Build failed

None of those three options seem to be applicable:

  • lume is intended to be a project, so leaving yarn.lock and package.json
  • lume is a project, but listing a dependency in node_modules that doesn't exist before I run yarn install as project doesn't seem correct
  • lume is fine, but modifying dependencies inside of node_modules doesn't seem correct either, especially because it would require a post-install patch or to commit node_modules into the repo

To reproduce

The lume repo at this commit should show the error, just yarn install:

https://github.com/lume/lume/tree/5bd0cac03077ffa98ee90d72de2ad7b902907db6

Environment

System:
    OS: Windows 10 10.0.19045
    CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
  Binaries:
    Node: 20.5.0 - ~\AppData\Local\Temp\xfs-c4ef94ef\node.CMD
    Yarn: 3.6.4 - ~\AppData\Local\Temp\xfs-c4ef94ef\yarn.CMD
    npm: 9.8.0 - D:\Program Files (x86)\nodejs\npm.CMD

Additional context

No response

@trusktr trusktr added the bug Something isn't working label Oct 18, 2023
@trusktr trusktr changed the title [Bug?]: Yarn refuses to build package that is inside of node_modules, saying it needs a yarn.lock file [Bug]: Yarn refuses to build package that is inside of node_modules, saying it needs a yarn.lock file Oct 18, 2023
@trusktr
Copy link
Author

trusktr commented Oct 18, 2023

Should I be trying Yarn 4 rc? I just learned that v4 exists. Maybe I'm wasting my time if issues I'm having are already solved.

@clemyan
Copy link
Member

clemyan commented Oct 18, 2023

It is a rare case where yarn is unable to detect deasync's implicit dependency on node-gyp. This has somehow caused yarn to treat deasync as a workspace when invoking node-gyp. When I use packageExtensions to add the node-gyp dependency to deasync it is invoked successfully.

@clemyan
Copy link
Member

clemyan commented Oct 20, 2023

Ok I dug a little deeper. What is happening here is, due to nmHoistingLimits: workspaces, the same locator deasync@npm:0.1.28 is installed into two locations: packages\docsifyjs+docsify\node_modules\deasync and packages\glas\node_modules\deasync.

Then, when running node-gyp on those, yarn tries to determine if the package at cwd is part of the current project via NodeModulesLinker#findPackageLocation

const location = await linker.findPackageLocation(locator, linkerOptions);
if (location.replace(TRAILING_SLASH_REGEXP, ``) !== cwd.replace(TRAILING_SLASH_REGEXP, ``)) {
continue;
}

But NodeModulesLinker can only return one location for the same locator, so yarn thinks packages\glas\node_modules\deasync is part of the project while packages\docsifyjs+docsify\node_modules\deasync is not, leading to two different error messages:

Usage Error: The nearest package directory (\lume\packages\docsifyjs+docsify\node_modules\deasync) doesn't seem to be part of the project declared in \lume.

- If \lume isn't intended to be a project, remove any yarn.lock and/or package.json file there.
- If \lume is intended to be a project, it might be that you forgot to list packages/docsifyjs+docsify/node_modules/deasync in its workspace configuration.
- Finally, if \lume is fine and you intend packages/docsifyjs+docsify/node_modules/deasync to be treated as a completely separate project (not even a workspace), create an empty yarn.lock file in it.

$ yarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] [--require #0] <scriptName> ...
Usage Error: Couldn't find a script name "node-gyp" in the top-level (used by deasync@npm:0.1.28). This typically happens because some package depends on "node-gyp" to build itself, but didn't list it in their dependencies. To fix that, please run "yarn add node-gyp" into your top-level workspace. You also can open an issue on the repository of the specified package to suggest them to use an optional peer dependency.

yarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] [--require #0] <scriptName> ...

That said, if the dependency on node-gyp is properly declared (e.g. via packageExtensions) none of the above happens.

@trusktr
Copy link
Author

trusktr commented Oct 21, 2023

Inferring from that, does deasync need a PR to get node-gyp as direct dependency and would that also avoid the problem? If so, I'll open a PR there. (EDIT: PR opened there.)

Why do packages sometimes not include node-gyp as a dependency? Is it because it is always in scope when using npm?

Is node-gyp not a special case that Yarn should make work regardless?

To fix that, please run "yarn add node-gyp" into your top-level workspace

Is that correct in nmHoistingLimits: workspaces mode, if Yarn limits what packages see?

Should Yarn's error message be improved to instead recommend fixing with packageExtensions, if it won't support node-gyp as a special case?

trusktr added a commit to lume/lume that referenced this issue Oct 21, 2023
…its direct dependency, to prevent the build error it has due to otherwise not depending directly on node-gyp directly (yarnpkg/berry#5804)
@trusktr
Copy link
Author

trusktr commented Oct 21, 2023

I tried adding node-gyp as a dependency for deasync in packageExtensions, and that got GitHub Actions passing, but on my local Windows machine Yarn install fails to build deasync with this output:

D:\src\lume+lume\packages\glas\node_modules\deasync\src\deasync.cc(3,10): fatal  error C1083: Cannot open include file: 'napi.h': No such file or directory [D:\src\lume+lume\packages\glas\node_modules\deasync\build\deasync.vcxproj]
gyp ERR! build error 
gyp ERR! stack Error: `C:\Program Files\Microsoft Visual Studio\2022\Community\MSBuild\Current\Bin\MSBuild.exe` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (D:\src\lume+lume\node_modules\node-gyp\lib\build.js:203:23)
gyp ERR! stack     at ChildProcess.emit (node:events:514:28)
gyp ERR! stack     at ChildProcess._handle.onexit (node:internal/child_process:294:12)
gyp ERR! System Windows_NT 10.0.19045
gyp ERR! command "D:\\Program Files (x86)\\nodejs\\node.exe" "D:\\src\\lume+lume\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild"
gyp ERR! cwd D:\src\lume+lume\packages\glas\node_modules\deasync
gyp ERR! node -v v20.5.0
gyp ERR! node-gyp -v v9.4.0
gyp ERR! not ok 
Build failed

I used latest as the version for node-gyp. That wouldn't have to do with it would it?

@clemyan
Copy link
Member

clemyan commented Oct 23, 2023

Why do packages sometimes not include node-gyp as a dependency? Is it because it is always in scope when using npm?

npm does include a node-gyp binary when running scripts, but why don't packages declare a dependency beats me

See also https://yarnpkg.com/advanced/error-codes#yn0032---node_gyp_injected

Is node-gyp not a special case that Yarn should make work regardless?

Yarn does special case node-gyp, but in a different way from npm. When running scripts from a package that does not declare a dependency on node-gyp it falls back to the node-gyp that is the dependency of the top-level project.

Is that correct in nmHoistingLimits: workspaces mode, if Yarn limits what packages see?

Should Yarn's error message be improved to instead recommend fixing with packageExtensions, if it won't support node-gyp as a special case?

No. If not for the issue I metioned above (packages\docsifyjs+docsify\node_modules\deasync not being detected as part of the project), installing node-gyp at the top level would have solved the issue.

I tried adding node-gyp as a dependency for deasync in packageExtensions, and that got GitHub Actions passing, but on my local Windows machine Yarn install fails to build deasync with this output:

In your GH pipeline deasync is using a prebuilt binary but on your local machine it tries to compile a fresh build, which then depends whether on your local environment support such compilation

@arcanis arcanis added the waiting for feedback Will autoclose in a while unless more data are provided label Oct 27, 2023
@yarnbot
Copy link
Collaborator

yarnbot commented Nov 26, 2023

Hi! 👋

It seems like this issue as been marked as probably resolved, or missing important information blocking its progression. As a result, it'll be closed in a few days unless a maintainer explicitly vouches for it.

@yarnbot yarnbot added the stale Issues that didn't get attention label Nov 26, 2023
@trusktr
Copy link
Author

trusktr commented Nov 29, 2023

I personally moved on from this by updating projects to not depend on deasync. Not sure if you all want to do something with the reproduction and the error handling in case of nmHoistingLimits: workspaces. It seems like the generated arror wasn't helpful, kinda a side effect, but now that you know what's happening, maybe a better error can be made (f.e. "install node-gyp at the top workspace to make it available to any workspaces or packages that need it")

@yarnbot yarnbot removed the stale Issues that didn't get attention label Nov 29, 2023
@yarnbot
Copy link
Collaborator

yarnbot commented Dec 29, 2023

Hi! 👋

It seems like this issue as been marked as probably resolved, or missing important information blocking its progression. As a result, it'll be closed in a few days unless a maintainer explicitly vouches for it.

@yarnbot yarnbot added the stale Issues that didn't get attention label Dec 29, 2023
@yarnbot yarnbot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale Issues that didn't get attention waiting for feedback Will autoclose in a while unless more data are provided
Projects
None yet
Development

No branches or pull requests

4 participants