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.

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

deps: add no-strict-aliasing to ICU cflags #23112

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Sep 27, 2018

This commit adds -Wno-strict-aliasing to the icu_implementation target.
The motivation for this is that this flags is enabled when building with
macosx, and will make the output a little cleaner when building on
other operating systems.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added i18n-api Issues and PRs related to the i18n implementation. tools Issues and PRs related to the tools directory. labels Sep 27, 2018
@danbev
Copy link
Contributor Author

danbev commented Sep 27, 2018

This commit adds -Wno-strict-aliasing to the icu_implementation target.
The motivation for this is that this flags is enabled when building with
macosx, and will make the output a little cleaner when building on
other operating systems.
@danbev
Copy link
Contributor Author

danbev commented Sep 28, 2018

@refack
Copy link
Contributor

refack commented Sep 28, 2018

FYI, I have a WIP PR #22920, part of is to ignore warnings on /deps/ and only warn for /src/

 common.gypi | 1 -
 node.gypi   | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/common.gypi b/common.gypi
index e87bb0f5a4..4b436ac828 100644
--- a/common.gypi
+++ b/common.gypi
@@ -367,7 +367,6 @@
         'ldflags': [ '-pthread' ],
       }],
       [ 'OS in "linux freebsd openbsd solaris android aix cloudabi"', {
-        'cflags': [ '-Wall', '-Wextra', '-Wno-unused-parameter', ],
         'cflags_cc': [ '-fno-rtti', '-fno-exceptions', '-std=gnu++1y' ],
         'ldflags': [ '-rdynamic' ],
         'target_conditions': [
diff --git a/node.gypi b/node.gypi
index db29084335..15d8a02bcc 100644
--- a/node.gypi
+++ b/node.gypi
@@ -24,6 +24,7 @@
     },
     'force_load%': '<(force_load)',
   },
+  'cflags': [ '-Wall', '-Wextra', '-Wno-unused-parameter', ],
   'conditions': [
     [ 'node_shared=="false"', {
       'msvs_settings': {

@addaleax
Copy link
Member

@danbev
Copy link
Contributor Author

danbev commented Oct 4, 2018

Re-run of failing node-test-commit-arm.
Re-run of failing node-test-commit-windows-fanned.

@danbev
Copy link
Contributor Author

danbev commented Oct 4, 2018

Landed in f01adb5.

@danbev danbev closed this Oct 4, 2018
@danbev danbev deleted the build_icu_linux branch October 4, 2018 10:32
danbev added a commit that referenced this pull request Oct 4, 2018
This commit adds -Wno-strict-aliasing to the icu_implementation target.
The motivation for this is that this flags is enabled when building with
macosx, and will make the output a little cleaner when building on
other operating systems.

PR-URL: #23112
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Oct 5, 2018
This commit adds -Wno-strict-aliasing to the icu_implementation target.
The motivation for this is that this flags is enabled when building with
macosx, and will make the output a little cleaner when building on
other operating systems.

PR-URL: #23112
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Oct 17, 2018
This commit adds -Wno-strict-aliasing to the icu_implementation target.
The motivation for this is that this flags is enabled when building with
macosx, and will make the output a little cleaner when building on
other operating systems.

PR-URL: #23112
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
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. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants