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.

Sign up for GitHub

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

Jump to bottom

deno install doesn't properly setup modules when used in moment #25862

Closed
satyarohith opened this issue Sep 25, 2024 · 0 comments · Fixed by #25873
Closed

deno install doesn't properly setup modules when used in moment #25862

satyarohith opened this issue Sep 25, 2024 · 0 comments · Fixed by #25873
Labels
bug Something isn't working correctly install

Comments

@satyarohith
Copy link
Member

When we clone https://github.com/moment/moment and run deno install && deno task test, it fails with the following error. We think it's something to do with how node modules are structured by deno install.

The same works if we install node modules using npm i and run test using deno task test.

Running "exec:typescript-test" (exec) task

> [email protected] typescript-test
> cross-env node_modules/typescript/bin/tsc --project typing-tests

>> node:events:498
>>       throw er; // Unhandled 'error' event
>>       ^
>> 
>> Error: spawn node_modules/typescript/bin/tsc EACCES
>>     at ChildProcess._handle.onexit (node:internal/child_process:286:19)
>>     at onErrorNT (node:internal/child_process:484:16)
>>     at process.processTicksAndRejections (node:internal/process/task_queues:90:21)
>> Emitted 'error' event on ChildProcess instance at:
>>     at ChildProcess._handle.onexit (node:internal/child_process:292:12)
>>     at onErrorNT (node:internal/child_process:484:16)
>>     at process.processTicksAndRejections (node:internal/process/task_queues:90:21) {
>>   errno: -13,
>>   code: 'EACCES',
>>   syscall: 'spawn node_modules/typescript/bin/tsc',
>>   path: 'node_modules/typescript/bin/tsc',
>>   spawnargs: [ '--project', 'typing-tests' ]
>> }
>> 
>> Node.js v22.8.0
>> Exited with code: 1.
>> Error executing child process: Error: Process exited with code 1.

Co-authored-by: littledivy
@satyarohith satyarohith added bug Something isn't working correctly install labels Sep 25, 2024
@nathanwhit nathanwhit mentioned this issue Sep 25, 2024
Merged
nathanwhit added a commit that referenced this issue Sep 26, 2024
@nathanwhit
fix(installl): make bin entries executable even if not put in `node_m…
13c53d9 
…odules/.bin` (#25873)

Fixes #25862.

npm only makes bin entries executable if they get linked into `.bin`, as
we did before this PR. So this PR actually deviates from npm, because
it's the only reasonable way to fix this that I can think of.

---

The reason this was broken in moment is the following:

Moment has dependencies on two typescript versions: 1.8 and 3.1

If you have two packages with conflicting bin entries (i.e. two
typescript versions which both have a bin entry `tsc`), in npm it is
non-deterministic and undefined which one will end up in `.bin`.

npm, due to implementation differences, chooses to put typescript 1.8
into the `.bin` directory, and so `node_modules/typescript/bin/tsc` ends
up getting marked executable. We, however, choose typescript 3.2, and so
we end up making `node_modules/typescript3/bin/tsc` executable.

As part of its tests, moment executes `node_modules/typescript/bin/tsc`.
Because we didn't make it executable, this fails.

Since the conflict resolution is undefined in npm, instead of trying to
match it, I think it makes more sense to just make bin entries
executable even if they aren't chosen in the case of a conflict.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working correctly install
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(installl): make bin entries executable even if not put in node_modules/.bin nathanwhit/deno
1 participant
@satyarohith