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.

Sign up for GitHub

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

node-packages: updates, including node2nix 1.9.0 #110394

Closed
wants to merge 3 commits into from
Closed

node-packages: updates, including node2nix 1.9.0 #110394

wants to merge 3 commits into from

Conversation

raboof
Copy link
Member

@raboof raboof commented Jan 21, 2021

Motivation for this change

My real goal is to add a packages, but there are many updates available
that are unrelated to the packages I want to add. To keep the diff of
the actual change small, it seemed to make sense to propose these
updates.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@raboof raboof force-pushed the node-packages-updates-20200121 branch 2 times, most recently from 5eab65e to 82dc994 Compare January 21, 2021 21:28
SuperSandro2000
SuperSandro2000 requested changes Jan 21, 2021
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Also please fix the eval error.

pkgs/development/node-packages/node-env.nix Show resolved Hide resolved
@raboof
Copy link
Member Author

raboof commented Jan 22, 2021

Also please fix the eval error.

Oops, will do!

@raboof raboof marked this pull request as draft January 22, 2021 07:54
@SuperSandro2000
Copy link
Member

I think we need to first update node2nix and then regenerate this file with it. I am not sure how to do that except running it twice and hope that it works.

@raboof
Copy link
Member Author

raboof commented Jan 22, 2021 •
edited
Loading

I think we need to first update node2nix and then regenerate this file with it. I am not sure how to do that except running it twice and hope that it works.

Right, so we can reproduce the problem with:

curl -o outpaths.nix https://raw.githubusercontent.com/NixOS/ofborg/released/ofborg/src/outpaths.nix
GC_INITIAL_HEAP_SIZE=4g nix-env -f ./outpaths.nix -qaP --no-name --out-path --arg checkMeta true > out-paths

... and then use that to fix all 31 places where we generated files with node2nix 1.8.0 but modified the generated file to point to /pkgs/development/node-packages/node-env.nix (which we're now updating to 1.9.0.

Alternatively we could keep 2 versions of node-env.nix (a 1.8.0 and a 1.9.0) around, but let's not do that if we can avoid it.

@raboof raboof changed the title node-packages: updates without further changes node-packages: updates, including node2nix 1.9.0 Jan 22, 2021
@ofborg ofborg bot requested review from michojel, midchildan and iblech January 22, 2021 11:38
@raboof raboof changed the base branch from master to staging January 22, 2021 11:45
@raboof raboof marked this pull request as ready for review January 22, 2021 13:13
@raboof
Copy link
Member Author

raboof commented Jan 22, 2021 •
edited
Loading

nixpkgs-review does not seem to work on this PR, but unclear if that's a problem with nixpkgs-review or with the PR... @ryantm can you see what's wrong?

@ryantm
Copy link
Member

ryantm commented Jan 22, 2021

@raboof have you already fixed the eval errors mentioned above?

@raboof
Copy link
Member Author

raboof commented Jan 22, 2021

well, I'm not entirely sure, but

curl -o outpaths.nix https://raw.githubusercontent.com/NixOS/ofborg/released/ofborg/src/outpaths.nix
GC_INITIAL_HEAP_SIZE=4g nix-env -f ./outpaths.nix -qaP --no-name --out-path --arg checkMeta true > out-paths

produced an error before and completes successfully now. Is there something else I can check?

SuperSandro2000
SuperSandro2000 reviewed Jan 23, 2021
View reviewed changes
pkgs/servers/zigbee2mqtt/update.sh Outdated
@@ -13,6 +13,7 @@ fi
curl -LO $ZIGBEE2MQTT/package.json
curl -LO $ZIGBEE2MQTT/npm-shrinkwrap.json

which node2nix
Copy link
Member

Choose a reason for hiding this comment

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

Is this a testing leftover?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, indeed, thanks, will fix

@SuperSandro2000
Copy link
Member

I think you need to revert any changes to zigbee2mqtt because of

Auto-merging pkgs/development/node-packages/node-env.nix
Automatic merge went well; stopped before committing as requested
error: stack overflow (possible infinite recursion)

@raboof raboof force-pushed the node-packages-updates-20200121 branch from 550cb5c to 1089bc1 Compare January 24, 2021 10:44
@raboof
Copy link
Member Author

raboof commented Jan 24, 2021

I think you need to revert any changes to zigbee2mqtt because of

Auto-merging pkgs/development/node-packages/node-env.nix
Automatic merge went well; stopped before committing as requested
error: stack overflow (possible infinite recursion)

I updated the PR to make only the minimal changes to make things evaluate for zigbee2mqtt, which indeed seems to make nixpkgs-review start again.

I'm curious how you found out zigbee2mqtt was the problem here - nix-env -f . -qaP --xml --out-path --show-trace --meta works without problems for me so didn't give any insight.

@SuperSandro2000
Copy link
Member

I'm curious how you found out zigbee2mqtt was the problem here - nix-env -f . -qaP --xml --out-path --show-trace --meta works without problems for me so didn't give any insight.

When running nixpkgs-review I got this. It prevents unstable from advancing and got reverted previously.

SuperSandro2000
SuperSandro2000 reviewed Jan 24, 2021
View reviewed changes
pkgs/applications/version-management/gitea/default.nix Outdated
@@ -2,6 +2,7 @@
, git, bash, gzip, openssh, pam
, sqliteSupport ? true
, pamSupport ? true
, nixosTests
Copy link
Member

Choose a reason for hiding this comment

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

Can you drop this commit?

@raboof
Copy link
Member Author

raboof commented Jan 24, 2021

Result of nixpkgs-review pr 110394 run on x86_64-linux 1

3 packages marked as broken and skipped:
  • dat
  • remarkjs
  • vimPlugins.coc-imselect
6 packages failed to build:
  • lumo
  • matrix-appservice-slack
  • sage (sagemath)
  • sageWithDoc
  • spacegun
  • vscode-extensions.vadimcn.vscode-lldb
113 packages built:
  • airfield
  • antora
  • base16-builder
  • bat-extras.prettybat
  • bitwarden-cli
  • castnow
  • create-cycle-app
  • cryptpad
  • emojione
  • epgstation
  • etcher
  • fast-cli
  • flood
  • fx
  • google-clasp
  • gtop
  • hueadm
  • imapnotify
  • imgbrd-grabber
  • iosevka
  • joplin
  • ldgallery
  • lessc
  • mastodon-bot
  • mirakurun
  • n8n
  • netlify-cli
  • nixui
  • parity-ui
  • pm2
  • postcss-cli
  • pscid
  • pulp
  • purescript-psa
  • python37Packages.batchspawner
  • python37Packages.dockerspawner
  • python37Packages.jupyterhub
  • python37Packages.jupyterhub-ldapauthenticator
  • python37Packages.jupyterhub-systemdspawner
  • python37Packages.jupyterhub-tmpauthenticator
  • python37Packages.oauthenticator
  • python38Packages.batchspawner
  • python38Packages.dockerspawner
  • python38Packages.jupyterhub
  • python38Packages.jupyterhub-ldapauthenticator
  • python38Packages.jupyterhub-systemdspawner
  • python38Packages.jupyterhub-tmpauthenticator
  • python38Packages.oauthenticator
  • python39Packages.batchspawner
  • python39Packages.dockerspawner
  • python39Packages.jupyterhub
  • python39Packages.jupyterhub-ldapauthenticator
  • python39Packages.jupyterhub-systemdspawner
  • python39Packages.jupyterhub-tmpauthenticator
  • python39Packages.oauthenticator
  • redoc-cli
  • ripcord
  • shout
  • slack (slack-dark)
  • sloc
  • styx
  • teleprompter
  • thelounge
  • triton
  • twemoji-color-font
  • uchiwa
  • vimPlugins.YouCompleteMe
  • vimPlugins.coc-css
  • vimPlugins.coc-diagnostic
  • vimPlugins.coc-emmet
  • vimPlugins.coc-eslint
  • vimPlugins.coc-git
  • vimPlugins.coc-go
  • vimPlugins.coc-highlight
  • vimPlugins.coc-html
  • vimPlugins.coc-java
  • vimPlugins.coc-jest
  • vimPlugins.coc-json
  • vimPlugins.coc-lists
  • vimPlugins.coc-markdownlint
  • vimPlugins.coc-metals
  • vimPlugins.coc-pairs
  • vimPlugins.coc-prettier
  • vimPlugins.coc-python
  • vimPlugins.coc-r-lsp
  • vimPlugins.coc-rls
  • vimPlugins.coc-rust-analyzer
  • vimPlugins.coc-smartf
  • vimPlugins.coc-snippets
  • vimPlugins.coc-solargraph
  • vimPlugins.coc-stylelint
  • vimPlugins.coc-tabnine
  • vimPlugins.coc-tslint
  • vimPlugins.coc-tslint-plugin
  • vimPlugins.coc-tsserver
  • vimPlugins.coc-vetur
  • vimPlugins.coc-vimlsp
  • vimPlugins.coc-vimtex
  • vimPlugins.coc-wxml
  • vimPlugins.coc-yaml
  • vimPlugins.coc-yank
  • vscode-extensions.matklad.rust-analyzer
  • vscode-extensions.ms-python.vscode-pylance
  • wasm-strip
  • wasm-text-gen
  • wast-refmt
  • webassemblyjs-cli
  • webassemblyjs-repl
  • whitebophir
  • wring
  • yaml-language-server
  • ycmd
  • zigbee2mqtt

@SuperSandro2000 SuperSandro2000 mentioned this pull request Jan 24, 2021
Closed
10 tasks
@raboof raboof force-pushed the node-packages-updates-20200121 branch from 1089bc1 to 773f1e1 Compare January 24, 2021 16:27
raboof added 3 commits January 24, 2021 20:27
@raboof
node-packages: updates without further changes
503909c
@raboof
nodePackages: generate with node2nix 1.9.0
35632d0
@raboof
node: update separately-packaged projects to node2nix 1.9.0
f00ff7b 
Didn't update zigbee2mqtt beyond the bare necessities because it
somehow confused nixpkgs-review

Didn't update spacegun beyond the bare necessities because it wouldn't
build anymore after the update.
@raboof raboof force-pushed the node-packages-updates-20200121 branch from 773f1e1 to f00ff7b Compare January 24, 2021 19:58
@SuperSandro2000
Copy link
Member

I just noticed you targeted staging which is not needed with only a few hundred rebuilds. If you change the base branch to master we should be good to go.

@raboof
Copy link
Member Author

raboof commented Jan 25, 2021

I just noticed you targeted staging which is not needed with only a few hundred rebuilds.

Ok!

If you change the base branch to master we should be good to go.

I fixed spacegun, but there are still a couple of build failures reported by nixpkgs-review (lumo, matrix-appservice-slack, sage (sagemath), sageWithDoc, vscode-extensions.vadimcn.vscode-lldb). Should we look into those before merging? lumo seems like something we'd perhaps not want to break, not familiar with the others.

@raboof raboof changed the base branch from staging to master January 25, 2021 08:30
@raboof
Copy link
Member Author

raboof commented Jan 25, 2021 •
edited
Loading

needs a rebase/re-generation... with some luck I can look into that tonight, if someone else wants to pick it up that's fine by me as well.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 25, 2021
@raboof
Copy link
Member Author

raboof commented Jan 25, 2021

I guess this PR was superseded by #110545

@raboof raboof closed this Jan 25, 2021
@raboof raboof mentioned this pull request Jan 25, 2021
Merged
10 tasks
@SuperSandro2000
Copy link
Member

I guess this PR was superseded by #110545

Not it is not because the PR you mentioned also updated zigbee2mqtt which breaks eval on master.

@SuperSandro2000 SuperSandro2000 mentioned this pull request Jan 25, 2021
Merged
@raboof
Copy link
Member Author

raboof commented Jan 25, 2021

I guess this PR was superseded by #110545

Not it is not because the PR you mentioned also updated zigbee2mqtt which breaks eval on master.

Makes sense. Still I think @svanderburg is better equipped to make the right fixes there than I am here, so let's focus on his work instead.

@raboof raboof closed this Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 requested changes

@michojel michojel Awaiting requested review from michojel

@midchildan midchildan Awaiting requested review from midchildan

@iblech iblech Awaiting requested review from iblech

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nodejs 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@raboof @SuperSandro2000 @ryantm