build: remove default values for `libname` by aduh95 · Pull Request #66790 · nodejs/node

aduh95 · 2026-10-16T13:47:12Z

By setting a default value, we override the pkg-config value, which is inconsistent with the other --shared-• flags.

legendecas

Lines 2200 to 2205 in f205c37

    
           default_libs = getattr(options, shared_lib + '_libname') 
        
           if default_libs: 
        
             libraries += [f'-l{l}' for l in default_libs.split(',')] 
        
           elif pkg_libs: 
        
             libraries += pkg_libs.split()

silently ignores a shared lib if no pkg config found, when --shared-* is set, without --shared-*-libname. Should it infer the default_libs from the lib name, and yield to the linker at build-time?

legendecas · 2026-10-16T17:59:24Z

I think the default_libs serves as a default value, yielding to the linter to find the library location at build-time.

aduh95 · 2026-10-16T19:08:50Z

I think the default_libs serves as a default value

default_libs is kind of a misnomer, because it is either the default value (which should be the fallback) OR the user provided value (which should take precedence over the pkg-config one). With the change in this PR, it's only the user-provided value, so I should rename it.

silently ignores a shared lib if no pkg config found, when --shared-* is set, without --shared-*-libname

It's not that it ignores it, the lib is still marked as shared, but it does not include any -l flag, so the compilation will fail, like it did in this case. I've opened nodejs/uvwasi#326 to fix the uvwasi pkg-config file, and https://boringssl-review.googlesource.com/c/boringssl/+/104208 for the BoringSSL one.

Should it infer the default_libs from the lib name?

I'd be fine with that, that would give some limited support for folks who are not using pkg-config and don't want to specify the libname manually.

legendecas · 2026-10-16T19:17:19Z

that would give some limited support for folks who are not using pkg-config and don't want to specify the libname manually.

Not sure how many people are doing this. I agree that a missing libname will eventually fail the build as the symbols are missing. This is merely an early error at configure time.

github-actions · 2026-10-18T15:02:50Z

Platform	Number of requisites	Proportion of new derivations
`x86_64-linux`	510 -> 513 => +3	3 / 513 = 0.5%
`aarch64-linux`	511 -> 514 => +3	3 / 514 = 0.5%
`x86_64-darwin`	443 -> 444 => +1	1 / 444 = 0.2%
`aarch64-darwin`	454 -> 457 => +3	3 / 457 = 0.6%
Total	1918 -> 1928 => +10	10 / 1928 = 0.5%

Changelog

@@ -101,0 +102 @@
+/nix/store/9qnmqyy3a96cxixkh5012vmdx7mmj8mn-boringssl-0.20260929.0 (aarch64-darwin)
@@ -102,0 +104,2 @@
+/nix/store/dqa82pf6zpbya2q5j5y27vqqrw79433a-boringssl-0.20260929.0 (aarch64-linux)
+/nix/store/50qpspny60wkd3n1s34mjj9hphp8lznj-boringssl-0.20260929.0 (x86_64-linux)
@@ -1834,0 +1838,2 @@
+/nix/store/zydhlwjc2p55kl6hih5x53qd3szlhpf5-uvwasi-0.0.23 (aarch64-darwin)
+/nix/store/mw2mpw871q1q7nf9pk297d5xm1yrp6xh-uvwasi-0.0.23 (aarch64-linux)
@@ -1836,0 +1842,2 @@
+/nix/store/y7qs50d944b2nvrpaqb94fjjqgdnfajs-uvwasi-0.0.23 (x86_64-darwin)
+/nix/store/3b8g90p5bixqz362w8f3bpmd62x43kax-uvwasi-0.0.23 (x86_64-linux)
@@ -1838,0 +1846 @@
+/nix/store/ms2rg0ykrrinrzf038lw8hhbi18hp9fw-uvwasi-0.0.23-dev (aarch64-darwin)
@@ -1839,0 +1848,2 @@
+/nix/store/qy9ya2sfzpzax4nvg7wdzx4rdy6dxdhs-uvwasi-0.0.23-dev (aarch64-linux)
+/nix/store/35hd49lx81ynfgq9438nkbhisvrsq4zk-uvwasi-0.0.23-dev (x86_64-linux)

codecov · 2026-10-18T16:35:46Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.97%. Comparing base (796a83a) to head (faa0590).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66790      +/-   ##
==========================================
+ Coverage   90.59%   92.97%   +2.37%     
==========================================
  Files         797      428     -369     
  Lines      278362   195171   -83191     
  Branches    53505    30465   -23040     
==========================================
- Hits       252195   181460   -70735     
+ Misses      16521    13385    -3136     
+ Partials     9646      326    -9320

see 503 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

aduh95 · 2026-10-19T07:53:52Z

I've taken the last commit to a separate PR, IMO it makes more sense to test (and maybe backport it) separately: #66844

By setting a default value, we override the pkg-config value, which is inconsistent with the other `--shared-*` flags. Signed-off-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

nodejs-github-bot · 2026-10-21T08:17:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78540/

github-actions · 2026-10-21T09:15:11Z

Platform	Number of requisites	Proportion of new derivations
`x86_64-linux`	541 -> 541 => no changes	2 / 541 = 0.3%
`aarch64-linux`	542 -> 542 => no changes	2 / 542 = 0.3%
`x86_64-darwin`	454 -> 454 => no changes	1 / 454 = 0.2%
`aarch64-darwin`	467 -> 467 => no changes	2 / 467 = 0.4%
Total	2004 -> 2004 => no changes	7 / 2004 = 0.3%

Changelog

@@ -1909 +1908,0 @@
-/nix/store/9fzzb199bajfljyb67lwydpgnj0sz6js-uvwasi-0.0.23 (aarch64-darwin)
@@ -1911 +1910,2 @@
-/nix/store/8xhg363fjq3sanag6dbrw0q3a8ldgl2x-uvwasi-0.0.23 (aarch64-linux)
+/nix/store/zydhlwjc2p55kl6hih5x53qd3szlhpf5-uvwasi-0.0.23 (aarch64-darwin)
+/nix/store/mw2mpw871q1q7nf9pk297d5xm1yrp6xh-uvwasi-0.0.23 (aarch64-linux)
@@ -1913 +1912,0 @@
-/nix/store/5ry3inlv2zz640a8wgpg6mxx67gr1mv8-uvwasi-0.0.23 (x86_64-darwin)
@@ -1914,0 +1914,2 @@
+/nix/store/y7qs50d944b2nvrpaqb94fjjqgdnfajs-uvwasi-0.0.23 (x86_64-darwin)
+/nix/store/3b8g90p5bixqz362w8f3bpmd62x43kax-uvwasi-0.0.23 (x86_64-linux)
@@ -1916,2 +1916,0 @@
-/nix/store/6hklmbp2y9gb9vd0dyhijk257f6iz3xy-uvwasi-0.0.23 (x86_64-linux)
-/nix/store/856zqnlnmvy2phamnjxqmi03dr15p7r7-uvwasi-0.0.23-dev (aarch64-darwin)
@@ -1919 +1918 @@
-/nix/store/acck6fyqrpxxdbc9chhcyjx3sxdmszcq-uvwasi-0.0.23-dev (aarch64-linux)
+/nix/store/ms2rg0ykrrinrzf038lw8hhbi18hp9fw-uvwasi-0.0.23-dev (aarch64-darwin)
@@ -1921 +1920,2 @@
-/nix/store/0dq16z78w91bcbx9rm0jfy57pjsrxasd-uvwasi-0.0.23-dev (x86_64-linux)
+/nix/store/qy9ya2sfzpzax4nvg7wdzx4rdy6dxdhs-uvwasi-0.0.23-dev (aarch64-linux)
+/nix/store/35hd49lx81ynfgq9438nkbhisvrsq4zk-uvwasi-0.0.23-dev (x86_64-linux)

nodejs-github-bot · 2026-10-21T14:08:58Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78546/

nodejs-github-bot · 2026-10-21T17:20:00Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78553/

github-actions · 2026-10-21T18:28:02Z

Platform	Number of requisites	Proportion of new derivations
`x86_64-linux`	544 -> 544 => no changes	2 / 544 = 0.3%
`aarch64-linux`	545 -> 545 => no changes	2 / 545 = 0.3%
`x86_64-darwin`	457 -> 457 => no changes	1 / 457 = 0.2%
`aarch64-darwin`	470 -> 470 => no changes	2 / 470 = 0.4%
Total	2016 -> 2016 => no changes	7 / 2016 = 0.3%

Changelog

@@ -1921 +1920,0 @@
-/nix/store/9fzzb199bajfljyb67lwydpgnj0sz6js-uvwasi-0.0.23 (aarch64-darwin)
@@ -1923 +1922,2 @@
-/nix/store/8xhg363fjq3sanag6dbrw0q3a8ldgl2x-uvwasi-0.0.23 (aarch64-linux)
+/nix/store/zydhlwjc2p55kl6hih5x53qd3szlhpf5-uvwasi-0.0.23 (aarch64-darwin)
+/nix/store/mw2mpw871q1q7nf9pk297d5xm1yrp6xh-uvwasi-0.0.23 (aarch64-linux)
@@ -1925 +1924,0 @@
-/nix/store/5ry3inlv2zz640a8wgpg6mxx67gr1mv8-uvwasi-0.0.23 (x86_64-darwin)
@@ -1926,0 +1926,2 @@
+/nix/store/y7qs50d944b2nvrpaqb94fjjqgdnfajs-uvwasi-0.0.23 (x86_64-darwin)
+/nix/store/3b8g90p5bixqz362w8f3bpmd62x43kax-uvwasi-0.0.23 (x86_64-linux)
@@ -1928,2 +1928,0 @@
-/nix/store/6hklmbp2y9gb9vd0dyhijk257f6iz3xy-uvwasi-0.0.23 (x86_64-linux)
-/nix/store/856zqnlnmvy2phamnjxqmi03dr15p7r7-uvwasi-0.0.23-dev (aarch64-darwin)
@@ -1931 +1930 @@
-/nix/store/acck6fyqrpxxdbc9chhcyjx3sxdmszcq-uvwasi-0.0.23-dev (aarch64-linux)
+/nix/store/ms2rg0ykrrinrzf038lw8hhbi18hp9fw-uvwasi-0.0.23-dev (aarch64-darwin)
@@ -1933 +1932,2 @@
-/nix/store/0dq16z78w91bcbx9rm0jfy57pjsrxasd-uvwasi-0.0.23-dev (x86_64-linux)
+/nix/store/qy9ya2sfzpzax4nvg7wdzx4rdy6dxdhs-uvwasi-0.0.23-dev (aarch64-linux)
+/nix/store/35hd49lx81ynfgq9438nkbhisvrsq4zk-uvwasi-0.0.23-dev (x86_64-linux)

nodejs-github-bot · 2026-10-21T18:34:51Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78555/

nodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. labels Oct 16, 2026

aduh95 force-pushed the no-default-libname branch from d2c7626 to f205c37 Compare October 16, 2026 13:52

This was referenced Oct 16, 2026

build: load gtest_main using pkg-config #66675

Merged

build: warn on incompatible quic configure flags #66754

Merged

legendecas reviewed Oct 16, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

cjihrig approved these changes Oct 17, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

build: remove default values for libname

faa0590

By setting a default value, we override the pkg-config value, which is inconsistent with the other `--shared-*` flags. Signed-off-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

aduh95 force-pushed the no-default-libname branch from c927bf4 to faa0590 Compare October 21, 2026 08:14

legendecas approved these changes Oct 21, 2026

View reviewed changes

aduh95 closed this Oct 21, 2026

aduh95 reopened this Oct 21, 2026

	default_libs = getattr(options, shared_lib + '_libname')

	if default_libs:
	libraries += [f'-l{l}' for l in default_libs.split(',')]
	elif pkg_libs:
	libraries += pkg_libs.split()

Conversation

aduh95 commented Oct 16, 2026

Uh oh!

legendecas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

legendecas commented Oct 16, 2026

Uh oh!

aduh95 commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

legendecas commented Oct 16, 2026

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

github-actions Bot commented Oct 18, 2026

Uh oh!

codecov Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aduh95 commented Oct 19, 2026

Uh oh!

nodejs-github-bot commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

nodejs-github-bot commented Oct 21, 2026

Uh oh!

nodejs-github-bot commented Oct 21, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

nodejs-github-bot commented Oct 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

legendecas left a comment •

edited

Loading

aduh95 commented Oct 16, 2026 •

edited

Loading

codecov Bot commented Oct 18, 2026 •

edited

Loading