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

dns: Update to c-ares 1.17.1 #36207

Merged
merged 1 commit into from Dec 16, 2020
Merged

Conversation

lxdicted
Copy link
Contributor

@lxdicted lxdicted commented Nov 21, 2020

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added the cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. label Nov 21, 2020
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 21, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 21, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@lxdicted lxdicted mentioned this pull request Nov 24, 2020
4 tasks
@nodejs-github-bot
Copy link
Collaborator

Comment on lines 63 to 65
#ifndef T_OPT
# define T_OPT 41 /* EDNS0 option (meta-RR) */
#endif
Copy link
Member

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?

Copy link
Contributor Author

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.

@lxdicted
Copy link
Contributor Author

@richardlau I can't see, how these failing tests relate to this pull request:

12:24:29 not ok 2497 parallel/test-webcrypto-derivebits-hkdf
12:24:29   ---
12:24:29   duration_ms: 122.197
12:24:29   severity: fail
12:24:29   exitcode: -15
12:24:29   stack: |-
12:24:29     timeout
12:24:29   ...
12:24:29 not ok 2498 parallel/test-webcrypto-derivebits-node-dh
12:24:29   ---
12:24:29   duration_ms: 121.988
12:24:29   severity: fail
12:24:29   exitcode: -15
12:24:29   stack: |-
12:24:29     timeout
12:24:29   ...
12:25:44 not ok 2499 parallel/test-webcrypto-derivebits-pbkdf2
12:25:50   ---
12:26:05   duration_ms: 180.679
12:26:05   severity: fail
12:26:05   exitcode: -15
12:26:05   stack: |-
12:26:05     timeout
12:26:05   ...

Help would be very welcome to track this down, if this is related to this pull request.

@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Nov 30, 2020

@nodejs/dns

@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Dec 2, 2020

@Trott
Copy link
Member

Trott commented Dec 2, 2020

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.

@lxdicted
Copy link
Contributor Author

lxdicted commented Dec 6, 2020

@Trott I've did the following:

  1. downloaded the old integrated version (1.16.1) and diffed its content against node's deps/cares
  2. downloaded the current version (1.17.1), and
  3. copied all files of it into deps/cares/{src,include}.
  4. verified, that the changes only in node (from 1.), are not removed with my commit.

@Trott Trott added the dns Issues and PRs related to the dns subsystem. label Dec 9, 2020
@Trott
Copy link
Member

Trott commented Dec 9, 2020

@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.

@Trott
Copy link
Member

Trott commented Dec 9, 2020

Note for whoever lands this: Subsystem should probably be deps.

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 9, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 9, 2020
@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Dec 12, 2020

Copy link
Member

@mcollina mcollina left a 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 <matteo.collina@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott Trott merged commit 3bd9b81 into nodejs:master Dec 16, 2020
@Trott
Copy link
Member

Trott commented Dec 16, 2020

Landed in 3bd9b81

targos pushed a commit that referenced this pull request Dec 21, 2020
PR-URL: #36207
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request May 1, 2021
PR-URL: #36207
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
codebytere added a commit to electron/electron that referenced this pull request May 14, 2021
codebytere added a commit to electron/electron that referenced this pull request May 18, 2021
richardlau pushed a commit that referenced this pull request Jul 26, 2021
PR-URL: #36207
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 26, 2021
PR-URL: #36207
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 26, 2021
PR-URL: #36207
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@richardlau richardlau mentioned this pull request Jul 26, 2021
richardlau pushed a commit that referenced this pull request Jul 26, 2021
PR-URL: #36207
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BethGriggs added a commit to BethGriggs/node that referenced this pull request Aug 3, 2021
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
BethGriggs added a commit to BethGriggs/node that referenced this pull request Aug 4, 2021
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
BethGriggs added a commit to BethGriggs/node that referenced this pull request Aug 5, 2021
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
nodejs-github-bot pushed a commit that referenced this pull request Aug 6, 2021
PR-URL: #39653
Refs: c-ares/c-ares#349
Refs: #36207
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Aug 6, 2021
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 <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#39653
Refs: c-ares/c-ares#349
Refs: nodejs#36207
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
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 <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. dns Issues and PRs related to the dns subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants