Skip to content

build: fix overriding node_path in GN build - #66805

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
dsanders11:patch-4
Oct 30, 2026
Merged

nodejs-github-bot merged 1 commit into
nodejs:mainfrom
dsanders11:patch-4

Conversation

@dsanders11

Copy link
Copy Markdown
Contributor

Overriding the node_path variable would lead to an error due to the default value for node_module_version using the default value of node_path. GN's documentation calls out this exact issue, so I implemented their suggested solution and added a comment explaining it. Validated the fix in electron/electron#55080

@inoway46

Copy link
Copy Markdown
Contributor

https://github.com/nodejs/node/actions/runs/39009516514/job/117328805687?pr=66805

UnicodeDecodeError: 'utf-8' codec can't decode byte 0xb2 in position 244: invalid start byte

FYI: This looks like the test runner’s UTF-8 decoding issue addressed by #66783, possibly unrelated to the changes here.

@inoway46 inoway46 added the build Issues and PRs related to Node.js builds or CI infrastructure. label Oct 17, 2026
nodejs-github-bot pushed a commit that referenced this pull request Oct 21, 2026
Assisted-by: Codex
Signed-off-by: inoway46 <[email protected]>
PR-URL: #66808
Refs: #66805
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@dsanders11

Copy link
Copy Markdown
Contributor Author

@inoway46, is there anything I need to do here to move this PR along?

@inoway46

Copy link
Copy Markdown
Contributor

@dsanders11 Thanks for following up! The test runner fix in #66783 has landed, so the previous CI failure should be resolved.

Could you rebase onto the latest main to trigger a fresh CI run? After that, this should be ready for review.

Signed-off-by: David Babenko-Sanders <[email protected]>
@dsanders11

Copy link
Copy Markdown
Contributor Author

@inoway46, rebased and things looks green from my perspective. 🙂

@didimoulaye2

Copy link
Copy Markdown

@inoway46

Copy link
Copy Markdown
Contributor

The change looks good to me, and CI is green.

@codebytere @zcbenz, since you both have experience with the GN build, would either of you mind taking a look when you have a chance?

@inoway46 inoway46 added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 30, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 30, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@inoway46 inoway46 added author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue PRs queued for automated landing through the Commit Queue. labels Oct 30, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 4e48b9b into nodejs:main Oct 30, 2026
68 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 4e48b9b

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. build Issues and PRs related to Node.js builds or CI infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants