FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add automation for updating `libuv` dependency by facutuesca · Pull Request #45362 · nodejs/node · GitHub

/ node Public

Add automation for updating libuv dependency - #45362

Merged
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
facutuesca:libuv-ci-update
Nov 21, 2022
ConversationCommits (3)ChecksFiles changed
Merged

Add automation for updating libuv dependency#45362
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
facutuesca:libuv-ci-update

Conversation

facutuesca commented Nov 7, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Add a Github Action that checks for new versions of the libuv C library, and creates a PR to update it if a newer version than the one present in the repo is found.

Refs: nodejs/security-wg#828

Details

Currently, if one runs the script to update to version 1.44.2, the uv folder will end up with changes (even though Node's libuv version is also at 1.44.2):

bash tools/dep_updaters/update-libuv.sh 1.44.2
# (...script output....)

git status
# Changes not staged for commit:
#  (use "git add <file>..." to update what will be committed)
#  (use "git restore <file>..." to discard changes in working directory)
#        modified:   deps/uv/include/uv/win.h
#        modified:   deps/uv/src/win/poll.c

This is due to a change made when merging #42340. More specifically, see #42340 (comment). This accounts for the differences between Node's repo and upstream.

Since this seems to be a change that will be reverted in the next release (as per the comment), it should already be taken care of when the update-libuv.sh script is run for that next version.

cc: @lpinca

nodejs-github-bot commented Nov 7, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/actions

nodejs-github-bot added meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Nov 7, 2022

lpinca commented Nov 7, 2022

Copy link
Copy Markdown
Member

Sometimes manual changes to common.gypi and uv.gyp are needed but I guess we will notice this due to compilation failures. Another thing is how to add the changelog in the commit message.

facutuesca commented Nov 7, 2022

Copy link
Copy Markdown
Contributor Author

Sometimes manual changes to common.gypi and uv.gyp are needed but I guess we will notice this due to compilation failures. Another thing is how to add the changelog in the commit message.

The idea for this script is to automate the copying of the source code to Node's repo, eliminating human errors in that step. Any extra changes (such as fixing the build files, or customizing the commit message) can be done on top of the branch created by the script, by the people reviewing/merging the PR.

My point is: automating the commit message to include the new version's changelog would add complexity, so maybe it could be a future improvement instead of adding it here.

lpinca approved these changes Nov 7, 2022
View reviewed changes
Comment thread tools/update-libuv.sh Outdated

lpinca Nov 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

https://dist.libuv.org/dist/v$LIBUV_VERSION/libuv-v$LIBUV_VERSION.tar.gz can also be used. I have no preference.

anonrig approved these changes Nov 10, 2022
View reviewed changes

anonrig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2022

anonrig commented Nov 10, 2022

Copy link
Copy Markdown
Member

@facutuesca Can you resolve the conflicts?

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2022

nodejs-github-bot commented Nov 10, 2022

Copy link
Copy Markdown
Collaborator

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

facutuesca commented Nov 10, 2022

Copy link
Copy Markdown
Contributor Author

@facutuesca Can you resolve the conflicts?

Fixed

anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2022

nodejs-github-bot commented Nov 10, 2022

Copy link
Copy Markdown
Collaborator

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

This was referenced Nov 11, 2022
Open
Open
This was referenced Nov 13, 2022
Open
Open
Open
mhdawson reviewed Nov 15, 2022
View reviewed changes
Comment thread tools/update-libuv.sh Outdated
gengjiawen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 17, 2022
aduh95 reviewed Nov 20, 2022
View reviewed changes

aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

s/Node/Node.js/

node/doc/README.md

Lines 87 to 89 in 184af1c

* Use _Node.js_ and not _Node_, _NodeJS_, or similar variants.
<!-- lint enable prohibited-strings remark-lint-->
* When referring to the executable, _`node`_ is acceptable.

Can you please wrap the lines in the markdown document to 80 chars?

Comment thread tools/dep_updaters/README.md Outdated
Comment thread tools/dep_updaters/README.md Outdated
Comment thread tools/dep_updaters/README.md Outdated
Comment thread tools/dep_updaters/README.md Outdated
Comment thread tools/dep_updaters/README.md Outdated
Comment thread tools/dep_updaters/README.md Outdated
Comment thread tools/dep_updaters/update-libuv.sh Outdated
Comment thread tools/dep_updaters/update-libuv.sh Outdated
Comment thread tools/dep_updaters/README.md Outdated
aduh95 removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 20, 2022
tools: add automation for updating libuv dependency
a392194 
Add a Github Action that checks for new versions of the `libuv` C
library, and creates a PR to update it if a newer version than the one
present in the repo is found.

Refs: nodejs/security-wg#828
tools: move libuv update script to dep_updaters folder
6299a6a
facutuesca requested a review from aduh95 November 21, 2022 09:14
aduh95 reviewed Nov 21, 2022
View reviewed changes
Comment thread tools/dep_updaters/README.md Outdated
Comment thread tools/dep_updaters/README.md Outdated
facutuesca requested a review from aduh95 November 21, 2022 11:17
aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Nov 21, 2022
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 21, 2022
nodejs-github-bot merged commit 59bb198 into nodejs:main Nov 21, 2022

nodejs-github-bot commented Nov 21, 2022

Copy link
Copy Markdown
Collaborator

Landed in 59bb198

ruyadorno pushed a commit that referenced this pull request Nov 21, 2022
tools: add automation for updating libuv dependency
f3f1aed 
Add a Github Action that checks for new versions of the `libuv` C
library, and creates a PR to update it if a newer version than the one
present in the repo is found.

Refs: nodejs/security-wg#828
PR-URL: #45362
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
ruyadorno mentioned this pull request Nov 24, 2022
Merged
facutuesca deleted the libuv-ci-update branch December 5, 2022 09:48
facutuesca mentioned this pull request Dec 8, 2022
Closed
23 tasks
danielleadams mentioned this pull request Dec 30, 2022
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
tools: add automation for updating libuv dependency
9e601db 
Add a Github Action that checks for new versions of the `libuv` C
library, and creates a PR to update it if a newer version than the one
present in the repo is found.

Refs: nodejs/security-wg#828
PR-URL: #45362
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
tools: add automation for updating libuv dependency
682a730 
Add a Github Action that checks for new versions of the `libuv` C
library, and creates a PR to update it if a newer version than the one
present in the repo is found.

Refs: nodejs/security-wg#828
PR-URL: #45362
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 5, 2023
tools: add automation for updating libuv dependency
66f85fd 
Add a Github Action that checks for new versions of the `libuv` C
library, and creates a PR to update it if a newer version than the one
present in the repo is found.

Refs: nodejs/security-wg#828
PR-URL: #45362
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lpinca lpinca approved these changes
anonrig anonrig approved these changes
aduh95 aduh95 approved these changes
+1 more reviewer
mhdawson mhdawson left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL