add nodePackages vega-cli and vega-lite #97908

MMesch · 2020-09-13T13:35:41Z

Motivation for this change

Adds the vega and the vega-lite command line tools for plotting. This means the binaries:

vg2svg
vg2pdf
vg2png
vl2vg
vl2pdf
vl2svg
vl2png

Vegalite required substituting some lines that call npx to install vega on the fly. Maybe there is a better solution. I tested these tools with:

nix-shell --pure -I nixpkgs=./nixpkgs -p busybox -p nodePackages.vega-cli --command "wget -O - https://vega.github.io/vega/examples/bar-chart.vg.json | vg2png > vega-cli-test.png"

nix-shell --pure -I nixpkgs=./nixpkgs -p busybox -p nodePackages.vega-lite --command "wget -O - https://raw.githubusercontent.com/vega/vega-lite/master/examples/specs/bar.vl.json | vl2png > vega-lite-test.png"

Things done

MMesch · 2020-09-14T10:47:34Z

I tried to run nixpkgs-review pr --post-result 97908 and stopped it after 2h because I needed the CPU. It somehow seems to hang on twemoji, mojione and sagedoc. inkscape also somehow appears there. The output that I got until then is:

$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0 pull/97908/head:refs/nixpkgs-review/1
$ git worktree add /home/matto/.cache/nixpkgs-review/pr-97908-1/nixpkgs 24fa210e772351f118e5f56e5acbe5eaea38a95f
Preparing worktree (detached HEAD 24fa210e772)
Updating files: 100% (22686/22686), done.
HEAD is now at 24fa210e772 rclone: 1.53.0 -> 1.53.1 (#97953)
$ nix-env -f /home/matto/.cache/nixpkgs-review/pr-97908-1/nixpkgs -qaP --xml --out-path --show-trace
$ git merge --no-commit 690fffccf47b4561013912dd37d5bda8ea8484b7
Automatic merge went well; stopped before committing as requested
$ nix-env -f /home/matto/.cache/nixpkgs-review/pr-97908-1/nixpkgs -qaP --xml --out-path --show-trace --meta
39 packages updated:
bitwarden-cli emojione etcher image_optim lumo node_castnow node_create-cycle-app node_imapnotify joplin (1.0.166 → 1.0.167) node_mirakurun netlify-cli (2.61.0 → 2.62.0) node_redoc-cli node_thelounge node_triton node__at_antora_slash_cli node__at_webassemblyjs_slash_wasm-text-gen parity-ui ripcord sage sage sage slack slack twemoji-color-font vimPlugins.coc-css (1.2.4 → 1.2.5) vimPlugins.coc-eslint (1.2.7 → 1.3.0) vimPlugins.coc-git (1.8.3 → 2.0.0) vimPlugins.coc-go (0.10.0 → 0.11.0) vimPlugins.coc-highlight (1.2.5 → 1.2.6) vimPlugins.coc-jest (1.0.3 → 1.0.4) vimPlugins.coc-json (1.2.6 → 1.3.0) vimPlugins.coc-metals (0.9.2 → 0.9.3) vimplugin-coc-prettier vimPlugins.coc-r-lsp (1.1.1 → 1.2.0) vimPlugins.coc-snippets (2.1.28 → 2.2.0) vimPlugins.coc-solargraph (1.1.6 → 1.1.7) vimplugin-coc-stylelint vimPlugins.coc-tsserver (1.5.5 → 1.5.6) vimPlugins.coc-vetur (1.1.12 → 1.1.13)

$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /home/matto/.cache/nixpkgs-review/pr-97908-1/build.nix
builder for '/nix/store/b42m79dxaz44gjb4gpg3mmlwdr4zxy4h-node_joplin-1.0.167.drv' failed with exit code 1; last 10 log lines:
  npm ERR! errno 1
  npm ERR! keytar@5.6.0 install: `prebuild-install || node-gyp rebuild`
  npm ERR! Exit status 1
  npm ERR! 
  npm ERR! Failed at the keytar@5.6.0 install script.
  npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
  
  npm ERR! A complete log of this run can be found in:
  npm ERR!     /build/.npm/_logs/2020-09-14T08_13_48_980Z-debug.log
 [...]

nixos-discourse · 2020-09-14T11:20:16Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/299

doronbehar · 2020-09-16T09:44:22Z

@MMesch It's almost impossible to merge changes to node packages, unfortunately. Could you please fix them and ping me when you are ready? Preferably, git add all the files (don't use --patch to select only the changes relevant to you). I promise I'll merge it before there are merge conflicts.

Once that'll be done, I'd be happy to get your help on #96961 .

MMesch · 2020-09-16T21:27:48Z

Hi @doronbehar

It's almost impossible to merge changes to node packages, unfortunately

What do you mean with this? Is it possible or not? I looked on your PR and saw that you suggested generating nix expressions for the packages you want to add independently. I have done that equally for vega-cli and vega-lite on my machine. Do you think it would be better to move forward like this?

Also, I already added git add everything! Are there files that you are missing?

doronbehar · 2020-09-17T06:21:07Z

What do you mean with this?

I mean that as a contributor, with no merge permissions, it's impossible to put out a PR, and when the time comes for a committer to merge it, the PR has no merge conflicts.

I have done that equally for vega-cli and vega-lite on my machine. Do you think it would be better to move forward like this?

It depends on you. For balena-cli, I realized at a certain point I have to do it because their releases are too frequent for node packages' generate.sh. If you'd like to have more influence upon it, I'd suggest to make it have it's own set of generated nix expressions. Plus, for sure you'd probably won't encounter merge conflicts with the generated expressions.

doronbehar · 2020-09-17T06:22:25Z

Are there files that you are missing?

I don't think so, but I haven't checked.

MMesch · 2020-09-18T22:10:51Z

@doronbehar I have rebased and regenerated node-packages.nix for the two relevant commits. Maybe I didn't understand if you wanted to do anything else. Let me know!

In case there are conflicts again, this can be rebased from the nodePackages folder with:

git fetch
git rebase origin/master
git checkout --theirs node-packages.nix
./generate.sh
git add .
git rebase --continue
git checkout --theirs node-packages.nix
./generate.sh
git add .
git rebase --continue

(takes ~2h on my machine)

pkgs/development/node-packages/default.bak

MMesch · 2020-09-19T12:09:07Z

Right! Sorry. It should be deleted. Do you want me to do it again? Doron Behar <notifications@github.com> schrieb am Sa., 19. Sep. 2020, 13:41:

…

***@***.**** commented on this pull request. ------------------------------ In pkgs/development/node-packages/default.bak <#97908 (comment)>: > @@ -0,0 +1,200 @@ +{ pkgs, nodejs, stdenv }: This file is a Git rebase leftover right? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#97908 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAT2HK2QY5ZJI4UQYK3UGZ3SGSKGVANCNFSM4RKVLFJQ> .

maintainers/maintainer-list.nix

doronbehar · 2020-09-19T12:17:05Z

You should be able to spare yourself running ./generate.sh, but you should need rewriting history a bit.

MMesch · 2020-09-19T12:30:01Z

I hope it's ok now @doronbehar . I ran the vega-cli and vega-lite tests that I posted above and they work.

pkgs/development/node-packages/default.nix

MMesch · 2020-09-19T13:08:12Z

Sorry for the formatting issues. How about now @doronbehar ?

doronbehar · 2020-09-19T14:34:43Z

pkgs/development/node-packages/default.nix

+        substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2pdf \
+          --replace "npx -p vega vg2pdf"  "${self.vega-cli}/bin/vg2pdf"
+        substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2svg \
+          --replace "npx -p vega vg2svg"  "${self.vega-cli}/bin/vg2svg"
+        substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2png \
+          --replace "npx -p vega vg2png"  "${self.vega-cli}/bin/vg2png"
+    '';


Suggested change

substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2pdf \

--replace "npx -p vega vg2pdf" "${self.vega-cli}/bin/vg2pdf"

substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2svg \

--replace "npx -p vega vg2svg" "${self.vega-cli}/bin/vg2svg"

substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2png \

--replace "npx -p vega vg2png" "${self.vega-cli}/bin/vg2png"

'';

substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2pdf \

--replace "npx -p vega vg2pdf" "${self.vega-cli}/bin/vg2pdf"

substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2svg \

--replace "npx -p vega vg2svg" "${self.vega-cli}/bin/vg2svg"

substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2png \

--replace "npx -p vega vg2png" "${self.vega-cli}/bin/vg2png"

'';

Sorry for the nitpick, but it should be perfect now!

doronbehar · 2020-09-19T14:49:42Z

@MMesch after fixing once more the indentation, please apply this patch:

diff --git i/pkgs/development/node-packages/default.nix w/pkgs/development/node-packages/default.nix
index 71b9bd77411..8d8d9fc89ae 100644
--- i/pkgs/development/node-packages/default.nix
+++ w/pkgs/development/node-packages/default.nix
@@ -181,6 +181,8 @@ let
         # https://sharp.pixelplumbing.com/install
         vips
 
+        libsecret
+        self.node-gyp-build
         self.node-pre-gyp
       ];
     };

And commit it as: joplin: fix build by adding libsecret and node-gyp-build.

It's not related to your PR, but these are new deps that your update to all of node packages require - since you ran ./generate.sh.

MMesch · 2020-09-19T14:51:00Z

Ah s... I forgot that one. Unfortunately I can't do it until tomorrow. If you have the time to amend this and merge go ahead, otherwise it has to wait. Thabks for the help and sorry for all the work :( I tried to use nixfmt on this but it changes a lot of other things. Doron Behar <notifications@github.com> schrieb am Sa., 19. Sep. 2020, 16:34:

…

***@***.**** commented on this pull request. ------------------------------ In pkgs/development/node-packages/default.nix <#97908 (comment)>: > + substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2pdf \ + --replace "npx -p vega vg2pdf" "${self.vega-cli}/bin/vg2pdf" + substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2svg \ + --replace "npx -p vega vg2svg" "${self.vega-cli}/bin/vg2svg" + substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2png \ + --replace "npx -p vega vg2png" "${self.vega-cli}/bin/vg2png" + ''; ⬇️ Suggested change - substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2pdf \ - --replace "npx -p vega vg2pdf" "${self.vega-cli}/bin/vg2pdf" - substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2svg \ - --replace "npx -p vega vg2svg" "${self.vega-cli}/bin/vg2svg" - substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2png \ - --replace "npx -p vega vg2png" "${self.vega-cli}/bin/vg2png" - ''; + substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2pdf \ + --replace "npx -p vega vg2pdf" "${self.vega-cli}/bin/vg2pdf" + substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2svg \ + --replace "npx -p vega vg2svg" "${self.vega-cli}/bin/vg2svg" + substituteInPlace $out/lib/node_modules/vega-lite/bin/vl2png \ + --replace "npx -p vega vg2png" "${self.vega-cli}/bin/vg2png" + ''; Sorry for the nitpick, but it should be perfect now! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#97908 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAT2HK7AGDYY33KHZW4DWL3SGS6RDANCNFSM4RKVLFJQ> .

Co-authored-by: Doron Behar <doron.behar@gmail.com>

ZHF: NixOS#97479

MMesch · 2020-09-20T18:23:31Z

Many thanks for you help @doronbehar . How can I help with your PR?

doronbehar · 2020-09-20T22:00:43Z

TBH I only want someone to approve it for the record, and verify that it works. And comments upon meta data or such would be welcome #96961 .

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 11-100 labels Sep 13, 2020

MMesch changed the title ~~add nodePackages.vega-cli: init at version 5.15.0~~ add nodePackages vega-cli and vega-lite Sep 13, 2020

MMesch mentioned this pull request Sep 14, 2020

Move all CLI scripts to be just node scripts instead of shell scripts vega/vega-lite#6863

Closed

MMesch force-pushed the mmesch/vega branch from 5a3cccf to 690fffc Compare September 14, 2020 07:46

MMesch force-pushed the mmesch/vega branch from 690fffc to 18d1580 Compare September 18, 2020 22:06

ofborg bot added 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 1 10.rebuild-linux: 11-100 labels Sep 18, 2020

doronbehar reviewed Sep 19, 2020

View reviewed changes

pkgs/development/node-packages/default.bak Outdated Show resolved Hide resolved

doronbehar reviewed Sep 19, 2020

View reviewed changes

maintainers/maintainer-list.nix Show resolved Hide resolved

MMesch added 2 commits September 19, 2020 14:26

maintainer: add mmesch

e192fe6

nodePackages.vega-cli: init at version 5.15.0

233331c

MMesch force-pushed the mmesch/vega branch from 18d1580 to ffefb19 Compare September 19, 2020 12:28

doronbehar requested changes Sep 19, 2020

View reviewed changes

pkgs/development/node-packages/default.nix Outdated Show resolved Hide resolved

pkgs/development/node-packages/default.nix Outdated Show resolved Hide resolved

MMesch force-pushed the mmesch/vega branch 3 times, most recently from 773aa67 to ae9af63 Compare September 19, 2020 13:05

doronbehar reviewed Sep 19, 2020

View reviewed changes

nodePackages.vega-lite: init at version 4.15.0

7fb1ee0

Co-authored-by: Doron Behar <doron.behar@gmail.com>

doronbehar force-pushed the mmesch/vega branch from ae9af63 to 90ef425 Compare September 19, 2020 16:34

joplin: fix build by adding missing deps

9c12892

ZHF: NixOS#97479

doronbehar force-pushed the mmesch/vega branch from 90ef425 to 9c12892 Compare September 19, 2020 16:40

doronbehar merged commit 32818fc into NixOS:master Sep 19, 2020

infinisil referenced this pull request Oct 20, 2020

Rebase mermaid, update nodePackages along the way

c1834f4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add nodePackages vega-cli and vega-lite #97908

add nodePackages vega-cli and vega-lite #97908

MMesch commented Sep 13, 2020 •

edited

MMesch commented Sep 14, 2020

nixos-discourse commented Sep 14, 2020

doronbehar commented Sep 16, 2020

MMesch commented Sep 16, 2020

doronbehar commented Sep 17, 2020

doronbehar commented Sep 17, 2020

MMesch commented Sep 18, 2020

MMesch commented Sep 19, 2020 via email

doronbehar commented Sep 19, 2020

MMesch commented Sep 19, 2020

MMesch commented Sep 19, 2020

doronbehar Sep 19, 2020

doronbehar commented Sep 19, 2020

MMesch commented Sep 19, 2020 via email

MMesch commented Sep 20, 2020

doronbehar commented Sep 20, 2020

add nodePackages vega-cli and vega-lite #97908

add nodePackages vega-cli and vega-lite #97908

Conversation

MMesch commented Sep 13, 2020 • edited

Motivation for this change

Things done

MMesch commented Sep 14, 2020

nixos-discourse commented Sep 14, 2020

doronbehar commented Sep 16, 2020

MMesch commented Sep 16, 2020

doronbehar commented Sep 17, 2020

doronbehar commented Sep 17, 2020

MMesch commented Sep 18, 2020

MMesch commented Sep 19, 2020 via email

doronbehar commented Sep 19, 2020

MMesch commented Sep 19, 2020

MMesch commented Sep 19, 2020

doronbehar Sep 19, 2020

Choose a reason for hiding this comment

doronbehar commented Sep 19, 2020

MMesch commented Sep 19, 2020 via email

MMesch commented Sep 20, 2020

doronbehar commented Sep 20, 2020

MMesch commented Sep 13, 2020 •

edited