Skip to content

Conversation

BethGriggs
Copy link
Member

@BethGriggs BethGriggs commented Aug 10, 2021

PR was created by downloading content of https://github.com/c-ares/c-ares/releases/tag/cares-1_17_2, and copying modifications to the files we already had in our tree. nameser.h is renamed to ares_nameser.h.

@nodejs-github-bot nodejs-github-bot added cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. needs-ci PRs that need a full CI run. labels Aug 10, 2021
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 10, 2021

@BethGriggs
Copy link
Member Author

Coverage actions failure is unrelated (being discussed in #39725). Would appreciate another review or two.

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Rubber-stamp LGTM

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

@BethGriggs BethGriggs added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 10, 2021
BethGriggs added a commit that referenced this pull request Aug 10, 2021
@BethGriggs
Copy link
Member Author

Landed in 3914354

@BethGriggs BethGriggs closed this Aug 11, 2021
felixonmars added a commit to felixonmars/node that referenced this pull request Aug 11, 2021
The change in nodejs#39724 breaks building with system c-ares (`--shared-cares`):
```
In file included from ../src/cares_wrap.cc:25:
../src/cares_wrap.h:25:11: fatal error: ares_nameser.h: No such file or directory
   25 | # include <ares_nameser.h>
      |           ^~~~~~~~~~~~~~~~
```

Since `ares_nameser.h` isn't available with a default system c-ares installation, let's add back the include check and use the old `arpa/nameser.h` routine instead.

Tested to build fine on Arch Linux with shared c-ares.
felixonmars added a commit to felixonmars/node that referenced this pull request Aug 11, 2021
The change in nodejs#39724 breaks building with system c-ares (`--shared-cares`):
```
In file included from ../src/cares_wrap.cc:25:
../src/cares_wrap.h:25:11: fatal error: ares_nameser.h: No such file or directory
   25 | # include <ares_nameser.h>
      |           ^~~~~~~~~~~~~~~~
```

Since `ares_nameser.h` isn't available with a default system c-ares installation, let's add back the include check and use the old `arpa/nameser.h` routine instead.

Tested to build fine on Arch Linux with shared c-ares.
felixonmars added a commit to felixonmars/node that referenced this pull request Aug 11, 2021
The change in nodejs#39724 breaks building with system c-ares
(`--shared-cares`):
```
In file included from ../src/cares_wrap.cc:25:
../src/cares_wrap.h:25:11: fatal error: ares_nameser.h: No such file or
directory
   25 | # include <ares_nameser.h>
      |           ^~~~~~~~~~~~~~~~
```

Since `ares_nameser.h` isn't available with a default system c-ares
installation, let's add back the include check and use the old
`arpa/nameser.h` routine instead.

Tested to build fine on Arch Linux with shared c-ares.
felixonmars added a commit to felixonmars/node that referenced this pull request Aug 12, 2021
The change in nodejs#39724 breaks building with system c-ares
(`--shared-cares`):
```
In file included from ../src/cares_wrap.cc:25:
../src/cares_wrap.h:25:11: fatal error: ares_nameser.h: No such file or
directory
   25 | # include <ares_nameser.h>
      |           ^~~~~~~~~~~~~~~~
```

Since `ares_nameser.h` isn't available with a default system c-ares
installation, let's copy it as our private header here.

Tested to build fine on Arch Linux with shared c-ares.
codebytere added a commit to electron/electron that referenced this pull request Aug 16, 2021
codebytere added a commit to electron/electron that referenced this pull request Aug 18, 2021
codebytere added a commit to electron/electron that referenced this pull request Aug 20, 2021
* chore: bump node in DEPS to v16.6.0

* chore: bump node in DEPS to v16.6.1

* crypto: fix generateKeyPair with encoding 'jwk'

nodejs/node#39319

* build: add library_files to gyp variables

nodejs/node#39293

* debugger: rename internal module

nodejs/node#39378

* chore: fixup patch indices

* deps: extract gtest source files to deps/googletest

nodejs/node#39386

* crypto: fix generateKeyPair with encoding 'jwk'

nodejs/node#39319

* deps: bump HdrHistogram_C to 0.11.2

nodejs/node#39462

* fixup! deps: extract gtest source files to deps/googletest

* chore: bump node in DEPS to v16.6.2

* chore: update patches

* deps: reflect c-ares source tree

nodejs/node#39653

* deps: update c-ares to 1.17.2

nodejs/node#39724

* fix: _ReadBarrier undefined symbol error on WOA arm64

* chore: update patches

* chore: bump node in DEPS to v16.7.0

* deps: upgrade to libuv 1.42.0

nodejs/node#39525

* chore: update filenames

* src: remove extra semicolons outside fns

* chore: fixup patch filenames

* chore: sort and alphabetize disabled tests

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
BlackHole1 pushed a commit to BlackHole1/electron that referenced this pull request Aug 30, 2021
* chore: bump node in DEPS to v16.6.0

* chore: bump node in DEPS to v16.6.1

* crypto: fix generateKeyPair with encoding 'jwk'

nodejs/node#39319

* build: add library_files to gyp variables

nodejs/node#39293

* debugger: rename internal module

nodejs/node#39378

* chore: fixup patch indices

* deps: extract gtest source files to deps/googletest

nodejs/node#39386

* crypto: fix generateKeyPair with encoding 'jwk'

nodejs/node#39319

* deps: bump HdrHistogram_C to 0.11.2

nodejs/node#39462

* fixup! deps: extract gtest source files to deps/googletest

* chore: bump node in DEPS to v16.6.2

* chore: update patches

* deps: reflect c-ares source tree

nodejs/node#39653

* deps: update c-ares to 1.17.2

nodejs/node#39724

* fix: _ReadBarrier undefined symbol error on WOA arm64

* chore: update patches

* chore: bump node in DEPS to v16.7.0

* deps: upgrade to libuv 1.42.0

nodejs/node#39525

* chore: update filenames

* src: remove extra semicolons outside fns

* chore: fixup patch filenames

* chore: sort and alphabetize disabled tests

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
@BethGriggs BethGriggs deleted the c-ares-1.17.2 branch November 15, 2021 00:17
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants