Skip to content

build: don't build icu with -fno-rtti - #8886

Closed
bnoordhuis wants to merge 1 commit into
nodejs:masterfrom
bnoordhuis:fix8867
Closed

build: don't build icu with -fno-rtti#8886
bnoordhuis wants to merge 1 commit into
nodejs:masterfrom
bnoordhuis:fix8867

Conversation

@bnoordhuis

Copy link
Copy Markdown
Member

ICU should be compiled with -frtti (and it sets that flag in its gyp
file) but it was also inheriting the -fno-rtti flag from common.gypi,
breaking the build on some systems.

Fixes: #8867

ICU should be compiled with -frtti (and it sets that flag in its gyp
file) but it was also inheriting the -fno-rtti flag from common.gypi,
breaking the build on some systems.
Fixes: nodejs#8867
@bnoordhuisbnoordhuis added build Issues and PRs related to build files or the CI. i18n-api Issues and PRs related to the i18n implementation. labels Oct 1, 2016
@nodejs-github-botnodejs-github-bot added the tools Issues and PRs related to the tools directory. label Oct 1, 2016
@mscdexmscdex removed the tools Issues and PRs related to the tools directory. label Oct 1, 2016

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

LGTM

@fhinkel

Copy link
Copy Markdown
Member

Landed in c1be609.

@fhinkelfhinkel closed this Oct 3, 2016
fhinkel pushed a commit that referenced this pull request Oct 3, 2016
ICU should be compiled with -frtti (and it sets that flag in its gyp
file) but it was also inheriting the -fno-rtti flag from common.gypi,
breaking the build on some systems.
Fixes: #8867
PR-URL: #8886
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
jasnell pushed a commit that referenced this pull request Oct 6, 2016
ICU should be compiled with -frtti (and it sets that flag in its gyp
file) but it was also inheriting the -fno-rtti flag from common.gypi,
breaking the build on some systems.
Fixes: #8867
PR-URL: #8886
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@MylesBorins

Copy link
Copy Markdown
Contributor

@bnoordhuis should this be backported or is it specific to v6?

@bnoordhuis

Copy link
Copy Markdown
MemberAuthor

Looking at tools/icu/icu-generic.gyp in the v4.x-staging branch, this would need to be back-ported, yes.

Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
ICU should be compiled with -frtti (and it sets that flag in its gyp
file) but it was also inheriting the -fno-rtti flag from common.gypi,
breaking the build on some systems.
Fixes: #8867
PR-URL: #8886
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 18, 2016
ICU should be compiled with -frtti (and it sets that flag in its gyp
file) but it was also inheriting the -fno-rtti flag from common.gypi,
breaking the build on some systems.
Fixes: #8867
PR-URL: #8886
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Nov 22, 2016

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

after the fact 👍

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

Labels

buildIssues and PRs related to build files or the CI.i18n-apiIssues and PRs related to the i18n implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@bnoordhuis@fhinkel@MylesBorins@jbergstroem@srl295@mscdex@nodejs-github-bot