Skip to content

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

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

Closed
3 tasks done
code-asher opened this issue Mar 31, 2022 · 4 comments · Fixed by #5071
Closed
3 tasks done

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

code-asher opened this issue Mar 31, 2022 · 4 comments · Fixed by #5071
Labels
bug Something isn't working triage This issue needs to be triaged by a maintainer

Comments

@code-asher
Copy link
Member

code-asher commented Mar 31, 2022

Is there an existing issue for this?

  • 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?

  • I cannot reproduce this in VS Code.

Are you accessing code-server over HTTPS?

  • 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 code-asher added bug Something isn't working triage This issue needs to be triaged by a maintainer labels Mar 31, 2022
@code-asher code-asher added this to the April 2022 milestone Mar 31, 2022
@code-asher
Copy link
Member Author

code-asher commented Mar 31, 2022

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.

@edvincent
Copy link
Contributor

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.

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 6, 2022

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!

@edvincent
Copy link
Contributor

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.)

@jsjoeio jsjoeio modified the milestones: April 2022, May 2022 Apr 26, 2022
@jsjoeio jsjoeio modified the milestones: June 2022, July 2022 Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage This issue needs to be triaged by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants