Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: fix warnings from uv for gn build #51069

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

zcbenz
Copy link
Contributor

@zcbenz zcbenz commented Dec 6, 2023

Fix build failure in GN build caused by upgrade of libuv in #50650, a fast track merge would be appreciated so we can bring CI of GN build to green to continue monitoring the health of GN build .

../../node/deps/uv/src/idna.c:385:30: error: parameter 'w_target_len' set but not used [-Werror,-Wunused-but-set-parameter]
  385 |                       size_t w_target_len) {
      |                              ^
1 error generated.

Note that this parameter was intentionally used by libuv for safety check without being "used", so fixing it in libuv would be tricky as we need to tell the compiler the parameter is used.
https://github.com/libuv/libuv/blob/5e302730cd29cfeb15d32369dd2edfd9d3c82c11/src/idna.c#L383-L404

I'll try to fix it (and some other compiler warnings) in libuv but it will be really helpful to merge this first.

@nodejs-github-bot nodejs-github-bot added libuv Issues and PRs related to the libuv dependency or the uv binding. needs-ci PRs that need a full CI run. labels Dec 6, 2023
@zcbenz
Copy link
Contributor Author

zcbenz commented Dec 6, 2023

/cc @nodejs/gyp @richardlau

@gengjiawen gengjiawen added fast-track PRs that do not need to wait for 48 hours to land. request-ci Add this label to start a Jenkins CI on a PR. labels Dec 6, 2023
Copy link
Contributor

github-actions bot commented Dec 6, 2023

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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 6, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 6, 2023
@nodejs-github-bot nodejs-github-bot merged commit 3f4ea7a into nodejs:main Dec 6, 2023
61 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 3f4ea7a

RafaelGSS pushed a commit that referenced this pull request Dec 15, 2023
PR-URL: #51069
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Dec 15, 2023
richardlau pushed a commit that referenced this pull request Mar 25, 2024
PR-URL: #51069
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track PRs that do not need to wait for 48 hours to land. libuv Issues and PRs related to the libuv dependency or the uv binding. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants