-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
dns: Update to c-ares 1.17.1 #36207
dns: Update to c-ares 1.17.1 #36207
Conversation
Review requested:
|
b6ef1e1
to
f8de31a
Compare
deps/cares/src/ares_process.c
Outdated
#ifndef T_OPT | ||
# define T_OPT 41 /* EDNS0 option (meta-RR) */ | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this PR is landed, should this be landed as a separate commit that can be floated as a patch for subsequent updates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This piece of code is used in other places of c-ares, too. I'm going to provide a pull request for it in c-ares. Don't know how this should be handled in nodejs.
@richardlau I can't see, how these failing tests relate to this pull request:
Help would be very welcome to track this down, if this is related to this pull request. |
@nodejs/dns |
CI Internet tests: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/17846/ |
Can you explain the process you did to update this? (Maybe there's a doc somewhere I should be following along with?) I would have expected this to update deps/cares/include/ares_version.h but that file is unchanged. |
916e38b
to
a4902f0
Compare
@Trott I've did the following:
|
@nodejs/tsc Any ideas for domain experts who can review this? I can review it, but I wouldn't feel great about it if there wasn't someone more knowledgable giving it another pair of eyes. |
Note for whoever lands this: Subsystem should probably be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
PR-URL: nodejs#36207 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Rich Trott <[email protected]>
a4902f0
to
3bd9b81
Compare
Landed in 3bd9b81 |
PR-URL: #36207 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #36207 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #36207 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #36207 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #36207 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #36207 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Rich Trott <[email protected]>
c-ares refactored their source tree in 1.17.0 which we did not apply in our update to 1.17.1. This commit syncs our source with their new structure for easier maintenance going forward. Refs: c-ares/c-ares#349 Refs: nodejs#36207
c-ares refactored their source tree in 1.17.0 which we did not apply in our update to 1.17.1. This commit syncs our source with their new structure for easier maintenance going forward. Refs: c-ares/c-ares#349 Refs: nodejs#36207
c-ares refactored their source tree in 1.17.0 which we did not apply in our update to 1.17.1. This commit syncs our source with their new structure for easier maintenance going forward. cares.gyp is updated accordingly. Refs: c-ares/c-ares#349 Refs: nodejs#36207
PR-URL: #39653 Refs: c-ares/c-ares#349 Refs: #36207 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
c-ares refactored their source tree in 1.17.0 which we did not apply in our update to 1.17.1. This commit syncs our source with their new structure for easier maintenance going forward. cares.gyp is updated accordingly. Refs: c-ares/c-ares#349 Refs: #36207 PR-URL: #39653 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#39653 Refs: c-ares/c-ares#349 Refs: nodejs#36207 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
c-ares refactored their source tree in 1.17.0 which we did not apply in our update to 1.17.1. This commit syncs our source with their new structure for easier maintenance going forward. cares.gyp is updated accordingly. Refs: c-ares/c-ares#349 Refs: nodejs#36207 PR-URL: nodejs#39653 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
make -j4 test
(UNIX), orvcbuild test
(Windows) passes