Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2181043003:
Fix GN build on mac with icu_use_data_file = false (Closed)
|
Created:
4 years, 5 months ago by jungshik at Google Modified:
4 years, 4 months ago CC:
chromium-reviews, mlamouri+watch-content_chromium.org, chromoting-reviews_chromium.org, Peter Beverloo, jam, darin-cc_chromium.org, jochen+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix GN build on mac with icu_use_data_file = false
Several BUILD.gn files were bundling icudtl.dat even when
icu_use_data_file = false (i.e. icudata is statically linked).
The TEST below was done with https://codereview.chromium.org/2174993002/ on
the ICU side. This CL still can get in without that change, though.
BUG=630929
TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files.
TEST=chrome / content_shell can be built.
TEST=In Chrome/content_shell, go to a non-UTF-8 page (www.hankyung.com) or
run `(new Date()).toLocaleString("de")` to make sure that ICU data
is accessible.
Committed: https://crrev.com/359df9c9f207bac75f1387250af4f30a88139dea
Cr-Commit-Position: refs/heads/master@{#408866}
Patch Set 1 #
Total comments: 4
Patch Set 2 : move (public)_deps inside if-icu_use_data_file #
Messages
Total messages: 27 (13 generated)
rsesek@chromium.org changed reviewers: + rsesek@chromium.org
https://codereview.chromium.org/2181043003/diff/1/chrome/BUILD.gn File chrome/BUILD.gn (right): https://codereview.chromium.org/2181043003/diff/1/chrome/BUILD.gn#newcode763 chrome/BUILD.gn:763: I'd move the sources += down here and then also remove line 740 and make it conditionally add public_deps += [ "//third_party/icu:icudata" ]. https://codereview.chromium.org/2181043003/diff/1/content/shell/BUILD.gn File content/shell/BUILD.gn (right): https://codereview.chromium.org/2181043003/diff/1/content/shell/BUILD.gn#newc... content/shell/BUILD.gn:590: Same.
Thank you, Robert. I updated the CL. I also changed remoting/host/BUILD.gn. PTAL. Thanks https://codereview.chromium.org/2181043003/diff/1/chrome/BUILD.gn File chrome/BUILD.gn (right): https://codereview.chromium.org/2181043003/diff/1/chrome/BUILD.gn#newcode763 chrome/BUILD.gn:763: On 2016/07/25 22:33:44, Robert Sesek wrote: > I'd move the sources += down here and then also remove line 740 and make it > conditionally add public_deps += [ "//third_party/icu:icudata" ]. Thanks. Done https://codereview.chromium.org/2181043003/diff/1/content/shell/BUILD.gn File content/shell/BUILD.gn (right): https://codereview.chromium.org/2181043003/diff/1/content/shell/BUILD.gn#newc... content/shell/BUILD.gn:590: On 2016/07/25 22:33:44, Robert Sesek wrote: > Same. Done.
Description was changed from ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome can be built. ========== to ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). The TEST below was done with https://codereview.chromium.org/2174993002/ on the ICU side. This CL still can get in without that change, though. BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome can be built. ==========
Description was changed from ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). The TEST below was done with https://codereview.chromium.org/2174993002/ on the ICU side. This CL still can get in without that change, though. BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome can be built. ========== to ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). The TEST below was done with https://codereview.chromium.org/2174993002/ on the ICU side. This CL still can get in without that change, though. BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome / content_shell can be built. ==========
LGTM
Description was changed from ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). The TEST below was done with https://codereview.chromium.org/2174993002/ on the ICU side. This CL still can get in without that change, though. BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome / content_shell can be built. ========== to ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). The TEST below was done with https://codereview.chromium.org/2174993002/ on the ICU side. This CL still can get in without that change, though. BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome / content_shell can be built. ==========
Description was changed from ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). The TEST below was done with https://codereview.chromium.org/2174993002/ on the ICU side. This CL still can get in without that change, though. BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome / content_shell can be built. ========== to ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). The TEST below was done with https://codereview.chromium.org/2174993002/ on the ICU side. This CL still can get in without that change, though. BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome / content_shell can be built. TEST=In Chrome/content_shell, go to a non-UTF-8 page (www.hankyung.com) or run `(new Date()).toLocaleString("de")` to make sure that ICU data is accessible. ==========
The CQ bit was checked by jshin@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
jshin@chromium.org changed reviewers: + dpranke@chromium.org
dpranke@ : can you approve? Thanks
dpranke@chromium.org changed reviewers: + brettw@chromium.org
lgtm, but I'm not an owner for these directories. @brettw, can you stamp this?
On 2016/07/26 22:08:01, Dirk Pranke wrote: > lgtm, but I'm not an owner for these directories. > > @brettw, can you stamp this? Thanks. I was misled by chrome/OWNERS pointing to //build/OWNERS for BUILD.gn (I also overlooked a comment that effectively reads " For structural changes, get an approval from chrome/OWNERS"). I'll ping brettw.
lgtm
The CQ bit was checked by jshin@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by jshin@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). The TEST below was done with https://codereview.chromium.org/2174993002/ on the ICU side. This CL still can get in without that change, though. BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome / content_shell can be built. TEST=In Chrome/content_shell, go to a non-UTF-8 page (www.hankyung.com) or run `(new Date()).toLocaleString("de")` to make sure that ICU data is accessible. ========== to ========== Fix GN build on mac with icu_use_data_file = false Several BUILD.gn files were bundling icudtl.dat even when icu_use_data_file = false (i.e. icudata is statically linked). The TEST below was done with https://codereview.chromium.org/2174993002/ on the ICU side. This CL still can get in without that change, though. BUG=630929 TEST='gn args <outdir>' with icu_use_data_file=false generates ninja files. TEST=chrome / content_shell can be built. TEST=In Chrome/content_shell, go to a non-UTF-8 page (www.hankyung.com) or run `(new Date()).toLocaleString("de")` to make sure that ICU data is accessible. Committed: https://crrev.com/359df9c9f207bac75f1387250af4f30a88139dea Cr-Commit-Position: refs/heads/master@{#408866} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/359df9c9f207bac75f1387250af4f30a88139dea Cr-Commit-Position: refs/heads/master@{#408866} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
