Skip to content

src: remove HAVE_QUIC macro in favor of OPENSSL_NO_QUIC - #66373

Open
aduh95 wants to merge 2 commits into
nodejs:mainfrom
aduh95:fix-quic-detection
Open

aduh95 wants to merge 2 commits into
nodejs:mainfrom
aduh95:fix-quic-detection

Conversation

@aduh95

@aduh95 aduh95 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

We use both OPENSSL_NO_QUIC and HAVE_QUIC to gate QUIC code in C++, getting rid of the latter seems to make the most sense

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Sep 28, 2026
@panva

This comment was marked as outdated.

@aduh95

This comment was marked as outdated.

Signed-off-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@aduh95 aduh95 changed the title src: fix ngtcp2 and nghttp3 detection src: remove HAVE_QUIC macro in favor of OPENSSL_NO_QUIC Sep 28, 2026

@jasnell jasnell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, nice. I'd been meaning to clean that up for ages but managed to keep forgetting.

@codecov

codecov Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.36%. Comparing base (cff7b12) to head (87b289f).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66373      +/-   ##
==========================================
- Coverage   90.37%   90.36%   -0.01%     
==========================================
  Files         792      792              
  Lines      275426   275426              
  Branches    52800    52776      -24     
==========================================
- Hits       248926   248899      -27     
- Misses      16931    16944      +13     
- Partials     9569     9583      +14     
Files with missing lines Coverage Δ
src/node_metadata.cc 92.85% <ø> (ø)

... and 28 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.

@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Sep 28, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 28, 2026
@nodejs-github-bot

This comment has been minimized.

@panva panva added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Sep 28, 2026
@github-actions github-actions Bot added resume-ci-failed Resuming CI with the resume-ci label failed and requires manual intervention. and removed resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. labels Sep 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Failed to resume CI

✖  Cannot resume PR CI job 78036: Jenkins does not offer a "Resume build" action. Check the existing CI run in Jenkins and rebase the PR if needed. To start a new CI run manually: ncu-ci run https://github.com/nodejs/node/pull/66373
   ✖  https://ci.nodejs.org/job/node-test-pull-request/78036/
Full Auto Start CI output
�[36m⠋�[39m Validating Jenkins credentials
�[36m⠋�[39m Validating Jenkins credentials
✔  Jenkins credentials valid
�[36m⠙�[39m Looking for CI runs for pull request 66373
�[36m⠙�[39m Looking for CI runs for pull request 66373
�[36m⠙�[39m Getting PR from nodejs/node/pull/66373
�[36m⠙�[39m Getting reviews from nodejs/node/pull/66373
�[36m⠙�[39m Getting comments from nodejs/node/pull/66373
✔  Found PR CI job 78036
�[36m⠹�[39m Querying data for job/node-test-pull-request/78036/
�[36m⠹�[39m Querying data for job/node-test-pull-request/78036/
�[36m⠹�[39m Querying API for job/node-test-pull-request/78036/
✔  Build data downloaded
�[36m⠹�[39m Checking whether PR CI job 78036 can be resumed
�[36m⠹�[39m Checking whether PR CI job 78036 can be resumed
✖  Cannot resume PR CI job 78036: Jenkins does not offer a "Resume build" action. Check the existing CI run in Jenkins and rebase the PR if needed. To start a new CI run manually: ncu-ci run https://github.com/nodejs/node/pull/66373
   ✖  https://ci.nodejs.org/job/node-test-pull-request/78036/

View workflow run

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva removed the resume-ci-failed Resuming CI with the resume-ci label failed and requires manual intervention. label Sep 28, 2026
@aduh95 aduh95 added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Sep 28, 2026
@github-actions github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Sep 28, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@aduh95 aduh95 added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue PRs queued for automated landing through the Commit Queue. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants