New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 630929 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

icu_use_data_file = false: ninja generation failure

Project Member Reported by js...@chromium.org, Jul 25 2016

Issue description

On Mac, when 'gn args out/<bld dir>' run with 'icu_use_data_file = false' fails with the following error:

$ gn args out/rel
Waiting for editor on "/Volumes/Works/cr/t/src/out/rel/args.gn"...
Generating files...
ERROR Input to targets not generated by a dependency.
The file:
  //out/rel/icudtl.dat
is listed as an input or source for the targets:
  //remoting/host:remoting_host_resources
  //remoting/host/it2me:remote_assistance_host_resources
  //chrome:chrome_framework_resources
  //content/shell:content_shell_framework_resources
  //remoting/host:remoting_me2me_host
  //remoting/host/it2me:remote_assistance_host
  //content/shell:content_shell_framework
  //chrome:chrome_framework
but no targets in the build generate that file.

If you have generated inputs, there needs to be a dependency path between the
two targets in addition to just listing the files. For indirect dependencies,
the intermediate ones must be public_deps. data_deps don't count since they're
only runtime dependencies. If you think a dependency chain exists, it might be
because the chain is private. Try "gn path" to analyze.
---------------



BTW, here's the output from 'gn path' 
$ gn path out/rel //chrome:chrome_framework_resources   //third_party/icu:icudata --all
//chrome:chrome_framework_resources --[public]-->
//third_party/icu:icudata

//chrome:chrome_framework_resources --[public]-->
//third_party/icu:icudata

//chrome:chrome_framework_resources --[public]-->
//v8:v8 --[public]-->
//v8:v8_base --[private]-->
//third_party/icu:icu --[public]-->
//third_party/icu:icuuc --[private]-->
//third_party/icu:icudata

//chrome:chrome_framework_resources --[public]-->
//v8:v8 --[public]-->
//v8:v8_maybe_snapshot --[public]-->
//v8:v8_external_snapshot --[public]-->
//v8:run_mksnapshot --[private]-->
//v8:mksnapshot --[private]-->
//v8:v8_base --> see private chain printed above...

//chrome:chrome_framework_resources --[public]-->
//v8:v8 --[public]-->
//v8:v8_maybe_snapshot --[public]-->
//v8:v8_external_snapshot --[public]-->
//v8:run_mksnapshot --[private]-->
//v8:mksnapshot --[private]-->
//v8:v8_nosnapshot --[private]-->
//v8:v8_base --> see private chain printed above...
---------------------------------------------------------

The issue is that the targets listed above (for gn args ) list 'icudtl.dat' as 'source' unconditionally. When I tried to make it conditional upon 'icu_use_data_file', GN complained that the variable icu_use_data_file is not declared. 


Some users of third_party/icu (e.g. some embeddeers of v8, and some builds of Windows. And, Android webview is another customer IIRC ) may still want to keep that option viable so that it has to be kept in BUILD.gn for third_party/icu. 

Mac may not need that option. (I haven't tried that option on Mac for a long while even with gyp. I'm trying it now). 

 

Comment 1 by js...@chromium.org, Jul 25 2016

This issue was found while testing https://codereview.chromium.org/2174993002/

Comment 2 by js...@chromium.org, Jul 25 2016

Cc: altimin@chromium.org
Related bug:  610673  (headless build wants to link icudata statically). 

On Mac, base_unittests is built fine but chrome doesn't get built (gyp) because icu_use_data_file_flag=0 does not lead ICU_UTIL_DATA_IMPL to be set to ICU_UTIL_DATA_STATIC when compiling some files (e.g. services/shell/runner/init.cc ) 

With GN, I can't even get there because of the issue in comment 0. 

>> When I tried to make it conditional upon 'icu_use_data_file', GN complained that the variable icu_use_data_file is not declared

icu_use_data_file is defined in //third_party/icu/config.gni, so you will need to have "import("//third_party/icu/config.gni")" line in the gn files using it.

Comment 4 by rsesek@chromium.org, Jul 25 2016

I see no reason why icu_use_data_file=false shouldn't work on Mac, but in practice we don't ship it and it may rot if nothing actually builds that configuration.

Comment 5 by js...@chromium.org, Jul 25 2016

Owner: js...@chromium.org
Status: Started (was: Untriaged)
Thanks a lot, altimin !  That worked. ( https://codereview.chromium.org/2181043003 ) 

rsesek@: in  bug 610673 , altimin wrote that 'headless' project would support Mac after Linux is done. Anyway, his suggestion worked. 
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/359df9c9f207bac75f1387250af4f30a88139dea

commit 359df9c9f207bac75f1387250af4f30a88139dea
Author: jshin <jshin@chromium.org>
Date: Sat Jul 30 06:59:49 2016

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.

Review-Url: https://codereview.chromium.org/2181043003
Cr-Commit-Position: refs/heads/master@{#408866}

[modify] https://crrev.com/359df9c9f207bac75f1387250af4f30a88139dea/chrome/BUILD.gn
[modify] https://crrev.com/359df9c9f207bac75f1387250af4f30a88139dea/content/shell/BUILD.gn
[modify] https://crrev.com/359df9c9f207bac75f1387250af4f30a88139dea/remoting/host/BUILD.gn
[modify] https://crrev.com/359df9c9f207bac75f1387250af4f30a88139dea/remoting/host/it2me/BUILD.gn

Comment 7 by js...@chromium.org, Aug 4 2016

Status: Fixed (was: Started)

Sign in to add a comment