GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add automation for updating `base64` dependency by facutuesca · Pull Request #45300 · nodejs/node

Conversation

@facutuesca

@facutuesca facutuesca commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

Add a Github Action that checks for new versions of the base64 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot

nodejs-github-bot commented Nov 3, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/actions
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot 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 3, 2022
aduh95
aduh95 reviewed Nov 3, 2022
View reviewed changes
Comment thread tools/update-base64.sh Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread .github/workflows/tools.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
@aduh95

aduh95 commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

I tried running the script locally to update to 0.5.0, and I see only two modified files. Is it expected?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@facutuesca

facutuesca commented Nov 3, 2022

Copy link
Copy Markdown
Contributor Author

I tried running the script locally to update to 0.5.0, and I see only two modified files. Is it expected?

Yeah, currently the version of base64 present in Node's repo is not 0.4.0, but rather a specific commit between 0.4.0 and 0.5.0 (which was done by this PR #44032)

Looking at the commit history of base64, you can see that that commit is the last one before the version was bumped to 0.5.0:
image

So indeed updating to 0.5.0 only changes two files (one leftover empty file that is removed, and the other with the version bump)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 approved these changes Nov 3, 2022
View reviewed changes

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It may need to be disabled next time we need to pull a version between releases, but hopefully this won't be needed, and it's certainly better to have it automated in the mean time anyway

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
lpinca
lpinca approved these changes Nov 3, 2022
View reviewed changes
Comment thread .github/workflows/tools.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread tools/update-base64.sh Outdated

@lpinca lpinca Nov 3, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
BASE_DIR="$( pwd )"
BASE_DIR=$(cd "$(dirname "$0")/.." && pwd)

This should make it work from anywhere.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@facutuesca facutuesca Nov 4, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@facutuesca
tools: add automation for updating base64 dependency
ca1857a 
Add a Github Action that checks for new versions of the `base64` 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
Trott
Trott approved these changes Nov 6, 2022
View reviewed changes
@Trott Trott added the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 6, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 6, 2022
@nodejs-github-bot
nodejs-github-bot merged commit 1f51713 into nodejs:main Nov 6, 2022
@nodejs-github-bot

nodejs-github-bot commented Nov 6, 2022

Copy link
Copy Markdown
Collaborator

Landed in 1f51713
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@facutuesca
facutuesca deleted the base64-ci-update branch November 7, 2022 10:18
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
@facutuesca @RafaelGSS
tools: add automation for updating base64 dependency
eda4ae5 
Add a Github Action that checks for new versions of the `base64` 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: #45300
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Nov 10, 2022
v19.1.0 proposal #45269
Merged
@lucshi

lucshi commented Nov 11, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

There is a Base64 update patch in PR #45091 which is approved but not merged yet.
To make the latest Base64 works best, it needs changes in gyp files as in that PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@facutuesca

facutuesca commented Nov 14, 2022

Copy link
Copy Markdown
Contributor Author

There is a Base64 update patch in PR #45091 which is approved but not merged yet. To make the latest Base64 works best, it needs changes in gyp files as in that PR.

@lucshi This script only deals with updating the folder base64 to the latest released version, any changes needed to Node's gyp files have to be added manually on top of that.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@facutuesca facutuesca mentioned this pull request Dec 8, 2022
Automate updates of all dependencies nodejs/security-wg#828
Closed
23 tasks done
@danielleadams danielleadams mentioned this pull request Dec 30, 2022
v18.13.0 release proposal #46025
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@facutuesca @danielleadams
tools: add automation for updating base64 dependency
4a4f63d 
Add a Github Action that checks for new versions of the `base64` 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: #45300
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@facutuesca @danielleadams
tools: add automation for updating base64 dependency
75a146f 
Add a Github Action that checks for new versions of the `base64` 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: #45300
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 approved these changes
@Trott Trott Trott approved these changes
@lpinca lpinca lpinca approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

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.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@facutuesca @nodejs-github-bot @aduh95 @lucshi @Trott @lpinca
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.