nodePackages.postcss-cli: fix execution of binary #130027

Luflosi · 2021-07-12T15:55:07Z

Motivation for this change

Without this change, executing the postcss binary results in the following error:
Error: Cannot find module 'postcss'

Things done

roberth

LGTM + nitpick.

pkgs/development/node-packages/default.nix

roberth · 2021-07-12T16:27:12Z

@GrahamcOfBorg build nodePackages.postcss-cli

Perhaps a passthru.tests would be nice.

Without this change, executing the postcss binary results in the following error: Error: Cannot find module 'postcss'

r-rmcgibbo · 2021-07-12T16:44:41Z

Result of nixpkgs-review pr 130027 at 87f1708 run on aarch64-linux 1

1 package skipped due to time constraints:

lumo

18 packages built successfully:

parity-ui
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
teleprompter
vscode-extensions.matklad.rust-analyzer
vscode-extensions.vadimcn.vscode-lldb

Result of nixpkgs-review pr 130027 at 87f1708 run on x86_64-linux 1

1 package skipped due to time constraints:

lumo

41 packages built successfully:

balanceofsatoshis
bitwarden-cli
castnow
create-cycle-app
epgstation
joplin
mirakurun
netlify-cli
parity-ui
postcss-cli
pulp
pyright
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
slack
teams
teleprompter
thelounge
timedoctor
vimPlugins.coc-clangd
vimPlugins.coc-explorer
vimPlugins.coc-prettier
vimPlugins.coc-pyright
vimPlugins.coc-rust-analyzer
vimPlugins.coc-stylelint
vscode-extensions.matklad.rust-analyzer
vscode-extensions.ms-python.vscode-pylance
vscode-extensions.vadimcn.vscode-lldb

36 suggestions:

warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:87962:5:
```
      |
87962 |     version = "2.1.2";
      |     ^
```
warning: missing-phase-hooks
installPhase should probably contain runHook preInstall.

Near pkgs/development/node-packages/node-env.nix:416:7:
```
    |
416 |       installPhase = ''
    |       ^
```
warning: missing-phase-hooks
configurePhase should probably contain runHook preConfigure and runHook postConfigure.

Near pkgs/misc/vim-plugins/build-vim-plugin.nix:28:14:
```
   |
28 |       inherit unpackPhase configurePhase buildPhase addonInfo preInstall postInstall;
   |              ^
```
warning: license-missing
Package is missing a license.

Near pkgs/development/node-packages/default.nix:211:7:
```
    |
211 |       meta.mainProgram = "postcss";
    |       ^
```

warning: unused-argument

Unused argument: stdenv.
Near pkgs/development/node-packages/node-packages.nix:3:46:

  |
3 | {nodeEnv, fetchurl, fetchgit, nix-gitignore, stdenv, lib, globalBuildInputs ? []}:
  |                                              ^

warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:94510:5:
```
      |
94510 |     version = "4.1.23";
      |     ^
```
warning: build-tools-in-build-inputs
nodePackages.node-pre-gyp is a build tool so it likely goes to nativeBuildInputs, not buildInputs.

Near pkgs/development/node-packages/node-env.nix:401:7:
```
    |
401 |       buildInputs = [ tarWrapper python nodejs ]
    |       ^
```
warning: build-tools-in-build-inputs
util-linux is a build tool so it likely goes to nativeBuildInputs, not buildInputs.

Near pkgs/development/node-packages/node-env.nix:401:7:
```
    |
401 |       buildInputs = [ tarWrapper python nodejs ]
    |       ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/default.nix:211:7:
```
    |
211 |       meta.mainProgram = "postcss";
    |       ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:102028:5:
```
       |
102028 |     meta = {
       |     ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:88668:5:
```
      |
