Include icu.isolate instead of listing icudt.dll

Include icu.isolate instead of listing icudt.dll

icu.isolate file added in https://codereview.chromium.org/99073008/
is used where icudt.dll is directly listed for
'isolate_dependency_tracked'.

icu.isolate includes either icudtl.dat or icudt.dll depending on
'OS' and 'icu_use_data_file_flag'.

Besides, 'icu_use_data_file_flag' is added to build/isolate.gypi as a
config-variable.

Once the above icu-side CL is checked in, this CL will also includes
icu DEPS roll.

This is a prerequisite for the following CLs to use icudtl.dat :

Windows : https://codereview.chromium.org/99473012
Linux : https://codereview.chromium.org/102413007
Mac : https://codereview.chromium.org/109013004

BUG=72633
TEST=All tests utilizing isolate pass on bots.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245136

[jungshik at Google, 2014-01-03 19:56:06]

Can you take a look? Thanks.

https://codereview.chromium.org/124143002/diff/1/chrome/chrome.isolate
File chrome/chrome.isolate (right):

https://codereview.chromium.org/124143002/diff/1/chrome/chrome.isolate#newcode82
chrome/chrome.isolate:82: '../third_party/icu/icu.isolate',
icu.isolate has 'conditions' at the top level. I'm not sure if that 'conditions'
will obscure 'conditions' in this file. If it does, I need to do something to
prevent that, but I'm not sure how to.

[Mark Mentovai, 2014-01-03 20:17:49]

LGTM, but I also defer to Marc-Antoine for isolate particulars again.

https://codereview.chromium.org/124143002/diff/1/chrome/chrome.isolate
File chrome/chrome.isolate (right):

https://codereview.chromium.org/124143002/diff/1/chrome/chrome.isolate#newcode82
chrome/chrome.isolate:82: '../third_party/icu/icu.isolate',
Jungshik Shin wrote:
> icu.isolate has 'conditions' at the top level. I'm not sure if that
'conditions'
> will obscure 'conditions' in this file. If it does, I need to do something to
> prevent that, but I'm not sure how to. 

Shouldn’t be a problem. See https://codereview.chromium.org/99073008/#msg4 .

[jungshik at Google, 2014-01-06 18:00:35]

Thank you, Mark. 
@maruel, can you take a look? thanks.

[M-A Ruel, 2014-01-07 13:56:46]

Sorry for the delays, I was still OOO.

lgtm with one nit.

https://codereview.chromium.org/124143002/diff/1/net/net_unittests.isolate
File net/net_unittests.isolate (right):

https://codereview.chromium.org/124143002/diff/1/net/net_unittests.isolate#ne...
net/net_unittests.isolate:43: 'isolate_dependency_tracked': [
You should remove the section.

[jungshik at Google, 2014-01-07 18:55:53]

Thank you, maruel. 
I'm afraid I asked you to review prematurely. I'm looking into fixing the
following error.

https://codereview.chromium.org/124143002/diff/1/build/isolate.gypi
File build/isolate.gypi (right):

https://codereview.chromium.org/124143002/diff/1/build/isolate.gypi#newcode73
build/isolate.gypi:73: '--config-variable',
'icu_use_data_file_flag=<(icu_use_data_file_flag)',
This does not seem to work : 

I got the following error when I ran 

$ ninja -C out/Release base_unittests_run -j 100

ninja: Entering directory `out/Release'
[1/1] RULE base_unittests_run: isolate_a9b5ed4f58cc2ce951f8c7e385152c2c
base_unittests.isolate
FAILED: D:\src\cr\t.git2\src\third_party\python_26\python.exe gyp-win-tool
action-wrapper environment.x86
base_unittests_run_target_isolate_a9b5ed4f58cc2ce951f8c7e385152c2c.7eb976ef1f47d67b191dfcf55446bf98.rsp
..\..\base
[------ Swarming Error ------]
Variables in merged .isolate files do not match: ('OS',) and ('OS',
'icu_use_data_file_flag')
Traceback (most recent call last):
  File "../tools/swarming_client/isolate.py", line 2189, in main
    return dispatcher.execute(OptionParserIsolate(version=__version__), argv)
  File
"D:\src\cr\t.git2\src\tools\swarming_client\third_party\depot_tools\subcommand.py",
line 242, in execute
    return command(parser, args[1:])
  File "../tools/swarming_client/isolate.py", line 1760, in CMDcheck
    options, os.getcwd(), options.subdir, False)
  File "../tools/swarming_client/isolate.py", line 1577, in load_complete_state
    options.extra_variables, options.ignore_broken_items)
  File "../tools/swarming_client/isolate.py", line 1380, in load_isolate
    self.saved_state.config_variables)
  File "../tools/swarming_client/isolate.py", line 1024, in
load_isolate_for_config
    isolate = load_isolate_as_config(isolate_dir, eval_content(content), None)
  File "../tools/swarming_client/isolate.py", line 1010, in
load_isolate_as_config
    isolate = union(isolate, included_isolate)
  File "../tools/swarming_client/isolate.py", line 513, in union
    return lhs.union(rhs)
  File "../tools/swarming_client/isolate.py", line 857, in union
    config_variables, rhs.config_variables))
ExecutionError: Variables in merged .isolate files do not match: ('OS',) and
('OS', 'icu_use_data_file_flag')
[----------------------------]

[M-A Ruel, 2014-01-07 19:10:23]

On 2014/01/07 18:55:53, Jungshik Shin wrote:
> Thank you, maruel. 
> I'm afraid I asked you to review prematurely. I'm looking into fixing the
> following error.
> 
> https://codereview.chromium.org/124143002/diff/1/build/isolate.gypi
> File build/isolate.gypi (right):
> 
> https://codereview.chromium.org/124143002/diff/1/build/isolate.gypi#newcode73
> build/isolate.gypi:73: '--config-variable',
> 'icu_use_data_file_flag=<(icu_use_data_file_flag)',
> This does not seem to work : 
> 
> I got the following error when I ran 
> 
> $ ninja -C out/Release base_unittests_run -j 100
> 
> ninja: Entering directory `out/Release'
> [1/1] RULE base_unittests_run: isolate_a9b5ed4f58cc2ce951f8c7e385152c2c
> base_unittests.isolate
> FAILED: D:\src\cr\t.git2\src\third_party\python_26\python.exe gyp-win-tool
> action-wrapper environment.x86
>
base_unittests_run_target_isolate_a9b5ed4f58cc2ce951f8c7e385152c2c.7eb976ef1f47d67b191dfcf55446bf98.rsp
> ..\..\base
> [------ Swarming Error ------]
> Variables in merged .isolate files do not match: ('OS',) and ('OS',
> 'icu_use_data_file_flag')
> Traceback (most recent call last):
>   File "../tools/swarming_client/isolate.py", line 2189, in main
>     return dispatcher.execute(OptionParserIsolate(version=__version__), argv)
>   File
>
"D:\src\cr\t.git2\src\tools\swarming_client\third_party\depot_tools\subcommand.py",
> line 242, in execute
>     return command(parser, args[1:])
>   File "../tools/swarming_client/isolate.py", line 1760, in CMDcheck
>     options, os.getcwd(), options.subdir, False)
>   File "../tools/swarming_client/isolate.py", line 1577, in
load_complete_state
>     options.extra_variables, options.ignore_broken_items)
>   File "../tools/swarming_client/isolate.py", line 1380, in load_isolate
>     self.saved_state.config_variables)
>   File "../tools/swarming_client/isolate.py", line 1024, in
> load_isolate_for_config
>     isolate = load_isolate_as_config(isolate_dir, eval_content(content), None)
>   File "../tools/swarming_client/isolate.py", line 1010, in
> load_isolate_as_config
>     isolate = union(isolate, included_isolate)
>   File "../tools/swarming_client/isolate.py", line 513, in union
>     return lhs.union(rhs)
>   File "../tools/swarming_client/isolate.py", line 857, in union
>     config_variables, rhs.config_variables))
> ExecutionError: Variables in merged .isolate files do not match: ('OS',) and
> ('OS', 'icu_use_data_file_flag')
> [----------------------------]

It's a bug in isolate.py, I'll look at this.

[jungshik at Google, 2014-01-07 19:31:39]

On 2014/01/07 19:10:23, M-A Ruel wrote:
> On 2014/01/07 18:55:53, Jungshik Shin wrote:
> > Thank you, maruel. 
> > I'm afraid I asked you to review prematurely. I'm looking into fixing the
> > following error.
> > 
> > https://codereview.chromium.org/124143002/diff/1/build/isolate.gypi
> > File build/isolate.gypi (right):
> > 
> >
https://codereview.chromium.org/124143002/diff/1/build/isolate.gypi#newcode73
> > build/isolate.gypi:73: '--config-variable',
> > 'icu_use_data_file_flag=<(icu_use_data_file_flag)',
> > This does not seem to work : 
> > 
> > I got the following error when I ran 
> > 
> > $ ninja -C out/Release base_unittests_run -j 100
> > 
> > ninja: Entering directory `out/Release'
> > [1/1] RULE base_unittests_run: isolate_a9b5ed4f58cc2ce951f8c7e385152c2c
> > base_unittests.isolate
> > FAILED: D:\src\cr\t.git2\src\third_party\python_26\python.exe gyp-win-tool
> > action-wrapper environment.x86
> >
>
base_unittests_run_target_isolate_a9b5ed4f58cc2ce951f8c7e385152c2c.7eb976ef1f47d67b191dfcf55446bf98.rsp
> > ..\..\base
> > [------ Swarming Error ------]
> > Variables in merged .isolate files do not match: ('OS',) and ('OS',
> > 'icu_use_data_file_flag')
> > Traceback (most recent call last):
> >   File "../tools/swarming_client/isolate.py", line 2189, in main
> >     return dispatcher.execute(OptionParserIsolate(version=__version__),
argv)
> >   File
> >
>
"D:\src\cr\t.git2\src\tools\swarming_client\third_party\depot_tools\subcommand.py",
> > line 242, in execute
> >     return command(parser, args[1:])
> >   File "../tools/swarming_client/isolate.py", line 1760, in CMDcheck
> >     options, os.getcwd(), options.subdir, False)
> >   File "../tools/swarming_client/isolate.py", line 1577, in
> load_complete_state
> >     options.extra_variables, options.ignore_broken_items)
> >   File "../tools/swarming_client/isolate.py", line 1380, in load_isolate
> >     self.saved_state.config_variables)
> >   File "../tools/swarming_client/isolate.py", line 1024, in
> > load_isolate_for_config
> >     isolate = load_isolate_as_config(isolate_dir, eval_content(content),
None)
> >   File "../tools/swarming_client/isolate.py", line 1010, in
> > load_isolate_as_config
> >     isolate = union(isolate, included_isolate)
> >   File "../tools/swarming_client/isolate.py", line 513, in union
> >     return lhs.union(rhs)
> >   File "../tools/swarming_client/isolate.py", line 857, in union
> >     config_variables, rhs.config_variables))
> > ExecutionError: Variables in merged .isolate files do not match: ('OS',) and
> > ('OS', 'icu_use_data_file_flag')
> > [----------------------------]
> 
> It's a bug in isolate.py, I'll look at this.

Thanks for looking at this.

[jungshik at Google, 2014-01-09 00:55:56]

I filed https://code.google.com/p/chromium/issues/detail?id=332594 for an 
isolate.py issue.

[jungshik at Google, 2014-01-13 19:35:28]

On 2014/01/09 00:55:56, Jungshik Shin wrote:
> I filed https://code.google.com/p/chromium/issues/detail?id=332594 for an 
> isolate.py issue.

Sorry that I didn't know you filed
https://code.google.com/p/swarming/issues/detail?id=61. I just duped mine to
that.  Also, thank you for fixing it.

[jungshik at Google, 2014-01-15 00:15:38]

On 2014/01/13 19:35:28, Jungshik Shin wrote:
> On 2014/01/09 00:55:56, Jungshik Shin wrote:
> > I filed https://code.google.com/p/chromium/issues/detail?id=332594 for an 
> > isolate.py issue.
> 
> Sorry that I didn't know you filed
> https://code.google.com/p/swarming/issues/detail?id=61. I just duped mine to
> that.  Also, thank you for fixing it.

Now that swarming bug is fixed (and rolled), I'm landing this CL. Thanks, maruel
!

[commit-bot: I haz the power, 2014-01-15 01:34:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jshin@chromium.org/124143002/240001

[jungshik at Google, 2014-01-15 19:26:58]

On 2014/01/15 01:34:32, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/jshin%40chromium.org/124143002/240001

android_dbg_triggered_tests keeps failing because
build/android/pylib/gtest/setup.py has hardcoded command line options for
isolate.py. 
(http://goo.gl/ALmmOS ) 

assert os.path.exists(isolate_abs_path)
  isolate_cmd = [
      'python', _ISOLATE_SCRIPT,
      'remap',
      '--isolate', isolate_abs_path,
      '--isolated', isolated_abs_path,
      '--path-variable', 'PRODUCT_DIR', constants.GetOutDirectory(),
      '--config-variable', 'OS', 'android',
      '--outdir', constants.ISOLATE_DEPS_DIR,
  ]

I can set 'icu_use_data_file_flag' to 0 in setup.py for now (because it's 0 on
Android at the moment), but I wonder if there's a better way to get the value
passed from gyp somehow.

[M-A Ruel, 2014-01-15 20:40:46]

lgtm with nit. Thanks for fixing.

https://codereview.chromium.org/124143002/diff/630002/build/android/pylib/gte...
File build/android/pylib/gtest/setup.py (right):

https://codereview.chromium.org/124143002/diff/630002/build/android/pylib/gte...
build/android/pylib/gtest/setup.py:111: isolate_cmd = [
While at it, can you add a comment at the top stating that this needs to be kept
in sync with src/build/isolate.gypi?

https://codereview.chromium.org/124143002/diff/630002/build/isolate.gypi
File build/isolate.gypi (right):

https://codereview.chromium.org/124143002/diff/630002/build/isolate.gypi#newc...
build/isolate.gypi:70: # like '0' or '1', aren't stripped out by GYP.
While at it, can you add a comment stating that this needs to be kept in sync
with src/build/android/pylib/gtest/setup.py?

[jungshik at Google, 2014-01-15 23:13:09]

Thanks for the review. I came across another problem, which I think is trivial
to fix (0 vs '0') and I did in PS 5.

https://codereview.chromium.org/124143002/diff/720001/build/android/pylib/gte...
File build/android/pylib/gtest/setup.py (right):

https://codereview.chromium.org/124143002/diff/720001/build/android/pylib/gte...
build/android/pylib/gtest/setup.py:120: '--config-variable',
'icu_use_data_file_flag', '0',
Using 0 (bare) led to a crash (argv has to be a string). Changing it to '0'
(qted). 

 File "/b/build/slave/android/build/src/build/android/pylib/cmd_helper.py", line
43, in RunCmd
    return Call(args, cwd=cwd)
  File "/b/build/slave/android/build/src/build/android/pylib/cmd_helper.py",
line 25, in Call
    env=env)
  File "/b/build/slave/android/build/src/build/android/pylib/cmd_helper.py",
line 20, in Popen
    preexec_fn=lambda: signal.signal(signal.SIGPIPE, signal.SIG_DFL))
  File "/usr/lib/python2.7/subprocess.py", line 679, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1249, in _execute_child
    raise child_exception
TypeError: execv() arg 2 must contain only strings

[jungshik at Google, 2014-01-15 23:14:14]

https://codereview.chromium.org/124143002/diff/630002/build/android/pylib/gte...
File build/android/pylib/gtest/setup.py (right):

https://codereview.chromium.org/124143002/diff/630002/build/android/pylib/gte...
build/android/pylib/gtest/setup.py:111: isolate_cmd = [
On 2014/01/15 20:40:47, M-A Ruel wrote:
> While at it, can you add a comment at the top stating that this needs to be
kept
> in sync with src/build/isolate.gypi?

Done.

https://codereview.chromium.org/124143002/diff/630002/build/isolate.gypi
File build/isolate.gypi (right):

https://codereview.chromium.org/124143002/diff/630002/build/isolate.gypi#newc...
build/isolate.gypi:70: # like '0' or '1', aren't stripped out by GYP.
On 2014/01/15 20:40:47, M-A Ruel wrote:
> While at it, can you add a comment stating that this needs to be kept in sync
> with src/build/android/pylib/gtest/setup.py?

Done.

[commit-bot: I haz the power, 2014-01-15 23:20:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jshin@chromium.org/124143002/720001

[M-A Ruel, 2014-01-15 23:21:19]

lgtm

https://codereview.chromium.org/124143002/diff/720001/build/android/pylib/gte...
File build/android/pylib/gtest/setup.py (right):

https://codereview.chromium.org/124143002/diff/720001/build/android/pylib/gte...
build/android/pylib/gtest/setup.py:111: // This needs to be kept in sync with
the cmd line options for isolate.py
Python comments are #, not //.

https://codereview.chromium.org/124143002/diff/720001/build/android/pylib/gte...
build/android/pylib/gtest/setup.py:120: '--config-variable',
'icu_use_data_file_flag', '0',
On 2014/01/15 23:13:10, Jungshik Shin wrote:
> Using 0 (bare) led to a crash (argv has to be a string). Changing it to '0'
> (qted). 

Yep, since it is on the command line. Note that using 'icu_use_data_file_flag=0'
works too.

[jungshik at Google, 2014-01-15 23:40:42]

On 2014/01/15 23:21:19, M-A Ruel wrote:
> lgtm
> 
>
https://codereview.chromium.org/124143002/diff/720001/build/android/pylib/gte...
> File build/android/pylib/gtest/setup.py (right):
> 
>
https://codereview.chromium.org/124143002/diff/720001/build/android/pylib/gte...
> build/android/pylib/gtest/setup.py:111: // This needs to be kept in sync with
> the cmd line options for isolate.py
> Python comments are #, not //.

oops. what was I thinking? 

> 
>
https://codereview.chromium.org/124143002/diff/720001/build/android/pylib/gte...
> build/android/pylib/gtest/setup.py:120: '--config-variable',
> 'icu_use_data_file_flag', '0',
> On 2014/01/15 23:13:10, Jungshik Shin wrote:
> > Using 0 (bare) led to a crash (argv has to be a string). Changing it to '0'
> > (qted). 
> 
> Yep, since it is on the command line. Note that using
'icu_use_data_file_flag=0'
> works too.

[commit-bot: I haz the power, 2014-01-16 01:26:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jshin@chromium.org/124143002/970001

[commit-bot: I haz the power, 2014-01-16 05:22:00]

Change committed as 245136