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

codimd: build with nodejs-8_x #59118

Merged
merged 1 commit into from Apr 7, 2019
Merged

codimd: build with nodejs-8_x #59118

merged 1 commit into from Apr 7, 2019

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Apr 7, 2019

Motivation for this change

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

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

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
@vcunat
Copy link
Member

vcunat commented Apr 7, 2019

/cc maintainer @WilliButz.

Copy link
Member

@WilliButz WilliButz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vcunat Tested it successfully, everything seems to work fine. @Ma27 thank's for your work on this 👍

@lheckemann
Copy link
Member

@GrahamcOfBorg test codimd

@lheckemann lheckemann merged commit e0205c1 into NixOS:master Apr 7, 2019
@Ma27 Ma27 deleted the codimd-node-8 branch April 7, 2019 22:02
lheckemann pushed a commit that referenced this pull request Apr 7, 2019
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants