Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Future Node Icu4c Discussion #44110

Closed
bcomnes opened this issue Sep 18, 2015 · 7 comments
Closed

Future Node Icu4c Discussion #44110

bcomnes opened this issue Sep 18, 2015 · 7 comments

Comments

@bcomnes
Copy link
Contributor

bcomnes commented Sep 18, 2015

So there is this longstanding issue: #36681

So the issue is that the node executable links against libstdc++ because of an archaic deployment target setting.

It would be cool if node would link against libc++, but that will require someone to convince upstream.

bret-mbr:~ bret$ otool -L `which node`
/usr/local/bin/node:
    /usr/local/opt/icu4c/lib/libicui18n.55.dylib (compatibility version 55.0.0, current version 55.1.0)
    /usr/local/opt/icu4c/lib/libicuuc.55.dylib (compatibility version 55.0.0, current version 55.1.0)
    /usr/local/opt/icu4c/lib/libicudata.55.1.dylib (compatibility version 55.0.0, current version 55.1.0)
    /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1153.18.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
    /usr/lib/libstdc++.6.dylib (compatibility version 7.0.0, current version 104.1.0)
    /usr/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 283.0.0)

Even in Node 4+, they are building against libstdc++ which presents a potential issue with the homebrew Icu4c which links against libc++. While there hasn't been a huge amount of direct evidence that this is a major issue, it still warrants the considerations of the following options:

  1. Stage Icu4c as a resource and staticly build along with node itself. chrmoritz@0dd892d cc @chrmoritz

  2. Link against homebrews Icu4c as a dependency bcomnes@309e9b4

Both seem to work fine. Long term we can see if we can encourage linking against libc++ upstream but it would be nice to figure something out for now.

cc @DomT4 @MikeMcQuaid

@bcomnes
Copy link
Contributor Author

bcomnes commented Sep 18, 2015

After thinking more on this, staging a resource seems to make the most sense currently.

  • It lets us include full icu4c support by default
  • Keeps homebrew's node in line with the distributed binary
  • We can always change to a dependency model once the outstanding issue is resolved upstream.

@chrmoritz
Copy link
Contributor

The problem is not node itself, but their upstream V8 or more precise V8's build system gyp, see: nodejs/node-v0.x-archive#7919

BTW: A more updated commit for the statically linking to a resource staged icu4c is in chrmoritz@0dd892d

@DomT4
Copy link
Member

DomT4 commented Sep 18, 2015

Yes, as @chrmoritz highlights this isn't technically a Node problem. I finally fixed Homebrew's v8 formula a month or so ago now to build against libc++, as the mix of deployment targets can cause trouble at runtime as seen with icu4c here.

People (including us) have talked to Node before about doing the same thing and been told that we could but it'd be wildly unsupported, as discussed in part in the thread @chrmoritz points to there. I don't expect much movement on this going forwards.

I think a resource is the way to go here.

@bcomnes
Copy link
Contributor Author

bcomnes commented Sep 18, 2015

👍 for resource.

@joshmanders
Copy link

👍 for resource.

@DomT4
Copy link
Member

DomT4 commented Sep 19, 2015

Feel free to take a swing at the resource whenever. Happy to do it if you'd like, but I'm kind of enjoying someone else looking after the Node formula 😜.

@DomT4 DomT4 closed this as completed Sep 19, 2015
@bcomnes
Copy link
Contributor Author

bcomnes commented Sep 19, 2015

I belive @chrmoritz was a PR away from the resource approach chrmoritz@0dd892d. Send it in if you feel its ready, or I can do it for you if you are busy today.

@Homebrew Homebrew locked and limited conversation to collaborators Jul 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants