code-server icon indicating copy to clipboard operation
code-server copied to clipboard

[Bug]: Use npm instead of yarn in postinstall script

Open code-asher opened this issue 2 years ago • 4 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

OS/Web Information

  • Web Browser: n/a
  • Local OS: n/a
  • Remote OS: n/a
  • Remote Architecture: n/a
  • code-server --version: development

Steps to Reproduce

  1. yarn build release
  2. Uninstall yarn
  3. cd release && npm install

Expected

Install completes successfully.

Actual

Although I have not actually done this (uninstalling yarn is a pain), the postinstall script uses yarn so it will probably error that yarn cannot be found.

Logs

No response

Screenshot/Video

No response

Does this issue happen in VS Code?

  • [X] I cannot reproduce this in VS Code.

Are you accessing code-server over HTTPS?

  • [X] I am using HTTPS.

Notes

To fix:

  1. Replace yarn with npm in npm-postinstall.sh
  2. Generate shrinkwrap files for:
    1. lib/vscode/package.json
    2. lib/vscode/extensions/package.json

code-asher avatar Mar 31 '22 16:03 code-asher

Another note: maybe we should use yarn in the postinstall script if installing with yarn otherwise npm? Just in case someone does install with yarn.

code-asher avatar Mar 31 '22 16:03 code-asher

Damn completely missed this.. I think I had something weird happening in my local dev box, and I was under the impression the upper-most one would take care of the dependencies. Let me try and take a stab at this, and also get something better on the way we generate that shrinkwrap file I might have discovered looking into this.

edvincent avatar Apr 06 '22 18:04 edvincent

Don't worry at all - sorry we missed this in the PR review.

Let me try and take a stab at this

Sounds good! Thank you for offering to chip in again. We appreciate it!

jsjoeio avatar Apr 06 '22 22:04 jsjoeio

Ha no, the root-cause for missing this was that I validated the other change by using the package.tar.gz generated from the CI/the yarn command... which actually doesn't strip the yarn.lock files, so the versions were still deterministic.

The PR that generates the extra shrinkwrap files also adds an actual NPM release artifact - let me know if you'd rather have that done in a separate PR.

(And sorry for the long PR explanation, but hopefully it helps with the why's / sharing the context & knowledge.)

edvincent avatar Apr 07 '22 02:04 edvincent