Skip to content

build: check Apple clang version in configure script - #59358

Closed
targos wants to merge 2 commits into
nodejs:mainfrom
targos:is_apple_clang
Closed

build: check Apple clang version in configure script#59358
targos wants to merge 2 commits into
nodejs:mainfrom
targos:is_apple_clang

Conversation

@targos

@targostargos commented Aug 5, 2025

Copy link
Copy Markdown
Member

Apple clang version number is not the same as the actual LLVM version

@targostargos added dont-land-on-v20.x dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. labels Aug 5, 2025
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Aug 5, 2025
@targostargos mentioned this pull request Aug 5, 2025
Merged
@legendecas

legendecas commented Aug 5, 2025

Copy link
Copy Markdown
Member

I wonder if there are issues with lower clang versions (e.g. v17), other than nodejs/node-v8#302? Which was an unexpected version requirement.

From nodejs/node-v8#302 (comment), it seems at least clang v17 is still supported.

@legendecas

legendecas commented Aug 5, 2025•
edited
Loading

Copy link
Copy Markdown
Member

We should bump the BUILDING.md prerequisites as well:

Both toolchains are almost the second latest versions of platform toolchains. It seems a bit aggressive to me.

@targos

targos commented Aug 5, 2025

Copy link
Copy Markdown
MemberAuthor

I wonder if there are issues with lower clang versions (e.g. v17), other than nodejs/node-v8#302 Which was an unexpected version requirement.

Maybe not yet, but since v19 is widely available, I'm not sure it's that aggressive. V8 moves fast and we don't know for how long we'll be able to compile it with lower versions. Clang 18 and 19 also improve C++20 support.

My hope is that we can stay with this new version requirement for a relatively long time.

@targos

targos commented Aug 5, 2025

Copy link
Copy Markdown
MemberAuthor

Also, Xcode and Visual Studio are both continuously updated by their editors. It's more difficult to install an earlier version than the latest.

@targostargos added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Aug 5, 2025
legendecas
legendecas reviewed Aug 5, 2025
View reviewed changes
Comment threadBUILDING.md Outdated
@targos
build: check Apple clang version in configure script
52b8993
Apple clang version number is not the same as the actual LLVM version
@targos
doc: update minimum Xcode and VS versions
65b463b
For Clang >= 19.1.0, Xcode 16.3 or Visual Studio 17.13 is required.
@legendecaslegendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 5, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 5, 2025
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented Aug 5, 2025

Copy link
Copy Markdown
Collaborator

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

@targos

targos commented Aug 6, 2025

Copy link
Copy Markdown
MemberAuthor

@nodejs/build

@targostargos added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 6, 2025
richardlau
richardlau approved these changes Aug 6, 2025
View reviewed changes

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

Comment is non-blocking.

Comment threadconfigure.py
elif clang_version < (19, 1, 0) if is_clang else gcc_version < (12, 2, 0):
warn(f'C++ compiler (CXX={CXX}, {version_str}) too old, need g++ 12.2.0 or clang++ 19.1.0')
elif (is_apple and clang_version < (17, 0, 0) or not is_apple and clang_version < (19, 1, 0)) if is_clang else gcc_version < (12, 2, 0):
warn(f'C++ compiler (CXX={CXX}, {version_str}) too old, need g++ 12.2.0, clang++ 19.1.0, or Apple clang++ 17.0.0')

@richardlaurichardlauAug 6, 2025•
edited
Loading

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.

Non-blocking observation -- would it be potentially confusing to mention Apple clang if, for example, you get this warning message on something other than macOS (e.g Linux)?

@targostargosAug 7, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

#59385

@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 7, 2025
nodejs-github-bot pushed a commit that referenced this pull request Aug 7, 2025
@targos@nodejs-github-bot
build: check Apple clang version in configure script
954d3f4
Apple clang version number is not the same as the actual LLVM version
PR-URL: #59358
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@nodejs-github-bot

nodejs-github-bot commented Aug 7, 2025

Copy link
Copy Markdown
Collaborator

Landed in 0309806...3f6f6db

nodejs-github-bot pushed a commit that referenced this pull request Aug 7, 2025
@targos@nodejs-github-bot
doc: update minimum Xcode and VS versions
3f6f6db
For Clang >= 19.1.0, Xcode 16.3 or Visual Studio 17.13 is required.
PR-URL: #59358
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targos
targos deleted the is_apple_clang branch August 7, 2025 07:50
mete0rfish pushed a commit to mete0rfish/node-contribute that referenced this pull request Aug 9, 2025
@targos@mete0rfish
build: check Apple clang version in configure script
5005e95
Apple clang version number is not the same as the actual LLVM version
PR-URL: nodejs#59358
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
mete0rfish pushed a commit to mete0rfish/node-contribute that referenced this pull request Aug 9, 2025
@targos@mete0rfish
doc: update minimum Xcode and VS versions
1e89912
For Clang >= 19.1.0, Xcode 16.3 or Visual Studio 17.13 is required.
PR-URL: nodejs#59358
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@RafaelGSSRafaelGSS mentioned this pull request Oct 9, 2025
Merged
9 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpincalpincalpinca approved these changes
@joyeecheungjoyeecheungjoyeecheung approved these changes
@richardlaurichardlaurichardlau approved these changes
@legendecaslegendecaslegendecas approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.buildIssues and PRs related to build files or the CI.commit-queue-rebaseAdd this label to allow the Commit Queue to land a PR in several commits.dont-land-on-v22.xPRs that should not land on the v22.x-staging branch and should not be released in v22.x.dont-land-on-v24.xPRs that should not land on the v24.x-staging branch and should not be released in v24.x.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@targos@legendecas@nodejs-github-bot@lpinca@joyeecheung@richardlau