build: do not pass vendored headers when building against shared libs

And setup `test-shared.yml` so we can detect regressions.

Signed-off-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66844
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

.github/workflows/test-shared.yml

@@ -199,7 +199,7 @@ jobs:
         id: v8-drv
         run: |
           V8_DRV=$(
-            nix-instantiate -E "builtins.filter (p: p.pname == ''v8'') (import $TAR_DIR/tools/nix/ci-shell.nix {}).buildInputs"
+            nix-instantiate -E "(import $TAR_DIR/tools/nix/ci-shell.nix {}).v8Derivation"
           )
           V8_STORE_PATH=$(nix-store -q --outputs "$V8_DRV")
           echo "V8_DRV=$V8_DRV" >> "$GITHUB_OUTPUT"

benchmark/napi/make_callback/binding.gyp

@@ -2,7 +2,8 @@
   'targets': [
     {
       'target_name': 'binding',
-      'sources': [ 'binding.cc' ]
+      'sources': [ 'binding.cc' ],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

common.gypi

@@ -26,8 +26,14 @@
     'node_module_version%': '',
     'node_with_ltcg%': '',
     'node_shared_openssl%': 'false',
+    'node_shared_openssl_include_dirs%': [],
     'openssl_is_boringssl%': 'false',
 
+    'node_shared_libuv_include_dirs%': [],
+    'node_shared_sqlite%': 'false',
+    'node_shared_sqlite_include_dirs%': [],
+    'node_shared_zlib_include_dirs%': [],
+
     'node_tag%': '',
     'uv_library%': 'static_library',
 

configure.py

@@ -2195,19 +2195,26 @@ def configure_napi(output):
   version = getnapibuildversion.get_napi_version()
   output['variables']['napi_build_version'] = version
 
-def configure_library(lib, output, pkgname=None, libraries_key='libraries'):
+def configure_library(lib, output, pkgname=None, libraries_key='libraries', with_separate_include_var=False):
   shared_lib = 'shared_' + lib
   output['variables']['node_' + shared_lib] = b(getattr(options, shared_lib))
+
   libraries = output[libraries_key] if libraries_key in output else output['variables'][libraries_key]
 
+  include_dirs = []
+  if with_separate_include_var:
+    output['variables'][f'node_{shared_lib}_include_dirs'] = include_dirs
+
   if getattr(options, shared_lib):
     (pkg_libs, pkg_cflags, pkg_libpath, _) = pkg_config(pkgname or lib)
 
     if options.__dict__[shared_lib + '_includes']:
-      output['include_dirs'] += [options.__dict__[shared_lib + '_includes']]
+      include_dirs += [options.__dict__[shared_lib + '_includes']]
     elif pkg_cflags:
       stripped_flags = [flag.strip() for flag in pkg_cflags.split('-I')]
-      output['include_dirs'] += [flag for flag in stripped_flags if flag]
+      include_dirs += [flag for flag in stripped_flags if flag]
+
+    output['include_dirs'] += include_dirs
 
     # libpath needs to be provided ahead libraries
     if options.__dict__[shared_lib + '_libpath']:
@@ -2376,7 +2383,7 @@ def configure_openssl(o):
   if options.openssl_is_fips and not options.shared_openssl:
     variables['node_fipsinstall'] = b(True)
 
-  configure_library('openssl', o)
+  configure_library('openssl', o, with_separate_include_var=True)
 
   o['variables']['openssl_version'] = get_openssl_version(o)
   o['variables']['openssl_is_boringssl'] = get_openssl_is_boringssl(o)
@@ -2407,7 +2414,7 @@ def configure_sqlite(o):
       without_sqlite_error('--shared-sqlite')
     return
 
-  configure_library('sqlite', o, pkgname='sqlite3')
+  configure_library('sqlite', o, pkgname='sqlite3', with_separate_include_var=True)
 
 def bundled_ffi_supported(os_name, target_arch):
   if target_arch == 'x86':
@@ -2905,9 +2912,9 @@ configure_node(output)
 configure_node_lib_files(output)
 configure_node_cctest_sources(output)
 configure_napi(output)
-configure_library('zlib', output)
+configure_library('zlib', output, with_separate_include_var=True)
 configure_library('http_parser', output, pkgname='libllhttp')
-configure_library('libuv', output)
+configure_library('libuv', output, with_separate_include_var=True)
 configure_library('abseil', output, pkgname=[
   'absl_absl_check',
   'absl_absl_log',

shell.nix

@@ -88,6 +88,20 @@ let
   ]
   ++ builtins.map (depName: "--shared-${depName}") sharedV8Deps
   ++ pkgs.lib.optional withPerfetto "--with-perfetto";
+
+  v8Derivation =
+    if useSeparateDerivationForV8 == true then
+      let
+        sharedLibsToMock = pkgs.callPackage ./tools/nix/non-v8-deps-mock.nix { };
+      in
+      pkgs.callPackage ./tools/nix/v8.nix {
+        inherit nativeBuildInputs icu;
+
+        configureFlags = configureFlags ++ sharedLibsToMock.configureFlags ++ [ "--ninja" ];
+        buildInputs = buildInputs ++ [ sharedLibsToMock ];
+      }
+    else
+      useSeparateDerivationForV8;
 in
 pkgs.mkShell {
   inherit nativeBuildInputs;
@@ -98,20 +112,7 @@ pkgs.mkShell {
   buildInputs =
     builtins.attrValues sharedLibDeps
     ++ buildInputs
-    ++ pkgs.lib.optional (useSeparateDerivationForV8 != false) (
-      if useSeparateDerivationForV8 == true then
-        let
-          sharedLibsToMock = pkgs.callPackage ./tools/nix/non-v8-deps-mock.nix { };
-        in
-        pkgs.callPackage ./tools/nix/v8.nix {
-          inherit nativeBuildInputs icu;
-
-          configureFlags = configureFlags ++ sharedLibsToMock.configureFlags ++ [ "--ninja" ];
-          buildInputs = buildInputs ++ [ sharedLibsToMock ];
-        }
-      else
-        useSeparateDerivationForV8
-    );
+    ++ pkgs.lib.optional (useSeparateDerivationForV8 != false) v8Derivation;
 
   packages = devTools ++ benchmarkTools ++ pkgs.lib.optional (ccache != null) ccache;
 
@@ -120,6 +121,10 @@ pkgs.mkShell {
     export CXX="${pkgs.lib.getExe ccache} $CXX"
   '';
 
+  passthru = {
+    inherit v8Derivation;
+  };
+
   env = {
     BUILD_WITH = if (ninja != null) then "ninja" else "make";
     NINJA = pkgs.lib.optionalString (ninja != null) "${pkgs.lib.getExe ninja}";

test/addons/async-cleanup-hook/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

test/addons/async-hello-world/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

test/addons/callback-scope/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': ['<@(node_shared_libuv_include_dirs)'],
     }
   ]
 }

test/addons/esm-export-default/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding-export-default',
       'sources': [ 'binding-export-default.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     },
   ]
 }

test/addons/esm-export-primitive/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding-export-primitive',
       'sources': [ 'binding-export-primitive.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     },
   ]
 }

test/addons/esm/binding.gyp

@@ -7,6 +7,7 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     },
     {
       'target_name': 'node_modules',

test/addons/hello-world-2/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     },
   ]
 }

test/addons/openssl-binding/binding.gyp

@@ -5,6 +5,7 @@
       'includes': ['../common.gypi'],
       'conditions': [
         ['node_use_openssl=="true"', {
+          'include_dirs': ['<@(node_shared_openssl_include_dirs)'],
           'conditions': [
             ['OS in "aix os400"', {
               'variables': {
@@ -17,12 +18,10 @@
                 }],
                 [ '"<(aix_variant_name)"!="OS400"', { # Not `OS400`(IBM i)
                   'sources': ['binding.cc'],
-                  'include_dirs': ['../../../deps/openssl/openssl/include'],
                 }],
               ],
             }, {
               'sources': ['binding.cc'],
-              'include_dirs': ['../../../deps/openssl/openssl/include'],
             }],
           ],
         }],

test/addons/openssl-get-ssl-ctx/binding.gyp

@@ -5,6 +5,7 @@
       'includes': ['../common.gypi'],
       'conditions': [
         ['node_use_openssl=="true"', {
+          'include_dirs': ['<@(node_shared_openssl_include_dirs)'],
           'conditions': [
             ['OS in "aix os400"', {
               'variables': {
@@ -17,12 +18,10 @@
                 }],
                 [ '"<(aix_variant_name)"!="OS400"', { # Not `OS400`(IBM i)
                   'sources': ['binding.cc'],
-                  'include_dirs': ['../../../deps/openssl/openssl/include'],
                 }],
               ],
             }, {
               'sources': ['binding.cc'],
-              'include_dirs': ['../../../deps/openssl/openssl/include'],
             }],
           ],
         }],

test/addons/openssl-providers/binding.gyp

@@ -5,6 +5,7 @@
       'includes': ['../common.gypi'],
       'conditions': [
         ['node_use_openssl=="true"', {
+          'include_dirs': ['<@(node_shared_openssl_include_dirs)'],
           'conditions': [
             ['OS in "aix os400"', {
               'variables': {
@@ -17,12 +18,10 @@
                 }],
                 [ '"<(aix_variant_name)"!="OS400"', { # Not `OS400`(IBM i)
                   'sources': ['binding.cc'],
-                  'include_dirs': ['../../../deps/openssl/openssl/include'],
                 }],
               ],
             }, {
               'sources': ['binding.cc'],
-              'include_dirs': ['../../../deps/openssl/openssl/include'],
             }],
           ],
         }],

test/addons/register-signal-handler/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

test/addons/uv-handle-leak/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

test/addons/uv-thread-name/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

test/addons/worker-addon/binding.gyp

@@ -4,6 +4,7 @@
       'target_name': 'binding',
       'sources': [ 'binding.cc' ],
       'includes': ['../common.gypi'],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

test/addons/zlib-binding/binding.gyp

@@ -14,15 +14,14 @@
             }],
             [ '"<(aix_variant_name)"!="OS400"', { # Not `OS400`(IBM i)
               'sources': ['binding.cc'],
-              'include_dirs': ['../../../deps/zlib'],
             }],
           ],
         }, {
           'sources': ['binding.cc'],
-          'include_dirs': ['../../../deps/zlib'],
         }],
       ],
       'includes': ['../common.gypi'],
+      'include_dirs': ['<@(node_shared_zlib_include_dirs)'],
     },
   ]
 }

test/node-api/test_async/binding.gyp

@@ -2,7 +2,8 @@
   "targets": [
     {
       "target_name": "test_async",
-      "sources": [ "test_async.c" ]
+      "sources": [ "test_async.c" ],
+      "include_dirs": [ "<@(node_shared_libuv_include_dirs)" ],
     }
   ]
 }

test/node-api/test_async_cleanup_hook/binding.gyp

@@ -3,7 +3,8 @@
     {
       'target_name': 'binding',
       'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
-      'sources': [ 'binding.c' ]
+      'sources': [ 'binding.c' ],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

test/node-api/test_callback_scope/binding.gyp

@@ -3,7 +3,8 @@
     {
       'target_name': 'binding',
       'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
-      'sources': [ 'binding.c' ]
+      'sources': [ 'binding.c' ],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

test/node-api/test_cleanup_hook/binding.gyp

@@ -3,7 +3,8 @@
     {
       'target_name': 'binding',
       'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
-      'sources': [ 'binding.c' ]
+      'sources': [ 'binding.c' ],
+      'include_dirs': [ '<@(node_shared_libuv_include_dirs)' ],
     }
   ]
 }

test/node-api/test_fatal/binding.gyp

@@ -2,7 +2,8 @@
   "targets": [
     {
       "target_name": "test_fatal",
-      "sources": [ "test_fatal.c" ]
+      "sources": [ "test_fatal.c" ],
+      "include_dirs": [ "<@(node_shared_libuv_include_dirs)" ],
     }
   ]
 }

test/node-api/test_instance_data/binding.gyp

@@ -4,7 +4,8 @@
       "target_name": "test_instance_data",
       "sources": [
         "test_instance_data.c"
-      ]
+      ],
+      "include_dirs": [ "<@(node_shared_libuv_include_dirs)" ],
     },
     {
       "target_name": "test_set_then_ref",

test/node-api/test_threadsafe_function/binding.gyp

@@ -2,7 +2,8 @@
   'targets': [
     {
       'target_name': 'binding',
-      'sources': ['binding.c']
+      'sources': ['binding.c'],
+      'include_dirs': ['<@(node_shared_libuv_include_dirs)'],
     },
     {
       'target_name': 'test_uncaught_exception_v9',

test/node-api/test_uv_loop/binding.gyp

@@ -2,7 +2,8 @@
   "targets": [
     {
       "target_name": "test_uv_loop",
-      "sources": [ "test_uv_loop.cc" ]
+      "sources": [ "test_uv_loop.cc" ],
+      "include_dirs": [ "<@(node_shared_libuv_include_dirs)" ],
     }
   ]
 }

test/node-api/test_uv_threadpool_size/binding.gyp

@@ -2,7 +2,8 @@
   "targets": [
     {
       "target_name": "test_uv_threadpool_size",
-      "sources": [ "test_uv_threadpool_size.c" ]
+      "sources": [ "test_uv_threadpool_size.c" ],
+      "include_dirs": [ "<@(node_shared_libuv_include_dirs)" ],
     }
   ]
 }

test/sqlite/test_sqlite_extensions/binding.gyp

@@ -4,7 +4,13 @@
       "target_name": "sqlite_extension",
       "type": "shared_library",
       "sources": [ "extension.c" ],
-      "include_dirs": [ "../../../deps/sqlite" ]
+      "conditions": [
+        ['node_shared_sqlite=="false"', {
+          "include_dirs": [ "../../../deps/sqlite" ],
+        }, {
+          "include_dirs": [ "<@(node_shared_sqlite_include_dirs)" ],
+        }],
+      ],
     }
   ]
 }

tools/nix/ci-shell.nix

@@ -2,7 +2,7 @@ args@{
   pkgs ? import ./pkgs.nix { },
   ...
 }:
-import ../../shell.nix (
+(import ../../shell.nix (
   {
     inherit pkgs;
     useSeparateDerivationForV8 = true;
@@ -26,4 +26,30 @@ import ../../shell.nix (
     }
   )
   // args
-)
+)).overrideAttrs
+  (old: {
+    # Make shared deps reachable only through pkg-config, so regressions aren't
+    # hidden by cc-wrapper's -isystem/-L for every buildInput.
+    buildInputs = map (
+      dep:
+      pkgs.runCommand
+        "${
+          dep.name or (
+            let
+              name = baseNameOf (toString dep);
+            in
+            if builtins.stringLength name > 35 then builtins.substring 33 (-1) name else name
+          )
+        }-pkg-config-only"
+        {
+          nativeBuildInputs = [ pkgs.pkgconf ];
+          buildInputs = [ dep ];
+        }
+        ''
+          mkdir -p "$out/lib/pkgconfig"
+          for pc in $(pkgconf --list-package-names); do
+            ln -s "$(pkgconf --path "$pc")" "$out/lib/pkgconfig/"
+          done
+        ''
+    ) old.buildInputs;
+  })

tools/nix/pkgs.nix

@@ -21,6 +21,23 @@ else
       openssl_3
       ;
 
+    # TODO: remove this when the pin we use has picked up https://boringssl-review.googlesource.com/c/boringssl/+/104208
+    boringssl =
+      let
+        inherit (nixpkgs) boringssl lib writeTextFile;
+      in
+      writeTextFile {
+        inherit (boringssl) name;
+        destination = "/lib/pkgconfig/openssl.pc";
+        text = ''
+          Name: BoringSSL
+          Description: BoringSSL cryptography library
+          Version: ${boringssl.version}
+          Libs: -L${lib.getLib boringssl}/lib -lcrypto -lssl
+          Cflags: -I${lib.getInclude boringssl}/include
+        '';
+      };
+
     # TODO: remove pin once https://github.com/mozilla/sccache/issues/2869 is resolved
     sccache = nixpkgs.callPackage (builtins.fetchurl {
       url = "${repo}/raw/aff8a0b28396750446e5537a96461bc4facdb287/pkgs/by-name/sc/sccache/package.nix";

tools/nix/sharedLibDeps.nix

@@ -19,7 +19,6 @@
     nghttp2
     simdjson
     simdutf
-    uvwasi
     zlib
     zstd
     ;
@@ -28,9 +27,30 @@
   hdr-histogram = pkgs.hdrhistogram_c;
   highway = pkgs.libhwy;
   http-parser = pkgs.llhttp;
+
+  # TODO: remove this override once the pin we use has picked up https://github.com/NixOS/nixpkgs/pull/575002
+  uvwasi = pkgs.uvwasi.overrideAttrs (old: {
+    patches = old.patches ++ [
+      (pkgs.fetchpatch2 {
+        url = "https://github.com/nodejs/uvwasi/pull/326.patch";
+        hash = "sha256-PPr62+qycCSfr1/JWXhJS6X8vW10ea2b4L4361tdwgE=";
+      })
+    ];
+
+    buildInputs = [ ];
+    propagatedBuildInputs = old.buildInputs;
+  });
 }
 // (pkgs.lib.optionalAttrs withLief {
-  inherit (pkgs) lief;
+  # TODO: remove this override once the pin we use has picked up a new LIEF version that includes that patch.
+  lief = pkgs.lief.overrideAttrs (old: {
+    patches = (old.patches or [ ]) ++ [
+      (pkgs.fetchpatch2 {
+        url = "https://github.com/lief-project/LIEF/commit/15777f106f6220046cf26ee99940d1f56751325e.patch?full_index=1";
+        hash = "sha256-Dm7YsBZ4+It+c5PEodsjq7cdOryYOSXt6Lf7h0OH+7M=";
+      })
+    ];
+  });
 })
 // (pkgs.lib.optionalAttrs withQuic {
   inherit (pkgs)