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 link-cli doesn't work with old Node.js versions #661

Closed
michaelfig opened this issue Mar 6, 2020 · 6 comments · Fixed by #663
Closed

yarn link-cli doesn't work with old Node.js versions #661

michaelfig opened this issue Mar 6, 2020 · 6 comments · Fixed by #663
Assignees
Labels
agoric-cli package: agoric-cli

Comments

@michaelfig
Copy link
Member

Reported by @dtribble:

$ sudo yarn link-cli /usr/local/bin/agoric
yarn run v1.22.0
$ node ./scripts/link-cli.js /usr/local/bin/agoric
creating /usr/local/bin/agoric
TypeError: Cannot read property 'writeFile' of undefined
    at <redacted>/agoric-sdk/scripts/link-cli.js:13:12
    at Object.<anonymous> (<redacted>/agoric-sdk/scripts/link-cli.js:15:3)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Function.Module.runMain (module.js:693:10)
    at startup (bootstrap_node.js:188:16)
    at bootstrap_node.js:609:3
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

This is caused by root's Node.js version not implementing the fs.promises API. It should be straightforward to rewrite scripts/link-cli.js to be more portable.

@michaelfig michaelfig self-assigned this Mar 6, 2020
@michaelfig michaelfig added the agoric-cli package: agoric-cli label Mar 6, 2020
michaelfig added a commit that referenced this issue Mar 6, 2020
Fixes #661

We avoid Promises to allow this script to run under older
system versions of node.
@katelynsills
Copy link
Contributor

Are we supporting old node versions?

@erights
Copy link
Member

erights commented Mar 6, 2020

Same question. What versions are we talking about? We don't yet have post-alpha customers. Why compromise the quality of our code to run on anything but the latest?

@michaelfig
Copy link
Member Author

Are we supporting old node versions?

Just for the specific link-cli script. The reason is that many people have old (i.e. pre-Node.js 10) versions lying around in system directories, so when they do a sudo yarn link-cli, it fails. fs.promises is a relatively new invention, and I don't see any real reason to use it when the compatible fs API works just as well.

This code will never be in XS, either.

@michaelfig
Copy link
Member Author

michaelfig commented Mar 6, 2020

This falls into the same category that certain tooling (such as eslint, prettier) needs to use CommonJS, and it will take a long time for them to support native ESM. Let's not force the world to migrate entirely to ESM and Node 13+ when just a few grungy tooling scripts (like link-cli) can live in the old world until the new world is fully here.

If you disagree, I would be happy to write link-cli as a portable Bourne shell script and dodge that bullet in a different way. 😉

@warner
Copy link
Member

warner commented Mar 6, 2020

I actually like the non-promise script better, it reads more easily. I've always found async filesystem operations to be a bit silly (it's not like it's spawning a thread or using aio under the hood).

@michaelfig
Copy link
Member Author

I actually like the non-promise script better, it reads more easily. I've always found async filesystem operations to be a bit silly (it's not like it's spawning a thread or using aio under the hood).

Actually, that's exactly what Node.js does

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

Successfully merging a pull request may close this issue.

4 participants