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

node 9.1.0 + patched npm (WIP / discuss) #20431

Closed
wants to merge 1 commit into from

Conversation

chrmoritz
Copy link
Contributor

@chrmoritz chrmoritz commented Nov 8, 2017

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

ToDo:


@ilovezfs This PR updates node to v9.1.0 and patches npm in a way, that the node 9 compatibility will survive self updates (by applying npm/npm@f24c1db and parts of npm/npm@4ca6958 (=isaacs/minizlib@a40d6ce)).

This work, because:

  1. there was only one node9 broken minizlib version (1.0.3) every bundled in npm since its introduction in npm@5.4.0 (with node-tar) and
  2. we are not applying the minizlib version bump from (1.0.3 to 1.0.4) but instead patching the minizlib@1.0.3 already bundled inside npm@5.5.1 to be node9 compatible (and keep it as v1.0.3).

You can test this by running multiple times npm i -g npm and npm i -g npm@5.4.2 after installing this formula version without breaking npm.

Note: While the minizlib patch (needed for node 9 compatibility) persists npm self upgrades, the also applied patch to suppress the incorrect incompatibility warnings does not persist self upgrades. Unfortunately you will notice a stupid runtime warning after the first self upgrade, telling you that your node 9 is to old for this npm version and you should install node 4+.


This happens when...

  • up/downgrading to a affected version of npm (5.4.0 to 5.5.1) (containing minizlib@1.0.3, npm i -g npm would update to 5.5.1 as of now): When downgrading to npm@5.4.0-5.5.1 npm only tries to update/replace the packages that have changed between both versions. Because we've labeled our minizlib version as the plain old v1.0.3 (while in reality it's a patched version identically to what would be in minizlib@1.0.4) npm doesn't touches minizlib@1.0.3 at all during upgrades (to these versions) and just keeps our patched node9 compatible version installed (because npm thinks our already installed version would be the same as the one requested by the update).
  • upgrading to a future version of npm (5.6+) (containing minizlib@1.0.4): In this case npm will replace our patched minizlib@1.0.3 with the real minizlib@1.0.4 (which is identical except metatdata) and npm will continue to work because we just installed a node 9 compatible npm version. At this point (when said version is npm@latest and not only npm@next) we should upgrade or formula to said npm version anyway and remove the workaround from this PR. Note: Downgrading at this future point in time to npm@5.4.0-5.5.1 will result in a broken npm, similar to how downgrading to early version of npm@2 or 1 have resulted in a broken npm since node 4+
  • downgrading to an earlier unaffected version of npm (<5.4.0): (pre node-tar, not using minizlib at all): In this case npm will remove our patched minizlib version, but because these version of npm are node 9 compatible, everything should work fine. Note: Upgrading from there again back to npm@5.4.0-5.5.1 will result in a broken npm, which is unfortunate but as said above we have to life forever with the fact, that installing npm@5.4.0-5.5.1 with node 9+ will break npm in the future similar to as installing a early version of npm@2 or 1 with node 4+ is breaking npm already as of now. Also downgrading npm to something pre npm@5.4.0 and the upgrading back again to npm@5.4.0-5.5.1 shouldn't be a common use case (but maybe we should warn about it in the caveats?).

@chrmoritz chrmoritz changed the title node 9.1.0 + patched npm node 9.1.0 + patched npm (WIP / discuss) Nov 8, 2017
@chrmoritz chrmoritz mentioned this pull request Nov 8, 2017
4 tasks
@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 9, 2017

Sorry I'd still like to wait for a compatible npm.

@ilovezfs ilovezfs closed this Nov 9, 2017
@Homebrew Homebrew locked and limited conversation to collaborators Nov 9, 2017
@Homebrew Homebrew unlocked this conversation Nov 28, 2017
@chrmoritz chrmoritz mentioned this pull request Nov 28, 2017
4 tasks
@ilovezfs
Copy link
Contributor

for anyone reading this,
brew install --devel node will now give you node 9.2.0 + npm 5.6.0

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
@chrmoritz chrmoritz deleted the node@9 branch February 16, 2019 00:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants