Skip to content

build: fix --with-intl=system-icu for cross compile #3808

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
Closed
wants to merge 1 commit into from
Closed

build: fix --with-intl=system-icu for cross compile #3808

wants to merge 1 commit into from

Conversation

@srl295
Copy link
Member

@srl295 srl295 commented Nov 13, 2015

  • add toolset stuff to icu-system.gyp

Fixes: #3801

@srl295 srl295 added the i18n-api Issues and PRs related to the i18n implementation. label Nov 13, 2015
@srl295
Copy link
Member Author

srl295 commented Nov 13, 2015

This does not fix all possible cross compilation situations. It does require that pkg-config knows about the command line arguments to find icu(s) that support both your host AND target . I updated the README to note this.

@srl295
Copy link
Member Author

srl295 commented Nov 13, 2015

@jasnell - may be LTS candidate? This file is unchanged from v0.12. Could bite those trying to package Node… like say @chrislea

I think any time 'want_separate_host_toolset': 1 and using system-icu, this patch will be needed.

@bnoordhuis
Copy link
Member

bnoordhuis commented Nov 13, 2015

The commit log says "update docs" but the commit itself only changes the .gyp? LGTM apart from that.

@srl295
build: fix --with-intl=system-icu for x-compile
d34db5b 
* add toolset stuff to icu-system.gyp
* update docs

Fixes: nodejs#3801
@srl295
Copy link
Member Author

srl295 commented Nov 13, 2015

@bnoordhuis slip of the finger— included now.

@bnoordhuis
Copy link
Member

bnoordhuis commented Nov 13, 2015

LGTM

1 similar comment
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

LGTM

srl295 added a commit that referenced this pull request Nov 15, 2015
@srl295 @jasnell
build: fix --with-intl=system-icu for x-compile
84f0964 
* add toolset stuff to icu-system.gyp
* update docs

Fixes: #3801
PR-URL: #3808
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Nov 15, 2015

Landed in 84f0964

@jasnell jasnell closed this Nov 15, 2015
srl295 added a commit that referenced this pull request Nov 17, 2015
@srl295 @Fishrock123
build: fix --with-intl=system-icu for x-compile
2a69b68 
* add toolset stuff to icu-system.gyp
* update docs

Fixes: #3801
PR-URL: #3808
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
srl295 added a commit that referenced this pull request Nov 30, 2015
@srl295
build: fix --with-intl=system-icu for x-compile
6d29b02 
* add toolset stuff to icu-system.gyp
* update docs

Fixes: #3801
PR-URL: #3808
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
srl295 added a commit that referenced this pull request Dec 4, 2015
@srl295 @rvagg
build: fix --with-intl=system-icu for x-compile
112b602 
* add toolset stuff to icu-system.gyp
* update docs

Fixes: #3801
PR-URL: #3808
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell jasnell mentioned this pull request Dec 17, 2015
Closed
srl295 added a commit that referenced this pull request Dec 17, 2015
@srl295 @jasnell
build: fix --with-intl=system-icu for x-compile
60dda70 
* add toolset stuff to icu-system.gyp
* update docs

Fixes: #3801
PR-URL: #3808
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
srl295 added a commit that referenced this pull request Dec 23, 2015
@srl295 @jasnell
build: fix --with-intl=system-icu for x-compile
77306f8 
* add toolset stuff to icu-system.gyp
* update docs

Fixes: #3801
PR-URL: #3808
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@srl295 srl295 deleted the fix-system-icu branch July 6, 2018 21:01
@Clemens-git76 Clemens-git76 mentioned this pull request Apr 16, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

i18n-api Issues and PRs related to the i18n implementation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@srl295 @bnoordhuis @jasnell @MylesBorins