NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.16k stars 14.19k forks source link

Build failure: nodePackages_latest.nodejs #355919

Open JohnRTitor opened 1 day ago

JohnRTitor commented 1 day ago

Steps To Reproduce

Steps to reproduce the behavior:

  1. build nodePackages_latest.nodejs from master

    Build log

Build Log ``` ┃ > ... ┃ > ok 4059 known_issues/test-url-parse-conformance ┃ > --- ┃ > duration_ms: 56.50900 ┃ > ... ┃ > ok 4060 known_issues/test-vm-function-declaration-uses-define ┃ > --- ┃ > duration_ms: 36.65000 ┃ > ... ┃ > ok 4061 known_issues/test-vm-timeout-escape-nexttick ┃ > --- ┃ > duration_ms: 806.78800 ┃ > ... ┃ > ┃ > Failed tests: ┃ > out/Release/node /build/node-v23.2.0/test/parallel/test-os.js ┃ > make: *** [Makefile:558: test-ci-js] Error 1 ┃ For full logs, run 'nix log /nix/store/ahgdyngidcphsmb7zn5b1lny589pyjdc-nodejs-23.2.0.drv'. ```

Additional context

Found it while building zed-editor on master.

Metadata

❯ nix-info -m

Notify maintainers

@aduh95 @Ma27


Note for maintainers: Please tag this issue in your PR.


Add a :+1: reaction to issues you find important.

aduh95 commented 1 day ago

Failed tests: ┃ > out/Release/node /build/node-v23.2.0/test/parallel/test-os.js

Could you share the output of this test?

JohnRTitor commented 1 day ago

Could you point me how to do that? I am not familiar with nodePackages, just got the log from the Nix build command.

aduh95 commented 1 day ago

As it says on the output:

For full logs, run 'nix log /nix/store/ahgdyngidcphsmb7zn5b1lny589pyjdc-nodejs-23.2.0.drv'.

From there, you can search for not ok (or for the test name, but I find not ok easier to remember).

JohnRTitor commented 1 day ago

Got it.

not ok 2075 parallel/test-os
  ---
  duration_ms: 37.37300
  severity: fail
  exitcode: 1
  stack: |-
    node:os:317
        throw new ERR_SYSTEM_ERROR(ctx);
        ^

    SystemError [ERR_SYSTEM_ERROR]: A system error occurred: uv_os_setpriority returned EACCES (permission denied)
        at Object.setPriority (node:os:317:11)
        at Object.<anonymous> (/build/node-v23.2.0/test/parallel/test-os.js:87:6)
        at Module._compile (node:internal/modules/cjs/loader:1550:14)
        at Object..js (node:internal/modules/cjs/loader:1702:10)
        at Module.load (node:internal/modules/cjs/loader:1307:32)
        at Function._load (node:internal/modules/cjs/loader:1121:12)
        at TracingChannel.traceSync (node:diagnostics_channel:322:14)
        at wrapModuleLoad (node:internal/modules/cjs/loader:219:24)
        at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:170:5)
        at node:internal/main/run_main_module:36:49 {
      code: 'ERR_SYSTEM_ERROR',
      info: {
        errno: -13,
        code: 'EACCES',
        message: 'permission denied',
        syscall: 'uv_os_setpriority'
      },
      errno: [Getter/Setter],
      syscall: [Getter/Setter]
    }

    Node.js v23.2.0
  ...
ok 2076 parallel/test-node-output-vm
  ---
  duration_ms: 331.84100
  ...
aduh95 commented 1 day ago

OK that's definitely interesting! Could you confirm if the test passes if you revert https://github.com/nodejs/node/commit/a094a8166cd772f89e92b5deef168e5e599fa815?

LiviaMedeiros commented 1 day ago

Does niceness on NixOS work differently than on most Unix systems? The os.constants.priority.PRIORITY_LOW is 19, there should be no numbers higher than that, and setting priority on same or lower level should be fine.

If it's indeed different, what's the returned value of require('node:os').type() on NixOS systems?

JohnRTitor commented 1 day ago

OK that's definitely interesting! Could you confirm if the test passes if you revert nodejs/node@a094a81?

Well this patch does not apply. https://github.com/JohnRTitor/nixpkgs/commit/24c5cc0adbf278652bdbf5695674f824f7f20c23

aduh95 commented 1 day ago

Well this patch does not apply. JohnRTitor@24c5cc0

Ah right, my bad, that commit didn't make it into 23.2.0. I reckon we have this test disabled on macOS, the comment says it's sandbox related: https://github.com/NixOS/nixpkgs/blob/4f93e624c544fb86e60cba7620c0d29083258809/pkgs/development/web/nodejs/nodejs.nix#L312-L314

In any case, we should probably amend the Node.js test to not fail on EACCES

JohnRTitor commented 1 day ago

Nice, can we cherry-pick the commit here or will it be part of a minor patch release?

JohnRTitor commented 23 hours ago

Looks like this does not directly apply either, we will either need to rebase and vendor the patch or disable the test, while we wait for a release.

aduh95 commented 23 hours ago

Well you would need to first apply https://github.com/nodejs/node/commit/a094a8166cd772f89e92b5deef168e5e599fa815 and confirm it's still an issue with that patch applies, then cherry-pick my commit on top of it. It should make its way to a future minor or patch release yes.

JohnRTitor commented 23 hours ago

Does niceness on NixOS work differently than on most Unix systems?

I don't think it is. But I am not entirely sure.

If it's indeed different, what's the returned value of require('node:os').type() on NixOS systems?

@LiviaMedeiros

image

EDIT: I tried running node seperately on my system, it assigns 16 nice value to the node interpreter.

JohnRTitor commented 23 hours ago

Well you would need to first apply nodejs/node@a094a81

I mean this fails to apply. 🥲

diff --git a/pkgs/development/web/nodejs/v23.nix b/pkgs/development/web/nodejs/v23.nix
index a34e53375e69..6a5b7626eb54 100644
--- a/pkgs/development/web/nodejs/v23.nix
+++ b/pkgs/development/web/nodejs/v23.nix
@@ -53,5 +53,10 @@ buildNodejs {
       hash = "sha256-gmIyiSyNzC3pClL1SM2YicckWM+/2tsbV1xv2S3d5G0=";
       revert = true;
     })
+    # possible fix for https://github.com/NixOS/nixpkgs/issues/355919
+    (fetchpatch2 {
+      url = "https://github.com/nodejs/node/commit/a094a8166cd772f89e92b5deef168e5e599fa815.patch?full_index=1";
+      hash = "sha256-Jwf5D/QvoKNN36IqWKnWYXIpHdkm/o1rmq+G+n05EG0=";
+    })
   ];
 }
aduh95 commented 21 hours ago

Well you would need to first apply nodejs/node@a094a81

I mean this fails to apply. 🥲

diff --git a/pkgs/development/web/nodejs/v23.nix b/pkgs/development/web/nodejs/v23.nix
index a34e53375e69..6a5b7626eb54 100644
--- a/pkgs/development/web/nodejs/v23.nix
+++ b/pkgs/development/web/nodejs/v23.nix
@@ -53,5 +53,10 @@ buildNodejs {
       hash = "sha256-gmIyiSyNzC3pClL1SM2YicckWM+/2tsbV1xv2S3d5G0=";
       revert = true;
     })
+    # possible fix for https://github.com/NixOS/nixpkgs/issues/355919
+    (fetchpatch2 {
+      url = "https://github.com/nodejs/node/commit/a094a8166cd772f89e92b5deef168e5e599fa815.patch?full_index=1";
+      hash = "sha256-Jwf5D/QvoKNN36IqWKnWYXIpHdkm/o1rmq+G+n05EG0=";
+    })
   ];
 }

You need to fix the hash, you should get a different hash depending if you pass revert=true or not

LiviaMedeiros commented 21 hours ago

EDIT: I tried running node seperately on my system, it assigns 16 nice value to the node interpreter.

Yep, this seems to be the issue. Before https://github.com/nodejs/node/commit/a094a8166cd772f89e92b5deef168e5e599fa815 the test niceness was hardcoded to 10; and process can't lower its niceness unless you're root. After applying that commit, it will check current priority and try 19 which should not cause EACCES.

aduh95 commented 13 hours ago

The hash you should be using is sha256-5FZfozYWRa1ZI/f+e+xpdn974Jg2DbiHbua13XUQP5E= for https://github.com/nodejs/node/commit/a094a8166cd772f89e92b5deef168e5e599fa815.patch?full_index=1

JohnRTitor commented 13 hours ago

Yeah I was just building the package. Looks like we need both https://github.com/nodejs/node/commit/a094a8166cd772f89e92b5deef168e5e599fa815 and https://github.com/nodejs/node/pull/55863/commits/f60cc5871b3950a078356dbd30cb16ba64890916 for the test failure to be fixed.

aduh95 commented 12 hours ago

Can you clarify what's the behavior without https://github.com/nodejs/node/commit/f60cc5871b3950a078356dbd30cb16ba64890916? Same error as without https://github.com/nodejs/node/commit/a094a8166cd772f89e92b5deef168e5e599fa815?

JohnRTitor commented 11 hours ago

The test indeed failed with just https://github.com/nodejs/node/commit/a094a8166cd772f89e92b5deef168e5e599fa815 applied and needed https://github.com/nodejs/node/commit/f60cc5871b3950a078356dbd30cb16ba64890916 to work.