Skip to content

build: do not rely on gn_helpers in GN build - #51439

Closed
zcbenz wants to merge 1 commit into
nodejs:mainfrom
zcbenz:remove-gn-helper-dep
Closed

build: do not rely on gn_helpers in GN build#51439
zcbenz wants to merge 1 commit into
nodejs:mainfrom
zcbenz:remove-gn-helper-dep

Conversation

@zcbenz

@zcbenzzcbenz commented Jan 12, 2024

Copy link
Copy Markdown
Contributor

Make the GN build more friendly for non-Chromium environment:

  1. Use relative imports to avoid assuming the location of Node.
  2. Remove the dependency of gn_helpers.

@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. gyp Issues and PRs related to the GYP tool and .gyp build files needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3. tools Issues and PRs related to the tools directory. labels Jan 12, 2024
@zcbenz
zcbenzforce-pushed the remove-gn-helper-dep branch from f487772 to 8c12697CompareJanuary 12, 2024 01:01
@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 12, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 12, 2024
@nodejs-github-bot

nodejs-github-bot commented Jan 12, 2024

Copy link
Copy Markdown
Collaborator

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

targos
targos approved these changes Jan 13, 2024
View reviewed changes

@targostargos 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.

RSLGTM

lpinca
lpinca approved these changes Jan 25, 2024
View reviewed changes

@lpincalpinca 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.

RSLGTM

@nodejs-github-bot

nodejs-github-bot commented Jan 25, 2024

Copy link
Copy Markdown
Collaborator

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

@zcbenz
zcbenzforce-pushed the remove-gn-helper-dep branch from 8c12697 to 0f8d7d5CompareFebruary 21, 2024 03:48
@zcbenzzcbenz added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 21, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 21, 2024
@nodejs-github-bot

nodejs-github-bot commented Feb 21, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 21, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 21, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 21, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 21, 2024

Copy link
Copy Markdown
Collaborator

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

@zcbenz
zcbenzforce-pushed the remove-gn-helper-dep branch from 0f8d7d5 to 9b6d263CompareFebruary 23, 2024 00:58
@zcbenzzcbenz added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2024
@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2024

Copy link
Copy Markdown
Collaborator

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

@zcbenzzcbenz added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2024
@nodejs-github-botnodejs-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 Feb 23, 2024
@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2024

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/51439
✔ Done loading data for nodejs/node/pull/51439
----------------------------------- PR info ------------------------------------
Title build: do not rely on gn_helpers in GN build (#51439)
⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch zcbenz:remove-gn-helper-dep -> nodejs:main
Labels build, tools, needs-ci, gyp, quic
Commits 1
- build: do not rely on gn_helpers in GN build
Committers 1
- Cheng Zhao PR-URL: https://github.com/nodejs/node/pull/51439
Reviewed-By: Yagiz Nizipli Reviewed-By: Michaël Zasso Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51439
Reviewed-By: Yagiz Nizipli Reviewed-By: Michaël Zasso Reviewed-By: Luigi Pinca --------------------------------------------------------------------------------
⚠ Commits were pushed since the last approving review:
⚠ - build: do not rely on gn_helpers in GN build
ℹ This PR was created on Fri, 12 Jan 2024 00:57:41 GMT
✔ Approvals: 3
✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/51439#pullrequestreview-1817092470
✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/51439#pullrequestreview-1819939080
✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/51439#pullrequestreview-1844641924
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2024-02-23T09:59:42Z: https://ci.nodejs.org/job/node-test-pull-request/57339/
- Querying data for job/node-test-pull-request/57339/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/8018573136

zcbenz added a commit that referenced this pull request Feb 23, 2024
@zcbenz
build: do not rely on gn_helpers in GN build
f04abdb
PR-URL: #51439
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@zcbenz

zcbenz commented Feb 23, 2024

Copy link
Copy Markdown
ContributorAuthor

Landed in f04abdb

@zcbenzzcbenz closed this Feb 23, 2024
@zcbenz
zcbenz deleted the remove-gn-helper-dep branch February 23, 2024 11:50
marco-ippolito pushed a commit that referenced this pull request Feb 26, 2024
@zcbenz@marco-ippolito
build: do not rely on gn_helpers in GN build
c3a5727
PR-URL: #51439
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Feb 27, 2024
@zcbenz@marco-ippolito
build: do not rely on gn_helpers in GN build
86ac787
PR-URL: #51439
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@marco-ippolitomarco-ippolito mentioned this pull request Mar 1, 2024
Merged
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@zcbenz@richardlau
build: do not rely on gn_helpers in GN build
012da16
PR-URL: #51439
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@richardlaurichardlau mentioned this pull request Mar 25, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpincalpincalpinca approved these changes
@anonriganonriganonrig approved these changes
@targostargostargos approved these changes
+1 more reviewer
@aymen94aymen94aymen94 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

buildIssues and PRs related to build files or the CI.commit-queue-failedAn error occurred while landing this pull request using GitHub Actions.gypIssues and PRs related to the GYP tool and .gyp build filesneeds-ciPRs that need a full CI run.quicIssues and PRs related to the QUIC implementation / HTTP/3.toolsIssues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@zcbenz@nodejs-github-bot@lpinca@anonrig@targos@aymen94