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

Jump to bottom

Remove nodejs-6_x which is about to enter EOL #58976

Merged
merged 1 commit into from Apr 7, 2019
Merged

Remove nodejs-6_x which is about to enter EOL #58976

merged 1 commit into from Apr 7, 2019

Conversation

gilligan
Copy link
Contributor

@gilligan gilligan commented Apr 4, 2019

Motivation for this change

nodejs 6.x is entering EOL soon, so we should get rid of it - Fixes #58953

Things done
  • Remove nodejs-6_x
  • Set nodejs / nodejs-slim to nodejs-8_x / nodejs-slim-8_x
  • Re-generate node2nix generated files using nodejs-8_x instead

@gilligan
Copy link
Contributor Author

gilligan commented Apr 4, 2019

/cc @adisbladis

@adisbladis adisbladis self-assigned this Apr 4, 2019
@adisbladis adisbladis added the 9.needs: port to stable A PR needs a backport to the stable release. label Apr 4, 2019
@gilligan @adisbladis
Remove nodejs-6_x which is about to enter EOL
0e296d5 
- Remove nodejs-6_x
- Set nodejs / nodejs-slim to nodejs-8_x / nodejs-slim-8_x
- Re-generate node2nix generated files using nodejs-8_x instead
@adisbladis
Copy link
Member

adisbladis commented Apr 4, 2019 •
edited

I had to repair a few packages:

  • bitwarden-cli
  • airfield
  • base16-builder

Still broken:

@gilligan
Copy link
Contributor Author

gilligan commented Apr 4, 2019 

building '/nix/store/bgvg4dvnh5iqb4j8yxs1x8zvc0jcfq8r-node-babel-plugin-transform-runtime-6.23.0.drv'...
npm ERR! code ENOTFOUND
npm ERR! errno ENOTFOUND
npm ERR! network request to http://www.example.com/lodash failed, reason: getaddrinfo ENOTFOUND www.example.com www.example.com:80
npm ERR! network This is a problem related to network connectivity.
npm ERR! network In most cases you are behind a proxy or have bad network settings.
npm ERR! network
npm ERR! network If you are behind a proxy, please make sure that the
npm ERR! network 'proxy' config is set properly.  See: 'npm help config'

npm ERR! A complete log of this run can be found in:
npm ERR!     /build/.npm/_logs/2019-04-04T18_09_12_784Z-debug.log

Hm yeah so node2nix failed to resolve all dependencies which leads to npm trying to fetch/install whatever is missing ;/

Oh dear, looking at the default.nix of codimd there is a lot of patching/hacking going on around npm dependencies which i really do not want to fiddle with. I hope @WilliButz is around.

@WilliButz
Copy link
Member

WilliButz commented Apr 4, 2019 •
edited

@gilligan @adisbladis Quite some time ago (according to my stash it was in the end of october) I abandoned the node2nix approach and repackaged it successfully using yarn2nix (and nodejs 8) because I also think that right now it's way too hacky.
Unfortunately I currently have two impediments regarding the upstreaming of it, for one I depend on (the not yet merged) git support for yarn2nix introduced in (edit: nix-community/yarn2nix#85 nix-community/yarn2nix#92) and I am also a little short of time for at least the next two weeks.

@Ma27
Copy link
Member

Ma27 commented Apr 4, 2019

As I'm fairly interested in keeping codimd in nixpkgs, here are some of my thoughts:

  • I'm currently not sure if it's such a good idea to use yarn2nix in nixpkgs as it's been removed recently (Remove yarn2nix, see #20637 #58424). But if we pull the needed yarn2nix sources to build codimd, I guess it should be fine for this use case to simply use a git rev which includes git support.
  • We could simply try to regenerate the node2nix package set and modify the expression accordingly. We currently use Codimd 1.2.0 which should work with NodeJS 8 as well (https://github.com/codimd/server/blob/1.2.0/package.json#L136-L138), the latest version (1.3.x supports NodeJS 10 as well).

Unless anybody else wants to take over, I can attempt to do an update of codimd tomorrow to ensure that it's working until we have a simpler solution based on yarn2nix.

@Ma27
Copy link
Member

Ma27 commented Apr 5, 2019 •
edited

Unfortunately it seems as it's not possible to regenerate the codimd package set in its current state as node2nix fails to resolve some dependencies. I hope that I can fix that this weekend, however codimd is certainly not a blocker for 19.03, so I'd suggest that we mark codimd as broken here and backport the fix for codimd as soon as it's ready :)

EDIT: it seems as there's a problem with the js-sequence-diagram (see https://www.npmjs.com/package/js-sequence-diagrams, bramp/js-sequence-diagrams#212). I'll check if there's an upstream fix tomorrow and/or package this manually (as done with CodeMirror) if needed.

Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Apr 7, 2019
@Ma27
codimd: build with nodejs-8_x
5feec42 
This diff regenerates the package sets for `codimd` and `codemirror`
using NodeJS 8 to get rid of the deprecated[1] `nodejs-6_x`.

Additionally the following issues had to be fixed during the update:

* The package `js-sequence-diagram` has been removed from the NPM
  registry and was replaced by a security holding package[2]. The
  package was published by a third-party (upstream only supports bower
  builds), so it's unclear whether the package will re-appear[3].

  As the tarballs still exist (and the hash didn't change), the package
  will be loaded manually into the build env.

* For the babel-related packages, `dontNpmInstall` will be set for
  `node2nix` installs as some of those packages bundle a
  `package-lock.json` that triggers `ENOTCACHED` errors for optional
  dependencies[4].

For now it should be sufficient to use NodeJS 8 (`codimd` v1.2.x doesn't
support NodeJS 10), in the long term we probably want to use `yarn2nix`
here with NodeJS 10. This is much rather a fix to get rid of another
NodeJS 6 dependency.

[1] `nodejs-6_x` is about to be deprecated, see NixOS#58976
[2] https://www.npmjs.com/package/js-sequence-diagrams,
    https://github.com/npm/security-holder
[3] bramp/js-sequence-diagrams#212
[4] svanderburg/node2nix#134
@Ma27 Ma27 mentioned this pull request Apr 7, 2019
Merged
10 tasks
@Ma27
Copy link
Member

Ma27 commented Apr 7, 2019

#59118 for codimd.

@samueldr
Copy link
Member

samueldr commented Apr 7, 2019

Just checking, @gilligan / @adisbladis that with #59118 now open, everything is fine?

This PR can be merged? The other one looks ready to me.

lheckemann pushed a commit that referenced this pull request Apr 7, 2019
@Ma27 @lheckemann
codimd: build with nodejs-8_x
6380740 
This diff regenerates the package sets for `codimd` and `codemirror`
using NodeJS 8 to get rid of the deprecated[1] `nodejs-6_x`.

Additionally the following issues had to be fixed during the update:

* The package `js-sequence-diagram` has been removed from the NPM
  registry and was replaced by a security holding package[2]. The
  package was published by a third-party (upstream only supports bower
  builds), so it's unclear whether the package will re-appear[3].

  As the tarballs still exist (and the hash didn't change), the package
  will be loaded manually into the build env.

* For the babel-related packages, `dontNpmInstall` will be set for
  `node2nix` installs as some of those packages bundle a
  `package-lock.json` that triggers `ENOTCACHED` errors for optional
  dependencies[4].

For now it should be sufficient to use NodeJS 8 (`codimd` v1.2.x doesn't
support NodeJS 10), in the long term we probably want to use `yarn2nix`
here with NodeJS 10. This is much rather a fix to get rid of another
NodeJS 6 dependency.

[1] `nodejs-6_x` is about to be deprecated, see #58976
[2] https://www.npmjs.com/package/js-sequence-diagrams,
    https://github.com/npm/security-holder
[3] bramp/js-sequence-diagrams#212
[4] svanderburg/node2nix#134

(cherry picked from commit 5feec42,
PR #59118)
@samueldr
Copy link
Member

samueldr commented Apr 7, 2019 •
edited

Shouldn't we remove remarkjs, marked broken since 2016-06-11? I think we should.

I guess this can be done as a follow-up PR anyway.

@samueldr samueldr merged commit 40d59c6 into NixOS:master Apr 7, 2019
@samueldr
Copy link
Member

samueldr commented Apr 7, 2019

Backported
[release-19.03 763e65f] Remove nodejs-6_x which is about to enter EOL
Date: Thu Apr 4 19:10:06 2019 +0200
23 files changed, 3520 insertions(+), 720 deletions(-)

@samueldr samueldr removed the 9.needs: port to stable A PR needs a backport to the stable release. label Apr 8, 2019
@Ma27 Ma27 mentioned this pull request Apr 12, 2019
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ma27 Ma27 Ma27 approved these changes

Assignees

@adisbladis adisbladis

Labels
6.topic: nixos 8.has: changelog 8.has: clean-up 8.has: documentation 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@gilligan @adisbladis @WilliButz @Ma27 @samueldr @GrahamcOfBorg