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

nodePackages: regenerate with node2nix 1.9.0 #110545

Merged
merged 4 commits into from
Jan 25, 2021

Conversation

svanderburg
Copy link
Member

Motivation for this change

This update provides updated Nix expressions generated with node2nix 1.9.0

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.

@svanderburg
Copy link
Member Author

svanderburg commented Jan 22, 2021

Most of it is done. There are still a bunch of packages I could get some help with:

@SuperSandro2000 SuperSandro2000 marked this pull request as draft January 23, 2021 12:18
@SuperSandro2000
Copy link
Member

@svanderburg please fix the eval error.

@svanderburg
Copy link
Member Author

svanderburg commented Jan 23, 2021

@SuperSandro2000 This evaluation failure is caused by the ldgallery/viewer package, which I don't know yet how to regenerate properly (see the list above).

@pacien @zowoq Could you help me fixing this package? Or alternatively, tell me how you invoke the generation script?

@pacien
Copy link
Contributor

pacien commented Jan 23, 2021

The generation script of the ldgallery/viewer package can be invoked as follows:

cd pkgs/tools/graphics/ldgallery/viewer
./generate.sh v2.0

@pacien
Copy link
Contributor

pacien commented Jan 23, 2021

I forgot to mention that the last four lines of ldgallery/viewer/generate.sh contain an ugly fix for the util-linux rename. Those lines probably need to be removed now that the issue is fixed in node2nix itself.

@svanderburg
Copy link
Member Author

@pacien Thanks, this seemed to do the trick. Just made a commit.

I also modified these last two lines and the node2nix invocation: the --no-copy-node-env will always force the generator not to overwrite the existing node-env.nix file that the expression is referring to.

@turboMaCk
Copy link
Member

turboMaCk commented Jan 23, 2021

For the record once this is merged we can act on this todo (I can take care of that):

https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/elm/packages/generate-node-packages.sh#L13-L15

it's also an example of how you can share the node-env.nix between different regeneration scripts if you need to do so.

@svanderburg
Copy link
Member Author

@turboMaCk Good one! Yes let's have a look at it as soon as the work for this PR is done.

The checks now have passed. I'm currently investigating how to fix spacegun.

@svanderburg
Copy link
Member Author

I made a little bit more progress with spacegun. The missing global dependency: node-gyp-build was added.

Now still facing a TypeScript compilation error:

ERROR in [at-loader] ./src/file-loading.ts:7:5 
    TS2322: Type 'string | object | undefined' is not assignable to type 'object'.
  Type 'undefined' is not assignable to type 'object'.

Copy link
Contributor

@freezeboy freezeboy left a comment

Choose a reason for hiding this comment

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

LGTM for my packages (just trivial changes in the generated files)

  • newman
  • n8n
  • commitizen

…t project using the package-lock.json configuration
@ofborg ofborg bot requested a review from freezeboy January 24, 2021 17:00
@svanderburg
Copy link
Member Author

I also fixed spacegun by implementing a workaround that checks out the Git repo and uses the package-lock.json file.

I believe there are no impediments anymore in getting this PR integrated.

@svanderburg svanderburg marked this pull request as ready for review January 24, 2021 17:05
@svanderburg svanderburg changed the title [WIP] nodePackages: regenerate with node2nix 1.9.0 nodePackages: regenerate with node2nix 1.9.0 Jan 24, 2021
@turboMaCk
Copy link
Member

@svanderburg I've pulled this branch locally and was able to build spacegun 👍

Let me try to run nix-review...

@turboMaCk
Copy link
Member

turboMaCk commented Jan 24, 2021

I've tried to build spacegun on macos machine I have laying around and node-gyp fails to build there:

../../nan/nan_typedarray_contents.h:34:43: warning: 'GetContents' is deprecated: Use GetBackingStore. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
      data   = static_cast<char*>(buffer->GetContents().Data()) + byte_offset;
                                          ^
/nix/store/9ydhbjglc5ixy1vw7igvp1hg1jph431j-node-sources/deps/v8/include/v8.h:5272:3: note: 'GetContents' has been explicitly marked deprecated here
  V8_DEPRECATE_SOON("Use GetBackingStore. See http://crbug.com/v8/9908.")
  ^
/nix/store/9ydhbjglc5ixy1vw7igvp1hg1jph431j-node-sources/deps/v8/include/v8config.h:402:39: note: expanded from macro 'V8_DEPRECATE_SOON'
# define V8_DEPRECATE_SOON(message) [[deprecated(message)]]
                                      ^
../fsevents.cc:10:10: fatal error: 'CoreServices/CoreServices.h' file not found
#include "CoreServices/CoreServices.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Not sure if this suppose to be supported or not, I'm not using macos normally myself. I tried and it fails in master as well. I think it should be fixable by inclusion of CoreServices but I would rather consider fixing it in separate PR. spaceship also seems to have ungodly number of dependencies 😅

Copy link
Member

@turboMaCk turboMaCk left a comment

Choose a reason for hiding this comment

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

LGTM

@svanderburg
Copy link
Member Author

@turboMaCk Judging from the kind of error, I think it's safe to conclude that it was already broken on darwin. I'm pretty sure that a change in the generator could not have caused this.

@turboMaCk
Copy link
Member

@svanderburg it definitely was. I've tested master and it fails there as well.

@svanderburg
Copy link
Member Author

Ok, I'll leave this open for just a short while to give other people the opportunity to respond.

Otherwise, I'll integrate this PR soon.

@svanderburg
Copy link
Member Author

No further comments received. I'll merge this.

@svanderburg svanderburg merged commit 40f27ff into NixOS:master Jan 25, 2021
@raboof
Copy link
Member

raboof commented Jan 25, 2021

nixpkgs-review now fails on master (so will now fail everywhere AFAICS?):

HEAD is now at 40f27ff2bef Merge pull request #110545 from svanderburg/node2nix-update
$ nix-env -f /home/aengelen/.cache/nixpkgs-review/rev-f217c0ea7c148ddc0103347051555c7c252dcafb/nixpkgs -qaP --xml --out-path --show-trace
error: stack overflow (possible infinite recursion)

Might be related to the zigbee2mqtt change in this PR, we saw something similar in #110394 .

@svanderburg
Copy link
Member Author

@raboof This is weird. I need to investigate it. In the meantime we should probably revert with this PR: #110752

@SuperSandro2000 feel free to do the revert

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.

6 participants