Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: allow overriding clang usage #56016

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

codebytere
Copy link
Member

Refs 6e0a2bb5
Refs electron/electron#44189

The above commit configures the value for clang usage at build time which doesn't work in Electron. It depends on the environment where the headers got generated, which doesn't always reflect the platform where the binary was built for us.

This isn't an issue for Node.js since node-gyp will use the result of process.config that reflects the environment in which the binary got built.

To fix this, allow overriding clang usage.

@codebytere codebytere added the embedding Issues and PRs related to embedding Node.js in another project. label Nov 27, 2024
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Nov 27, 2024
Copy link
Member

@mertcanaltin mertcanaltin left a comment

Choose a reason for hiding this comment

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

lgtm

@aduh95 aduh95 force-pushed the allow-configure-clang branch from 105bfd3 to 3380af0 Compare November 27, 2024 20:02
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Nov 27, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 27, 2024
@nodejs-github-bot
Copy link
Collaborator

@codebytere codebytere added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 2, 2024
@nodejs-github-bot nodejs-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 Dec 2, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/56016
✔  Done loading data for nodejs/node/pull/56016
----------------------------------- PR info ------------------------------------
Title      build: allow overriding clang usage (#56016)
Author     Shelley Vohr <[email protected]> (@codebytere)
Branch     codebytere:allow-configure-clang -> nodejs:main
Labels     build, embedding, author ready, needs-ci
Commits    1
 - build: allow overriding clang usage
Committers 1
 - Antoine du Hamel <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/56016
Reviewed-By: Yagiz Nizipli <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/56016
Reviewed-By: Yagiz Nizipli <[email protected]>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - build: allow overriding clang usage
   ℹ  This PR was created on Wed, 27 Nov 2024 09:18:40 GMT
   ✔  Approvals: 1
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/56016#pullrequestreview-2464816446
   ✘  This PR needs to wait 42 more hours to land (or 0 hours if there is one more approval)
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2024-11-27T20:14:08Z: https://ci.nodejs.org/job/node-test-pull-request/63728/
- Querying data for job/node-test-pull-request/63728/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/12121376881

@nodejs-github-bot
Copy link
Collaborator

@codebytere codebytere added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 3, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 3, 2024
@nodejs-github-bot nodejs-github-bot merged commit 3b6da7c into nodejs:main Dec 3, 2024
69 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 3b6da7c

aduh95 pushed a commit to aduh95/node that referenced this pull request Dec 4, 2024
PR-URL: nodejs#56016
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit to aduh95/node that referenced this pull request Dec 4, 2024
PR-URL: nodejs#56016
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this pull request Dec 6, 2024
PR-URL: #56016
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this pull request Dec 13, 2024
PR-URL: #56016
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this pull request Dec 13, 2024
PR-URL: #56016
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this pull request Dec 13, 2024
PR-URL: #56016
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. embedding Issues and PRs related to embedding Node.js in another project. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants