Skip to content

build: fix Windows small-icu build with ClangCL#64263

Open
moluopro wants to merge 1 commit into
nodejs:mainfrom
moluopro:fix-win-small-icu-genccode-arch
Open

build: fix Windows small-icu build with ClangCL#64263
moluopro wants to merge 1 commit into
nodejs:mainfrom
moluopro:fix-win-small-icu-genccode-arch

Conversation

@moluopro

@moluopro moluopro commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

When building the static libnode library with small-icu on Windows, the genccode step fails under ClangCL because the target CPU architecture is not passed to the tool.

The Windows full-ICU branch already passes -c <(target_arch) to genccode when clang==1, while preserving the existing MSVC command. The small-ICU branch did not have the same conditional handling, so it hit ICU's architecture check under ClangCL.

This change makes the Windows small-ICU branch consistent with the full-ICU branch:

  • Pass -c <(target_arch) when clang==1.
  • Keep the existing genccode command unchanged for the non-ClangCL MSVC path.

Reproduction

$env:config_flags = '--v8-disable-temporal-support'
.\vcbuild.bat static nonpm small-icu no-cctest openssl-no-asm x64

Windows builds for Node.js 24+ use ClangCL, and the build fails with:

genccode
genccode: CPU architecture must be set for Clang-CL
Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for '..\..\deps\icu-tmp\icudt78l.dat;..\..\out\Release\\obj\global_intermediate\icutmp\icudt78l.dat' exited with code 1. [...\tools\icu\icudata.vcxproj]

Signed-off-by: moluopro <moluopro@qq.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added i18n-api Issues and PRs related to the i18n implementation. icu Issues and PRs related to the ICU dependency. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Jul 2, 2026

@StefanStojanovic StefanStojanovic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested it locally and it worked.

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 6, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 6, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

Labels

i18n-api Issues and PRs related to the i18n implementation. icu Issues and PRs related to the ICU dependency. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants