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.

Sign up for GitHub

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

Jump to bottom

build: fix building with system icu 76 #55563

Merged
merged 1 commit into from
Oct 29, 2024
Merged

build: fix building with system icu 76 #55563

merged 1 commit into from
Oct 29, 2024
+1 −1

Conversation

cho-m
Copy link
Contributor

@cho-m cho-m commented Oct 27, 2024

ICU 76 decided to reduce overlinking1 thus icu-i18n will no longer add icu-uc when linking to shared libraries. This results in undefined symbols/references when trying to build with system ICU 76.

Example of build error (seen on v22.10.0 and v23.1.0) is:

.../out/Release/obj.target/gen-regexp-special-case/deps/v8/src/regexp/gen-regexp-special-case.o: in function `v8::internal::PrintSet(std::basic_ofstream<char, std::char_traits<char> >&, char const*, icu_76::UnicodeSet const&)':
gen-regexp-special-case.cc:(.text._ZN2v88internal8PrintSetERSt14basic_ofstreamIcSt11char_traitsIcEEPKcRKN6icu_7610UnicodeSetE+0x9c): undefined reference to `icu_76::UnicodeSet::getRangeStart(int) const'
/home/linuxbrew/.linuxbrew/opt/binutils/bin/ld: gen-regexp-special-case.cc:(.text._ZN2v88internal8PrintSetERSt14basic_ofstreamIcSt11char_traitsIcEEPKcRKN6icu_7610UnicodeSetE+0xc4): undefined reference to `icu_76::UnicodeSet::getRangeEnd(int) const'

ICU 76:

$ pkg-config --libs-only-l icu-i18n
-licui18n

$ pkg-config --libs-only-l icu-i18n icu-uc
-licui18n -licuuc

ICU 75:

$ pkg-config --libs-only-l icu-i18n
-licui18n -licuuc -licudata

Specifically seen at Homebrew when testing new ICU 76.1 release Homebrew/homebrew-core#193114.

CI successfully built v22.10.0 after applying this patch.

Should be backwards compatible as icu-uc.pc has existed as long as icu-i18n.pc - unicode-org/icu@d390758

Footnotes

  1. unicode-org/icu@199bc82 ↩

@cho-m
build: fix building with system icu 76
54299ac 
ICU 76 decided to reduce overlinking[^1] thus `icu-i18n` will no longer
add `icu-uc` when linking to shared libraries. This results in undefined
symbols/references when trying to build with system ICU 76.

[^1]: unicode-org/icu@199bc82
@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 Oct 27, 2024
@cho-m cho-m mentioned this pull request Oct 27, 2024
Merged
12 tasks
@RedYetiDev RedYetiDev added the icu Issues and PRs related to the ICU dependency. label Oct 27, 2024
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 28, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 28, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 28, 2024
@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 29, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 29, 2024
@nodejs-github-bot nodejs-github-bot merged commit 81517fa into nodejs:main Oct 29, 2024
73 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 81517fa

@richardlau richardlau added lts-watch-v18.x PRs that may need to be released in v18.x. lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x labels Oct 29, 2024
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Oct 30, 2024
@cho-m @Ceres6
build: fix building with system icu 76
67c40f2 
ICU 76 decided to reduce overlinking[^1] thus `icu-i18n` will no longer
add `icu-uc` when linking to shared libraries. This results in undefined
symbols/references when trying to build with system ICU 76.

[^1]: unicode-org/icu@199bc82

PR-URL: nodejs#55563
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@cho-m cho-m deleted the icu76-support branch October 31, 2024 12:56
RafaelGSS pushed a commit that referenced this pull request Nov 1, 2024
@cho-m @RafaelGSS
build: fix building with system icu 76
4887214 
ICU 76 decided to reduce overlinking[^1] thus `icu-i18n` will no longer
add `icu-uc` when linking to shared libraries. This results in undefined
symbols/references when trying to build with system ICU 76.

[^1]: unicode-org/icu@199bc82

PR-URL: #55563
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
@cho-m @louwers
build: fix building with system icu 76
e38046e 
ICU 76 decided to reduce overlinking[^1] thus `icu-i18n` will no longer
add `icu-uc` when linking to shared libraries. This results in undefined
symbols/references when trying to build with system ICU 76.

[^1]: unicode-org/icu@199bc82

PR-URL: nodejs#55563
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
aduh95 pushed a commit that referenced this pull request Nov 2, 2024
@cho-m @aduh95
build: fix building with system icu 76
5c9073b 
ICU 76 decided to reduce overlinking[^1] thus `icu-i18n` will no longer
add `icu-uc` when linking to shared libraries. This results in undefined
symbols/references when trying to build with system ICU 76.

[^1]: unicode-org/icu@199bc82

PR-URL: #55563
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@richardlau richardlau richardlau approved these changes

Assignees
No one assigned
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. icu Issues and PRs related to the ICU dependency. lts-watch-v18.x PRs that may need to be released in v18.x. lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cho-m @nodejs-github-bot @lpinca @richardlau @RedYetiDev