node icon indicating copy to clipboard operation
node copied to clipboard
verified publisher icon nodejs

tools: add script to synch c-ares source lists

Open richardlau opened this issue 1 year ago • 3 comments

Add step to the updater script for c-ares to synchronize the list of sources in our gyp file with the lists in c-ares' Makefiles.

This should hopefully mean less manual fix ups (e.g. https://github.com/nodejs/node/pull/55369/commits/21c6853cc8a0c10f4abbda7f53ca2c4d3801615b) due to files in c-ares being added, removed or moved.

The second and third commits in this PR are to show the result of running the script on the current c-ares version. Some platform specific files are moved to "common", but they were not in platform specific sections in c-ares' Makefiles and appear to have the appropriate C preprocessor guards in them to prevent them affecting the other platforms (we'll find out in the CI runs 🙂).

There are three removed files from cares.gyp:

  • 'src/lib/str/ares__buf.h' -- this appears to have been overlooked when it moved in c-ares 1.34.1 (the new location is also in the gyp file but the old location wasn't removed).
  • 'src/tools/ares_getopt.c' and 'src/tools/ares_getopt.h -- these used to live in src/lib but were moved in a re-org (https://github.com/c-ares/c-ares/pull/349) in c-ares 1.17.0 and reflected in https://github.com/nodejs/node/pull/39653. However it doesn't look like anything in src/lib or Node.js source references these, so looks like we don't need to list them.

richardlau avatar Oct 18 '24 17:10 richardlau

Review requested:

  • [ ] @nodejs/gyp
  • [ ] @nodejs/net
  • [ ] @nodejs/security-wg

nodejs-github-bot avatar Oct 18 '24 17:10 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/63188/

nodejs-github-bot avatar Oct 18 '24 17:10 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/63190/

nodejs-github-bot avatar Oct 18 '24 17:10 nodejs-github-bot

Commit Queue failed
- Loading data for nodejs/node/pull/55445
✔  Done loading data for nodejs/node/pull/55445
----------------------------------- PR info ------------------------------------
Title      tools: add script to synch c-ares source lists (#55445)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     richardlau:update-c-ares -> nodejs:main
Labels     tools, cares, author ready, needs-ci, dependencies
Commits    3
 - tools: add script to synch c-ares source lists
 - build: synchronize list of c-ares source files
 - build: tidy up cares.gyp
Committers 1
 - Richard Lau <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/55445
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55445
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 18 Oct 2024 17:11:09 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/55445#pullrequestreview-2379242712
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/55445#pullrequestreview-2379505297
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-10-18T17:28:22Z: https://ci.nodejs.org/job/node-test-pull-request/63190/
- Querying data for job/node-test-pull-request/63190/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 55445
From https://github.com/nodejs/node
 * branch                  refs/pull/55445/merge -> FETCH_HEAD
✔  Fetched commits as b0ffe9ed3553..973df1038d70
--------------------------------------------------------------------------------
[main 0d3b9ccfac] tools: add script to synch c-ares source lists
 Author: Richard Lau <[email protected]>
 Date: Fri Oct 18 16:04:25 2024 +0000
 2 files changed, 41 insertions(+)
 create mode 100644 tools/dep_updaters/update-c-ares.mjs
[main 941b8ec092] build: synchronize list of c-ares source files
 Author: Richard Lau <[email protected]>
 Date: Fri Oct 18 16:47:05 2024 +0000
 1 file changed, 5 insertions(+), 3 deletions(-)
[main 8dd556d1b7] build: tidy up cares.gyp
 Author: Richard Lau <[email protected]>
 Date: Fri Oct 18 16:52:00 2024 +0000
 1 file changed, 1 insertion(+), 10 deletions(-)
   ✔  Patches applied
There are 3 commits in the PR. Attempting autorebase.
Rebasing (2/6)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
tools: add script to synch c-ares source lists

Add step to the updater script for c-ares to synchronize the list
of sources in our gyp file with the lists in c-ares' Makefiles.


PR-URL: https://github.com/nodejs/node/pull/55445
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>


[detached HEAD 3fee2cfca7] tools: add script to synch c-ares source lists
Author: Richard Lau <[email protected]>
Date: Fri Oct 18 16:04:25 2024 +0000
2 files changed, 41 insertions(+)
create mode 100644 tools/dep_updaters/update-c-ares.mjs
Rebasing (3/6)
Rebasing (4/6)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
build: synchronize list of c-ares source files


Run the tools/dep_updaters/update-c-ares.mjs script to synchronize
the list of source files in our gyp file with the lists from c-ares'
Makefiles.


PR-URL: https://github.com/nodejs/node/pull/55445
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>


[detached HEAD b43ba9ef2f] build: synchronize list of c-ares source files
Author: Richard Lau <[email protected]>
Date: Fri Oct 18 16:47:05 2024 +0000
1 file changed, 5 insertions(+), 3 deletions(-)
Rebasing (5/6)
Rebasing (6/6)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
build: tidy up cares.gyp


Add comment noting that cares_sources_common is generated by tooling.
Remove duplicated entries.


PR-URL: https://github.com/nodejs/node/pull/55445
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>


[detached HEAD a857ed1d34] build: tidy up cares.gyp
Author: Richard Lau <[email protected]>
Date: Fri Oct 18 16:52:00 2024 +0000
1 file changed, 1 insertion(+), 10 deletions(-)
Successfully rebased and updated refs/heads/main.


ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.
https://github.com/nodejs/node/actions/runs/11428219308

nodejs-github-bot avatar Oct 20 '24 17:10 nodejs-github-bot

Landed in b0ffe9ed355313c2d7c2d655b3af8947b842c683...c124cfb4facb85ef861575f084e503c23aef963f

nodejs-github-bot avatar Oct 20 '24 18:10 nodejs-github-bot