Convert "yarn" executable to a shell script that runs either "node" or "nodejs" #1180
Conversation
…r "nodejs" Also fixes Cygwin. Closes yarnpkg#1142 Closes yarnpkg#819
| @@ -1,2 +1,27 @@ | |||
| #!/usr/bin/env node | |||
| require('./yarn.js'); | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
This is mostly copied from what npm does, which uses @ForbesLindesay's cmd-shim: https://github.com/ForbesLindesay/cmd-shim/blob/master/index.js#L115
| esac | ||
|
|
||
| command_exists() { | ||
| command -v "$1" >/dev/null 2>&1; |
There was a problem hiding this comment.
command -v is a built-in POSIX command that returns the path to the specified command, or exits with an error code if the command doesn't exist.
daniel@debian:~/src/yarn$ command -v nodejs; echo $?
/usr/bin/nodejs
0
daniel@debian:~/src/yarn$ command -v does_not_exist; echo $?
1
It's built-in to the shell so it's very fast.
|
cc @cpojer @bestander - Would be good to get this into the next release, we just need to test it on Mac OS to ensure the script works there (I don't have a Mac to test it on) |
|
I have to question the sanity of this, by doing this you are basically saying that node can be called whatever and that all programs out there have to implement this logic, even in rarely-used scripts in some dusty corner of the code. Debian has an alternatives mechanism iirc, which lets you choose which program maps to a given name. Why not use that instead? And NixOS makes wrappers that set up the PATH so that I believe that by renaming |
|
I've found that people who end up in the situation of having Although we can make yarn resilient to this issue, it just pushes the problem further down the line. I think the right solution here is just to provide clear messaging on how to correctly alias nodejs to node. In the long run, this will save people a lot of pain. |
|
I didn't want to break the out-of-the-box experience for Ubuntu and Debian
That's not my choice though; I'm not the maintainer of Node.js. That's the Maybe I'll email the Node.js maintainers and see if they can use the Sent from my phone. On Oct 18, 2016 3:15 AM, "Forbes Lindesay" notifications@github.com wrote: I've found that people who end up in the situation of having node called Although we can make yarn resilient to this issue, it just pushes the — |
|
Let's merge this in as it solves the current problem. |
|
@Daniel15 this PR broke symlinking to the yarn executable, because you're not taking into account that |
|
Yeah I noticed that :(. This is how Node does it though - you'd see the We can use readlink to resolve the symlink, but I don't know if that's Sent from my phone. On Oct 20, 2016 8:54 AM, "Bouke van der Bijl" notifications@github.com
|
|
So now we are piling code on the hole made by supporting non-standard practices… this way madness lies. |
|
I'm going to email the maintainers of Node.js packaging and see how they feel about using the update-alternatives mechanism for symlinking
We need to support it as long as the Linux distribution with majority marketshare (Ubuntu) continues to do it this way. I'm reasonably certain that more people are using a Debian/Ubuntu Node.js package compared to something like |
|
Alternatively, we could symlink |
|
Please don't proliferate |
|
Well, you could test if |
@ljharb - The new version is strictly better than the old one - The Debian/Ubuntu package used to only look for The naming of the binary on Ubuntu/Debian is up to the Node.js maintainers, not me. We need to support whatever they do, particularly given Ubuntu's prevalence. |
sure, we can create a separate Github issue for following up on that :) |
|
@wmertens @ForbesLindesay et. al - I emailed the Debian JavaScript maintainers and cc'd the maintainer of Node.js:
Feel free to reply to the mailing list post if there's anything you'd like to add 😄 https://lists.alioth.debian.org/pipermail/pkg-javascript-devel/2016-October/014444.html |
|
Doesn't look like their view on this has changed much. Sadly this is a triumph of rules and technical correctness over pragmatism. We should push for:
even if that will take until 2019, at least that way this nonsense won't be permanent. I expect node (and I hope yarn) will still be in common usage in 2019 so this will still be a huge win. In the mean time, questions I would have are:
|
This is risky since the user might already have a |
Fixes a problem on commercialhaskell/lts-haskell#94 See yarnpkg/yarn#1180 for more context on the underlying problem
Fixes a problem on commercialhaskell/lts-haskell#94 See yarnpkg/yarn#1180 for more context on the underlying problem
Summary
Converted
yarnto be a shell script that checks for bothnodeornodejs. If neither is found, shows an error stating that Node.js needs to be installed.Test plan
Tested in the following environments:
Built Debian package with
./scripts/build-deb.shand tested it on Ubuntu.Background
Yarn always executes
node(due to the shebang of#!/usr/bin/env node), and always executesnodejson Debian/Ubuntu (wesedthe shebang when building the package: https://github.com/yarnpkg/yarn/blob/master/scripts/build-deb.sh#L74).The tricky/unfortunate thing with Node.js on Debian is that
/usr/bin/nodeconflicted with another package. See:https://lists.debian.org/debian-devel-announce/2012/07/msg00002.html
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=614907
In the end, both packages that used the
nodebinary name were renamed - Node.js becamenodejs, and the other Node (some ham radio app) becameax25-node. This means that when installing Node.js via Debian package, the binary is callednodejs. However, for other installation methods (such asnvm), the binary is callednode.Installing both Yarn and Node.js via Debian package is generally the preferred method, and works fine. However, this naming does have several implications when mixing and matching different installation styles:
npmwill fail with Node.js installed via Debian package (it'll try to runnodewhich doesn't exist)nodejswhich doesn't exist)Often people work around this by symlinking
/usr/bin/nodeto/usr/bin/nodejsor vice-versa, but that's hacky.Closes #1142
Closes #819