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

Yarn doesn't create simlink to node binary #6583

Open
bdadam opened this issue Oct 25, 2018 · 12 comments
Open

Yarn doesn't create simlink to node binary #6583

bdadam opened this issue Oct 25, 2018 · 12 comments
Assignees
Labels

Comments

@bdadam
Copy link

bdadam commented Oct 25, 2018

What is the current behavior?
If the node package is installed as a dependency to the current package, Yarn does not link to the node binary in node_modules/.bin directory. But npm does.

Steps to reproduce:

  1. yarn init -y
  2. yarn add node@10.5.0 --exact
  3. Add "scripts": { "v": "node -v" }, to the package.json
  4. With running yarn v I expect it to report v10.5.0 (the one I set in step 2). But it reports the one I have installed on my machine.
  5. ls node_modules/.bin shows 0 files in the directory.
  6. rm -r node_modules
  7. npm i
  8. Running yarn v reports v10.5.0 as expected.
  9. ls node_modules/.bin shows the symlinked node binary in the directory.

What is the expected behavior?
I expect that yarn creates a symlink to the node binary in the node_modules/.bin directory for the node package as well. Please note that this works for other packages, e.g. webpack but not for node.

Here is an article (not written by me) about why I expect this behavior: https://nitayneeman.com/posts/standardizing-node.js-version-in-an-npm-package/

Please mention your node.js, yarn and operating system version.

node -v
v10.6.0

yarn -v
1.10.1
@ghost ghost assigned kaylie-alexa Oct 25, 2018
@ghost ghost added the triaged label Oct 25, 2018
@arcanis
Copy link
Member

arcanis commented Oct 26, 2018

This is caused by 6601f4b

cc @connectdotz, can you take a look please? It sounds like override is set to false, and since the symlink target doesn't exist until after the package gets installed (because it gets installed as part of a build script) it gets ignored.

@connectdotz
Copy link
Contributor

It sounds like override is set to false, and since the symlink target doesn't exist until after the package gets installed (because it gets installed as part of a build script) it gets ignored.

@arcanis how do you symlink a target if it doesnt exist? The subsequent fs.symlink will fail anyway if you don't skip... The problem is not whether we should skip the symlink here but why it is missing...

It looks like the preinstall of the node package got executed only at step 4 "Building fresh packages", which comes after step 3 "Linking dependencies", so it is not surprising that the symlink target is missing during linkSelfDependencies. One would think the preinstall should be executed before the actual linking step... no?

@arcanis
Copy link
Member

arcanis commented Oct 27, 2018

Symlinks can point to non-existing destinations, fs.symlink shouldn't throw (I seem to remember that fsExtra.ensureSymlink does, though - in which case we shouldn't use this function).

One would think the preinstall should be executed before the actual linking step... no?

No, packages need to be linked against their dependencies before being built (take the example of a package that uses babel to compile itself: it needs to be able to access its babel dependency, meaning that the link must happen before the build).

@connectdotz
Copy link
Contributor

connectdotz commented Oct 27, 2018

code-1

from the diff of 6601f4b you can see the original logic is always to skip symlink when target doesn't exist, defaulting override to false merely preserving that logic.

if I comment out the continue above, yarn failed at fs.chmod in linkBin:

error An unexpected error occurred: "ENOENT: no such file or directory, chmod '/tmp/x/node_modules/.bin/node'".

given fs.chmod (#439) and skipping non-existing symlink target (82b77a5) all have been there for over 2 years, I am puzzled why we are only seeing this issue now? What's so special about the node package...?

packages need to be linked against their dependencies before being built

but when should the dependencies kick off their builds? shouldn't they be built before the root package can link against them? In other words, I expect the following sequence:

fetch all packages => dependencies link/build => root package link => root package build

instead we have:

fetch all packages => root (all?) package link => all packages build

@arcanis
Copy link
Member

arcanis commented Oct 27, 2018

if I comment out the continue above, yarn failed at fs.chmod in linkBin

It seems like this chmod only purpose is to fix packages where the author mistakenly forgot to add the +x flag to one of their binary - we should just ignore it if the destination doesn't exist.

What's so special about the node package...?

Not sure, I don't see why it worked before 😐 Maybe the node package was differently architectured before (like, maybe the bin entry previously referenced an indirection script, whereas it now points directly to the node binary)?

shouldn't they be built before the root package can link against them?

No - linking (in Javascript, at least) is a step that doesn't require the packages to be built. It's not "linked" in the same sense as in C/C++, where all object files need to be built to then be merged into the final executable.

@connectdotz
Copy link
Contributor

when I said build I meant executing the preinstall script for example, this is what yarn do during the "build" step. The node package uses the preinstall to copy the binary into its own bin directory:

"scripts": {
    "preinstall": "node installArchSpecificPackage"
  },

Only after that the node/bin/node got created, which the root package was trying to symlink but failed:

### from the root package directory
$ ls -l node_modules/.bin
node -> ../node/bin/node

I am not sure what the right fix is here if we don't execute preinstall of node package first, prior to attempting to symlink to it... If you are implying that we should just comment out the fs.chmod() and create all symlinks regardless if the target exists, while it might fix this issue, it just didn't seem right and I am not sure it will not create other new problems...

@arcanis
Copy link
Member

arcanis commented Oct 27, 2018

while it might fix this issue, it just didn't seem right and I am not sure it will not create other new problems...

Why that? What issue do you think it might cause?

@arcanis
Copy link
Member

arcanis commented Oct 27, 2018

Note that I don't suggest commenting the fs.chmod (that would be a breaking change), but merely ignore any ENOENT error.

@connectdotz
Copy link
Contributor

Why that? What issue do you think it might cause?

exactly the problem 82b77a5 tried to fix, we don't want the node_module/.bin end up with bunch of "dead" symlinks that point to nowhere!

"linking to a non-existing target" violates the basic data/logic integrity. How do we know they will be created later? It seems at best a hack, and worse we didn't address the real issue here that might manifest to other problems:

from npm-scripts

preinstall: Run BEFORE the package is installed

yarn didn't honor it... when the root package tried to symlink to the node package, the node package's preinstall has NOT been executed yet...

@arcanis
Copy link
Member

arcanis commented Oct 27, 2018

exactly the problem 82b77a5 tried to fix, we don't want the node_module/.bin end up with bunch of "dead" symlinks that point to nowhere!

Sure we do. If the package tells us there's a bin entry, then we should assume it knows what it is doing, Even if the target doesn't exist it might still exist later (it might be created at runtime for all we know).

"linking to a non-existing target" violates the basic data/logic integrity

Because the package tells us so. Not creating the symlink actually breaks the contract we make with the package, since it explicitely tells us to create a link to a binary but we refuse to do so.

How do we know they will be created later

We don't, but that's the point: we shouldn't make any assumption in the first place.

preinstall: Run BEFORE the package is installed

That doesn't have any sense, like most of the npm documentation. What does it mean for a package to be "installed"? Are they all executed as very first step, before anything is copied from the cache? Before it is copied to the node_modules? Before it is built? Can a package preinstall script be run after the postinstall from another one? As always we're left to reverse engineer the thing ...

@connectdotz
Copy link
Contributor

Because the package tells us so. Not creating the symlink actually breaks the contract we make with the package since it explicitly tells us to create a link to a binary but we refuse to do so.

the node package also explicitly tell us to preinstall the node binary but we refuse to do! Just imagine if we have done that, the symlink will be correctly created and there would be no need to justify "why linking with a non-existing binary is actually a good thing..."

I searched around the issues and found that this is not a new issue, it has been mentioned quite a few times (see #3421 and related). While allowing linking to non-existing file could close this issue, so is running yarn --check-files or simply running yarn again (2nd run will find node binary and symlink correctly). They are all workarounds for a fundamental issue: lifecycle script execution order... @rally25rs made a clear diagnosis here.

Can a package preinstall script be run after the postinstall from another one

Based on @rally25rs diagnosis, it seems the lifecycle execution is "horizontal", i.e. all preinstall got executed first, then linking, then all postinstall.

I would love to see the lifecycle issue got resolved, if you guys are interested in addressing it and need some help, just let me know, I might be able to find some cycle.

@namannehra
Copy link

Has there been any update on this? Is there any life cycle script that we can use for our packages that is called before the symlink is created but after the dependencies are installed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants