Skip to content

asyn-ares: bump required c-ares version to >= 1.16.0#20911

Closed
bagder wants to merge 1 commit intomasterfrom
bagder/rm-old-c-ares
Closed

asyn-ares: bump required c-ares version to >= 1.16.0#20911
bagder wants to merge 1 commit intomasterfrom
bagder/rm-old-c-ares

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Mar 12, 2026

No description provided.

@bagder bagder added feature-window A merge of this requires an open feature window name lookup DNS and related tech labels Mar 12, 2026
@bagder bagder marked this pull request as ready for review March 12, 2026 16:13
@bagder bagder requested a review from Copilot March 12, 2026 16:14
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR raises the minimum supported c-ares version for the async resolver backend to >= 1.16.0, allowing removal of legacy compatibility paths and simplifying the resolver implementation accordingly.

Changes:

  • Enforce c-ares >= 1.16.0 in the c-ares resolver backend and remove the pre-1.16.0 code paths (including the non-ares_getaddrinfo() flow).
  • Simplify c-ares feature gating by removing several version-dependent HAVE_CARES_* compile-time branches that are now always true under the new minimum.
  • Update internal documentation to reflect the new minimum supported c-ares version.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
lib/asyn-ares.c Enforces the new minimum c-ares version and removes legacy resolver code paths/feature gates tied to older c-ares versions.
docs/INTERNALS.md Updates the documented minimum supported c-ares dependency version to 1.16.0.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread lib/asyn-ares.c Outdated
Comment thread docs/INTERNALS.md
@testclutch
Copy link
Copy Markdown

Analysis of PR #20911 at eaefb959:

Test 799 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 12 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (7, to be exact) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Generated by Testclutch

@github-actions github-actions bot added the CI Continuous Integration label Mar 12, 2026
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Mar 12, 2026

augment review

@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Mar 12, 2026

🤖 Augment PR Summary

Summary: This PR raises the minimum supported c-ares version for the async resolver backend to >= 1.16.0 and removes legacy compatibility paths.

Changes:

  • Update INTERNALS dependency matrix to list c-ares 1.16.0 as the baseline.
  • Enforce the new minimum in lib/asyn-ares.c and lib/asyn-base.c via compile-time checks.
  • Remove pre-1.16 resolver fallback code and rely on ares_getaddrinfo()-based resolution.
  • Simplify c-ares option handling (e.g., always using ares_set_servers_ports_csv() and local bind APIs under the new minimum).
  • Update Autotools detection to require ares_getaddrinfo and adjust the “linux-old” CI workflow to disable c-ares until a new enough package is available.

Technical notes: Builds using USE_HTTPSRR already require an even newer c-ares (1.28.0+), and this PR keeps that enforcement.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread .github/workflows/linux-old.yml Outdated
Comment thread m4/curl-confopts.m4
bagder added a commit that referenced this pull request Mar 21, 2026
@bagder bagder force-pushed the bagder/rm-old-c-ares branch from 3b727a5 to 8e21361 Compare March 21, 2026 13:23
@bagder bagder requested a review from Copilot March 21, 2026 13:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread m4/curl-confopts.m4
Comment thread .github/workflows/linux-old.yml
Comment thread lib/asyn-ares.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous Integration feature-window A merge of this requires an open feature window name lookup DNS and related tech

Development

Successfully merging this pull request may close these issues.

3 participants