Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(84)

Issue 348183002: Add GN build file for third_party hunspell library. (Closed)

Created:
6 years, 6 months ago by tfarina
Modified:
6 years, 6 months ago
Reviewers:
brettw
CC:
chromium-reviews
Visibility:
Public.

Description

Add GN build file for third_party hunspell library. This way we can hook it up in the GN build on Chromium and it is blocking chrome/renderer. BUG=None TEST=gn gen out/Debug_gn && ninja -C out/Debug_gn hunspell R=brettw@chromium.org Committed: 278884

Patch Set 1 #

Patch Set 2 : stringprintf.h fix #

Patch Set 3 : cflags #

Total comments: 7

Patch Set 4 : review #

Patch Set 5 : REBASE on top of origin/master #

Unified diffs Side-by-side diffs Delta from patch set Stats (+101 lines, -0 lines) Patch
A BUILD.gn View 1 2 3 1 chunk +101 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
tfarina
6 years, 6 months ago (2014-06-21 00:43:06 UTC) #1
brettw
lgtm https://codereview.chromium.org/348183002/diff/40001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/348183002/diff/40001/BUILD.gn#newcode58 BUILD.gn:58: defines = [ You can delete this, direct_dependent_configs ...
6 years, 6 months ago (2014-06-21 00:45:34 UTC) #2
tfarina
https://codereview.chromium.org/348183002/diff/40001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/348183002/diff/40001/BUILD.gn#newcode58 BUILD.gn:58: defines = [ On 2014/06/21 00:45:34, brettw wrote: > ...
6 years, 6 months ago (2014-06-21 00:53:51 UTC) #3
tfarina
Committed patchset #5 manually as r278884 (presubmit successful).
6 years, 6 months ago (2014-06-21 01:05:23 UTC) #4
tfarina
6 years, 6 months ago (2014-06-21 01:56:43 UTC) #5
Message was sent while issue was closed.
https://codereview.chromium.org/348183002/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/348183002/diff/40001/BUILD.gn#newcode86
BUILD.gn:86: if (is_posix && !is_mac && !is_ios) { # gcc_version >= 48
On 2014/06/21 00:45:34, brettw wrote:
> I'd do is_linux for this (I think this is what this condition is going for).
And
> I think we require gcc 4.8 now so you can delete the comment.

Looks like it would have been better if we had went with is_posix instead:

In file included from ../../third_party/hunspell/src/hunspell/affentry.cxx:9:0:
../../third_party/hunspell/src/hunspell/affentry.hxx:30:105: error: converting
to non-pointer type 'short unsigned int' from NULL [-Werror=conversion-null]
   struct hentry *      check_twosfx(const char * word, int len, char
in_compound, const FLAG needflag = NULL);

http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...

Powered by Google App Engine
This is Rietveld 408576698