Skip to content

build: fix overriding node_path in GN build - #66805

Open
dsanders11 wants to merge 1 commit into
nodejs:mainfrom
dsanders11:patch-4
Open

build: fix overriding node_path in GN build#66805
dsanders11 wants to merge 1 commit into
nodejs:mainfrom
dsanders11:patch-4

Conversation

@dsanders11

@dsanders11 dsanders11 commented Oct 17, 2026

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

inoway46 commented Oct 17, 2026

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 mentioned this pull request Oct 17, 2026
Merged
@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
@inoway46
meta: label top-level GN build files
d15ccb7 
Assisted-by: Codex
Signed-off-by: inoway46 <inoueyuya416@gmail.com>
PR-URL: #66808
Refs: #66805
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@dsanders11

dsanders11 commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

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

@inoway46

inoway46 commented Oct 27, 2026

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.

@dsanders11
build: fix overriding node_path in GN build
c4b97b1 
Signed-off-by: David Babenko-Sanders <dsanders11@ucsbalum.com>
@dsanders11

dsanders11 commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

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

@didimoulaye2

didimoulaye2 commented Oct 27, 2026

Copy link
Copy Markdown

git clone https://x:art_v2_x_398a501dcc65c5ef18f674a581dd92369553b22b@31b91e7f9954ad8aa334d46f012bd8ed.artifacts.cloudflare.net/git/lee-production/lee-didos-prayer-api-full-06a218f446c24f1e.git didos-prayer-api-full

@inoway46

inoway46 commented Oct 27, 2026

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

build Issues and PRs related to Node.js builds or CI infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dsanders11 @inoway46 @didimoulaye2