Skip to content

build: fix ffi dependency compilation#62731

Open
ShogunPanda wants to merge 4 commits intonodejs:mainfrom
ShogunPanda:ffi-compile
Open

build: fix ffi dependency compilation#62731
ShogunPanda wants to merge 4 commits intonodejs:mainfrom
ShogunPanda:ffi-compile

Conversation

@ShogunPanda
Copy link
Copy Markdown
Contributor

@ShogunPanda ShogunPanda commented Apr 14, 2026 •
edited
Loading

This fix a little build regression introduced in #62072.

@ShogunPanda
build: fix ffi dependency compilation
3418d7e 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 14, 2026

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Apr 14, 2026
@ShogunPanda ShogunPanda added the fast-track PRs that do not need to wait for 72 hours to land. label Apr 14, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 14, 2026

Fast-track has been requested by @ShogunPanda. Please 👍 to approve.

@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented Apr 14, 2026

I'm still getting an error when building with --shared-ffi on macOS:

../../src/node_metadata.cc:176:12: error: use of undeclared identifier 'FFI_VERSION_STRING'
  176 |   libffi = FFI_VERSION_STRING;
      |            ^~~~~~~~~~~~~~~~~~

@ShogunPanda
Copy link
Copy Markdown
Contributor Author

ShogunPanda commented Apr 14, 2026

@aduh95 That might happen if you have a local libffi lower than 3.5.x. Can you please verify that?

@ShogunPanda
fixup
8305199 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@aduh95 aduh95 mentioned this pull request Apr 14, 2026
Merged
@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented Apr 14, 2026

@aduh95 That might happen if you have a local libffi lower than 3.5.x. Can you please verify that?

Yeah we have it set to an Apple fork that's on version 3.4.1-rc for some reason. I've opened #62737 to fix that, I've confirmed locally it does build on top of this PR.

mcollina
mcollina approved these changes Apr 14, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 14, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 14, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 14, 2026

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

@ShogunPanda
fixup
9467757 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@ShogunPanda
fixup
c68e4f7 
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes

@bengl bengl bengl approved these changes

@panva panva panva approved these changes

@aduh95 aduh95 aduh95 approved these changes

@atlowChemi atlowChemi atlowChemi approved these changes

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. fast-track PRs that do not need to wait for 72 hours to land. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ShogunPanda @nodejs-github-bot @aduh95 @mcollina @bengl @panva @atlowChemi