88668 |     meta = {
      |     ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:112742:5:
```
       |
112742 |     meta = {
       |     ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:101944:5:
```
       |
101944 |     meta = {
       |     ^
```

warning: unused-argument

Unused argument: lib.
Near pkgs/development/node-packages/node-packages.nix:3:54:

  |
3 | {nodeEnv, fetchurl, fetchgit, nix-gitignore, stdenv, lib, globalBuildInputs ? []}:
  |                                                      ^

warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:69637:5:
```
      |
69637 |     meta = {
      |     ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:96464:5:
```
      |
96464 |     meta = {
      |     ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:74123:5:
```
      |
74123 |     meta = {
      |     ^
```
warning: missing-phase-hooks
buildPhase should probably contain runHook preBuild and runHook postBuild.

Near pkgs/misc/vim-plugins/build-vim-plugin.nix:28:14:
```
   |
28 |       inherit unpackPhase configurePhase buildPhase addonInfo preInstall postInstall;
   |              ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:104594:5:
```
       |
104594 |     meta = {
       |     ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/default.nix:65:7:
```
   |
65 |       meta.mainProgram = "bw";
   |       ^
```
warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:101519:5:
```
       |
101519 |     version = "8.3.1";
       |     ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:94244:5:
```
      |
94244 |     meta = {
      |     ^
```
warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:112306:5:
```
       |
112306 |     version = "4.2.0";
       |     ^
```
warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:67520:5:
```
      |
67520 |     version = "10.6.1";
      |     ^
```
warning: license-missing
Package is missing a license.

Near pkgs/development/node-packages/default.nix:65:7:
```
   |
65 |       meta.mainProgram = "bw";
   |       ^
```
warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:69226:5:
```
      |
69226 |     version = "0.6.0";
      |     ^
```
warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:102022:5:
```
       |
102022 |     version = "1.1.156";
       |     ^
```
warning: missing-phase-hooks
buildPhase should probably contain runHook preBuild and runHook postBuild.

Near pkgs/development/node-packages/node-env.nix:409:14:
```
    |
409 |       inherit dontNpmInstall preRebuild unpackPhase buildPhase;
    |              ^
```
warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:93977:5:
```
      |
93977 |     version = "3.6.0";
      |     ^
```
warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:65226:5:
```
      |
65226 |     version = "1.17.1";
      |     ^
```

warning: unused-argument

Unused argument: nix-gitignore.
Near pkgs/development/node-packages/node-packages.nix:3:31:

  |
3 | {nodeEnv, fetchurl, fetchgit, nix-gitignore, stdenv, lib, globalBuildInputs ? []}:
  |                               ^

warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:101691:5:
```
       |
101691 |     version = "15.0.0";
       |     ^
```
warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:73968:5:
```
      |
73968 |     version = "5.0.0";
      |     ^
```
warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/development/node-packages/node-packages.nix:68241:5:
```
      |
68241 |     meta = {
      |     ^
```
warning: name-and-version
Did you mean to pass pname instead of name to mkDerivation?

Near pkgs/development/node-packages/node-env.nix:400:7:
```
    |
400 |       name = "node_${name}-${version}";
    |       ^
```
Near pkgs/development/node-packages/node-packages.nix:104281:5:
```
       |
104281 |     version = "0.12.1";
       |     ^
```
warning: build-tools-in-build-inputs
nodePackages.node-gyp-build is a build tool so it likely goes to nativeBuildInputs, not buildInputs.

Near pkgs/development/node-packages/node-env.nix:401:7:
```
    |
401 |       buildInputs = [ tarWrapper python nodejs ]
    |       ^
```

SuperSandro2000 · 2021-07-13T23:57:15Z

Merging this before it has a merge conflict. We can add the passthru tests later.

Luflosi · 2021-07-14T12:21:50Z

How can I add such a test? I'm having trouble finding a good example in Nixpkgs. I don't think any Node packages have any tests.

roberth · 2021-07-14T12:29:31Z

The manual has a section. It's somewhat of a new thing to write this kind of test.

Tests in checkPhase are more prevalent, but those don't run in a clean environment, which I think is quite important because of the presence of node modules in the build environment, which is not realistic.

Luflosi · 2021-07-14T12:46:14Z

I saw that section. I could probably successfully write a test for a "normal" package.
However this is a Node package, where the list of packages is in pkgs/development/node-packages/node-packages.json, the automatically generated package definitions are in pkgs/development/node-packages/node-packages.nix and my override is in pkgs/development/node-packages/default.nix. I could probably add

passthru.tests = {
  simple-execution = callPackage ./tests.nix { };
};

to the override but where would I put test.nix? Putting it in pkgs/development/node-packages/ seems very wrong as there are no other tests there. I could probably inline the tests but I don't think it's done like that anywhere else, is it?

roberth · 2021-07-14T12:59:45Z

You could definitely add the passthru.tests attribute in the overrides file.
I'm never hard to please when someone wants to write a test and you're the first one to do it for a node package, so more power to you.
If I were doing it, I'd use callPackage and put the file at a path like pkgs/development/node-packages/package-tests/postcss-cli.nix.
I've written this kind of test inline before, but only when it was one package per file.

Luflosi · 2021-07-14T16:10:04Z

Thank you for your suggestion. I created #130204 to add a basic test.

ofborg bot requested a review from midchildan July 12, 2021 16:05

ofborg bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Jul 12, 2021

roberth reviewed Jul 12, 2021

View reviewed changes

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

Luflosi added 2 commits July 12, 2021 18:36

nodePackages.postcss: init at 8.3.5

b3d8979

nodePackages.postcss-cli: fix execution of binary

87f1708

Without this change, executing the postcss binary results in the following error: Error: Cannot find module 'postcss'

Luflosi force-pushed the fix-postcss-cli branch from 4a55b01 to 87f1708 Compare July 12, 2021 16:37

SuperSandro2000 merged commit 5fa835b into NixOS:master Jul 13, 2021

Luflosi deleted the fix-postcss-cli branch July 14, 2021 11:59

Luflosi mentioned this pull request Jul 14, 2021

nodePackages.postcss-cli: add passthru.tests #130204

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nodePackages.postcss-cli: fix execution of binary #130027

nodePackages.postcss-cli: fix execution of binary #130027

Luflosi commented Jul 12, 2021

roberth left a comment

roberth commented Jul 12, 2021 •

edited

Loading

r-rmcgibbo commented Jul 12, 2021 •

edited

Loading

SuperSandro2000 commented Jul 13, 2021

Luflosi commented Jul 14, 2021

roberth commented Jul 14, 2021

Luflosi commented Jul 14, 2021

roberth commented Jul 14, 2021

Luflosi commented Jul 14, 2021

nodePackages.postcss-cli: fix execution of binary #130027

nodePackages.postcss-cli: fix execution of binary #130027

Conversation

Luflosi commented Jul 12, 2021

Motivation for this change

Things done

roberth left a comment

Choose a reason for hiding this comment

roberth commented Jul 12, 2021 • edited Loading

r-rmcgibbo commented Jul 12, 2021 • edited Loading

SuperSandro2000 commented Jul 13, 2021

Luflosi commented Jul 14, 2021

roberth commented Jul 14, 2021

Luflosi commented Jul 14, 2021

roberth commented Jul 14, 2021

Luflosi commented Jul 14, 2021

roberth commented Jul 12, 2021 •

edited

Loading

r-rmcgibbo commented Jul 12, 2021 •

edited

Loading