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

tools: add script to synch c-ares source lists #55445

Closed
wants to merge 3 commits into from

Conversation

richardlau
Copy link
Member

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. 21c6853) 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 (Reorg source tree c-ares/c-ares#349) in c-ares 1.17.0 and reflected in deps: reflect c-ares source tree #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 richardlau added the tools Issues and PRs related to the tools directory. label Oct 18, 2024
@nodejs-github-bot
Copy link
Collaborator

Review requested:

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

@nodejs-github-bot nodejs-github-bot added cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Oct 18, 2024
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 18, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 18, 2024
@nodejs-github-bot

This comment was marked as outdated.

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.
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.
Add comment noting that `cares_sources_common` is generated by tooling.
Remove duplicated entries.
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 18, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 18, 2024
@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 19, 2024
@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 19, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 20, 2024
@nodejs-github-bot
Copy link
Collaborator

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 <rlau@redhat.com>
PR-URL: https://github.com/nodejs/node/pull/55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
--------------------------------------------------------------------------------
   ℹ  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 <rlau@redhat.com>
 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 <rlau@redhat.com>
 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 <rlau@redhat.com>
 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: #55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

[detached HEAD 3fee2cfca7] tools: add script to synch c-ares source lists
Author: Richard Lau <rlau@redhat.com>
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: #55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

[detached HEAD b43ba9ef2f] build: synchronize list of c-ares source files
Author: Richard Lau <rlau@redhat.com>
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: #55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

[detached HEAD a857ed1d34] build: tidy up cares.gyp
Author: Richard Lau <rlau@redhat.com>
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

@richardlau richardlau added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 20, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 20, 2024
@nodejs-github-bot
Copy link
Collaborator

Landed in b0ffe9e...c124cfb

nodejs-github-bot pushed a commit that referenced this pull request Oct 20, 2024
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: #55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
nodejs-github-bot pushed a commit that referenced this pull request Oct 20, 2024
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: #55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
nodejs-github-bot pushed a commit that referenced this pull request Oct 20, 2024
Add comment noting that `cares_sources_common` is generated by tooling.
Remove duplicated entries.

PR-URL: #55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@richardlau richardlau deleted the update-c-ares branch October 20, 2024 18:11
aduh95 pushed a commit that referenced this pull request Oct 23, 2024
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: #55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
aduh95 pushed a commit that referenced this pull request Oct 23, 2024
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: #55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
aduh95 pushed a commit that referenced this pull request Oct 23, 2024
Add comment noting that `cares_sources_common` is generated by tooling.
Remove duplicated entries.

PR-URL: #55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@aduh95 aduh95 mentioned this pull request Oct 24, 2024
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
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: nodejs#55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
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: nodejs#55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
Add comment noting that `cares_sources_common` is generated by tooling.
Remove duplicated entries.

PR-URL: nodejs#55445
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants