Skip to content

Update libuv#321

Merged
aduh95 merged 3 commits into
nodejs:mainfrom
vshymanskyy:master
Jul 2, 2026
Merged

Update libuv#321
aduh95 merged 3 commits into
nodejs:mainfrom
vshymanskyy:master

Conversation

@vshymanskyy

@vshymanskyy vshymanskyy commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

The change fixes this issue:

CMake Error at _deps/libuv-src/CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 3.5 has been removed from CMake.

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should 3 or 4 PRs, it's very hard to review as is. Can you please split that?

@vshymanskyy

Copy link
Copy Markdown
Contributor Author

@aduh95 cleaned up, this now only represents the libuv version update.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this land separately? It doesn't seem related with the libuv update

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, this is required for the newer libuv to build on CI

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we know why?

@vshymanskyy vshymanskyy Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, libuv switched to using getifaddrs() that is only available on android-24 and up:
libuv/libuv@c8cbdbd

For reference, here's the failed Github Action without this change:
https://github.com/vshymanskyy/uvwasi/actions/runs/28289880098/job/83819965569

Comment thread .clusterfuzzlite/build.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this land separately? It doesn't seem related with the libuv update

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, this is required for the newer libuv to build on CI

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we know why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the fuzzer build does not use CMake, and the patch to the static lib was hardcoded in the build.sh.
The path of the libuv.a changed, and without this fix the final binary won't link, see:
https://github.com/nodejs/uvwasi/actions/runs/28291462616/job/83824055824

Comment thread CMakeLists.txt
@vshymanskyy

Copy link
Copy Markdown
Contributor Author

@aduh95 all these changes are relevant, otherwise the CI checks will fail.

@vshymanskyy vshymanskyy requested a review from aduh95 June 30, 2026 09:36
@aduh95 aduh95 merged commit b6fbc1f into nodejs:main Jul 2, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants