Add support for icu_use_data_file=1 to icu.gyp on Windows

Add support for icu_use_data_file=1 to icu.gyp on Windows/Linux

1. When icu_use_data_file_flag is set to 1 on Windows, we don't need
to copy icudt.dll. Instead, we have to copy icudt*.dat.

2. Put copies statement for icudt*.dat outside link_settings on all platforms.

For static builds on Linux, Windows and Mac,
'copies' should be pulled out of 'link_settings'. Putting it outside
'link_settings' does not hurt shared builds, either.


iOS is the only platform with icu_use_data_file=1 at the moment and
is affected directly by this CL (dropping link_settings).

3. Simplify the condition for mac_bundle_resource by dropping
'OS=="mac" and _mac_bundle' because it's never satisfied here.

4. Rename icudt46l.dat in source/data/in and android to icudtl.dat. This
is to simplify the ICU upgrade process down the road.
With the version number removed, there's no need to change the data filename
in multiple gyp files and windows package configuration files when upgrading
ICU. (the same was done for icudt.dll on Windows a few years ago.)

This CL has to go first before we land the chromium-side changes
to set icu_use_data_file to 1 on
Windows (https://codereview.chromium.org/99473012 ),
Linux ( https://codereview.chromium.org/102413007 ), and
Mac ( https://codereview.chromium.org/109013004 )

The chromium-side change for Mac will follow this, too.

BUG=72633
TEST=None (until this is rolled).
R=mark@chromium.org, scottmg@chromium.org

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

[scottmg, 2013-12-16 19:23:31]

lgtm (as the question below is not specific to the Windows changes here).

https://codereview.chromium.org/111723007/diff/1/icu.gyp
File icu.gyp (right):

https://codereview.chromium.org/111723007/diff/1/icu.gyp#newcode98
icu.gyp:98: 'copies': [{
I don't understand why this 'copies' is inside of 'link_settings'?

[jungshik at Google, 2013-12-16 21:08:02]

On 2013/12/16 19:23:31, scottmg wrote:
> lgtm (as the question below is not specific to the Windows changes here).
> 
> https://codereview.chromium.org/111723007/diff/1/icu.gyp
> File icu.gyp (right):
> 
> https://codereview.chromium.org/111723007/diff/1/icu.gyp#newcode98
> icu.gyp:98: 'copies': [{
> I don't understand why this 'copies' is inside of 'link_settings'?

I don't either (I was adjusting the condition for what's already there without
paying attention to that (link_settings)). However, putting 'copies' outside
'link_settings' on Windows seems to resolve the issue I'm having in 
https://codereview.chromium.org/99473012.  Thanks !! 

I'll update the CL to put that outside link_settings at least on Windows.[1] Two
ninja output files for v8 have 'build icudt46.dat' on Windows but on Linux
(leading to a circular dependency), the same ninja files do not. 

[1] For now, I don't want to touch what's not broken (i.e. I'll not touch Linux
but add a TODO comment on that).

[jungshik at Google, 2013-12-17 01:24:31]

On 2013/12/16 21:08:02, Jungshik Shin wrote:
> On 2013/12/16 19:23:31, scottmg wrote:
> > lgtm (as the question below is not specific to the Windows changes here).
> > 
> > https://codereview.chromium.org/111723007/diff/1/icu.gyp
> > File icu.gyp (right):
> > 
> > https://codereview.chromium.org/111723007/diff/1/icu.gyp#newcode98
> > icu.gyp:98: 'copies': [{
> > I don't understand why this 'copies' is inside of 'link_settings'?
> 
> I don't either (I was adjusting the condition for what's already there without
> paying attention to that (link_settings)). However, putting 'copies' outside
> 'link_settings' on Windows seems to resolve the issue I'm having in 
> https://codereview.chromium.org/99473012.  Thanks !! 
> 
> I'll update the CL to put that outside link_settings at least on Windows.[1]
Two
> ninja output files for v8 have 'build icudt46.dat' on Windows but on Linux
> (leading to a circular dependency), the same ninja files do not. 
> 
> [1] For now, I don't want to touch what's not broken (i.e. I'll not touch
Linux
> but add a TODO comment on that).

Actually, Linux static builds require the same change (Linux shared build
doesn't care either way). 
I don't want to disturb iOS/Mac OS (I haven't managed to get 'icu*.dat' to be
used on Mac OS, yet and iOS has been using icu*.dat for a while and I'll leave
it alone here for now). 

Anyway, can you take another look?  

Mark, can you also take a look at this and two other CLs (Windows and Linux)?

[jungshik at Google, 2013-12-17 19:47:57]

Mark and Scott, 

Can you take another look? Mac OS static build (component=static_library) also
needs 'link_settings' dropped. 

I guess iOS is ok with 'link_settings' dropped around 'copied ... icudt*dat'. 
So, I went ahead and removed 'link_settings'. 

Mike, do you see any problem with this change on iOS? I don't have iOS build set
up so that I have to rely on a buildbot, which is not possible until I land this
change and make a CL to roll ICU.

[scottmg, 2013-12-17 19:59:57]

lgtm

https://codereview.chromium.org/111723007/diff/60001/icu.gyp
File icu.gyp (right):

https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode92
icu.gyp:92: 'target_conditions': [
I don't think this target_conditions block necessary? Just a regular conditions?

[Mark Mentovai, 2013-12-17 20:15:19]

https://codereview.chromium.org/111723007/diff/60001/icu.gyp
File icu.gyp (right):

https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode94
icu.gyp:94: 'mac_bundle_resources': [
This isn’t doing anything because it’s not in a mac_bundle target.

[jungshik at Google, 2013-12-17 21:18:16]

On 2013/12/17 20:15:19, Mark Mentovai wrote:
> https://codereview.chromium.org/111723007/diff/60001/icu.gyp
> File icu.gyp (right):
> 
> https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode94
> icu.gyp:94: 'mac_bundle_resources': [
> This isn’t doing anything because it’s not in a mac_bundle target.

Yes, I learned that from you. I'm pondering

[jungshik at Google, 2013-12-17 22:40:10]

https://codereview.chromium.org/111723007/diff/60001/icu.gyp
File icu.gyp (right):

https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode94
icu.gyp:94: 'mac_bundle_resources': [
On 2013/12/17 20:15:19, Mark Mentovai wrote:
> This isn’t doing anything because it’s not in a mac_bundle target.

On ios, it's still relevant, isn't it?  

I'm pondering what exactly to do handle Mac OS. Adding icu*dat to
mac_bundle_resources in chrome_dll*.gyp(i) (and other bundled target if any) is
one, but what to do here is TBD. 


Well, because 'OS=="mac" and _mac_bundle' will never be satisfied here and
dropping it does not make any difference in the actual result (either way,
there'll be an unnecessary copy of icu*dat in the top of the build directory),
I'll just change it the above condition to 'OS=="ios"'.  Then, the only question
is whether that unnecessary copy of icu*dat will be included in our shipping
binary (dmg). I guess not because only Chromium.app, Chromium
Framework.framework and Chromium Helper.app (or 'Google Chrome'-equivalent) will
be included.

[jungshik at Google, 2013-12-17 22:42:27]

On 2013/12/17 19:59:57, scottmg wrote:
> lgtm
> 
> https://codereview.chromium.org/111723007/diff/60001/icu.gyp
> File icu.gyp (right):
> 
> https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode92
> icu.gyp:92: 'target_conditions': [
> I don't think this target_conditions block necessary? Just a regular
conditions?

iOS and Android builds may need that, no?

[scottmg, 2013-12-17 23:03:48]

On 2013/12/17 22:42:27, Jungshik Shin wrote:
> On 2013/12/17 19:59:57, scottmg wrote:
> > lgtm
> > 
> > https://codereview.chromium.org/111723007/diff/60001/icu.gyp
> > File icu.gyp (right):
> > 
> > https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode92
> > icu.gyp:92: 'target_conditions': [
> > I don't think this target_conditions block necessary? Just a regular
> conditions?
> 
> iOS and Android builds may need that, no?

I don't know. I'm not really familiar with target_conditions, but from looking
at https://code.google.com/p/gyp/wiki/InputFormatReference it doesn't seem like
that sort of delayed processing is necessary. So if you believe there's some
reason, it should probably be commented here.

[jungshik at Google, 2013-12-17 23:28:17]

On 2013/12/17 23:03:48, scottmg wrote:
> On 2013/12/17 22:42:27, Jungshik Shin wrote:
> > On 2013/12/17 19:59:57, scottmg wrote:
> > > lgtm
> > > 
> > > https://codereview.chromium.org/111723007/diff/60001/icu.gyp
> > > File icu.gyp (right):
> > > 
> > > https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode92
> > > icu.gyp:92: 'target_conditions': [
> > > I don't think this target_conditions block necessary? Just a regular
> > conditions?
> > 
> > iOS and Android builds may need that, no?
> 
> I don't know. I'm not really familiar with target_conditions, but from looking
> at https://code.google.com/p/gyp/wiki/InputFormatReference it doesn't seem
like
> that sort of delayed processing is necessary. So if you believe there's some
> reason, it should probably be commented here.

Ahah.. Thank you for the pointer. I totally misunderstood it. I guess
target_conditions was used with an intention to add icud*dat to
mac_resource_bundle *only* when 'mac_bundle' is set to 1 in target for which
icu.gyp is included (directly or indirectly?). With 'OS="mac" and _mac_bundle'
removed, it appears unnecessary. 

However, if it works as intended, it'd do exactly what I wanted to do.
Unfortunately, it didn't work that way. 

Mark wrote, "none of the targets in icu.gyp are themselves mac_bundle. The
chrome_dll target is, and that’s the bundle where you’d need to get the ICU data
copied." ( https://code.google.com/p/chromium/issues/detail?id=72633#c41 )

[scottmg, 2013-12-17 23:49:27]

On 2013/12/17 23:28:17, Jungshik Shin wrote:
> On 2013/12/17 23:03:48, scottmg wrote:
> > On 2013/12/17 22:42:27, Jungshik Shin wrote:
> > > On 2013/12/17 19:59:57, scottmg wrote:
> > > > lgtm
> > > > 
> > > > https://codereview.chromium.org/111723007/diff/60001/icu.gyp
> > > > File icu.gyp (right):
> > > > 
> > > > https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode92
> > > > icu.gyp:92: 'target_conditions': [
> > > > I don't think this target_conditions block necessary? Just a regular
> > > conditions?
> > > 
> > > iOS and Android builds may need that, no?
> > 
> > I don't know. I'm not really familiar with target_conditions, but from
looking
> > at https://code.google.com/p/gyp/wiki/InputFormatReference it doesn't seem
> like
> > that sort of delayed processing is necessary. So if you believe there's some
> > reason, it should probably be commented here.
> 
> Ahah.. Thank you for the pointer. I totally misunderstood it. I guess
> target_conditions was used with an intention to add icud*dat to
> mac_resource_bundle *only* when 'mac_bundle' is set to 1 in target for which
> icu.gyp is included (directly or indirectly?). With 'OS="mac" and _mac_bundle'
> removed, it appears unnecessary. 
> 
> However, if it works as intended, it'd do exactly what I wanted to do.
> Unfortunately, it didn't work that way. 
> 
> Mark wrote, "none of the targets in icu.gyp are themselves mac_bundle. The
> chrome_dll target is, and that’s the bundle where you’d need to get the ICU
data
> copied." ( https://code.google.com/p/chromium/issues/detail?id=72633#c41 )

Sorry, I might have misunderstood.

The goal is to get targets that depend on 'icudata' to have that data file
copied to <(PRODUCT_DIR), right?

I guess target_conditions is an attempt to defer the evaluation of PRODUCT_DIR,
and push it into the one that depends on this one, so that it's copied to the
PRODUCT_DIR of chrome_dll, not PRODUCT_DIR of icudata. Is that right? I don't
know if it works like that, but I would be somewhat surprised if
target_conditions wasn't scoped to the target/file where it was used.

I think the normal way to "export" functionality like this would be to have an
icudata.gypi file that targets that want this functionality would add to an
'includes' block. This block would have the copies block or bundle.

If you're still not sure by testing with some trybots, I would suggest
describing the problem to gyp-developer@ as someone there will probably be more
able to help.

[jungshik at Google, 2013-12-18 00:42:09]

On 2013/12/17 23:49:27, scottmg wrote:
> On 2013/12/17 23:28:17, Jungshik Shin wrote:
> > On 2013/12/17 23:03:48, scottmg wrote:
> > > On 2013/12/17 22:42:27, Jungshik Shin wrote:
> > > > On 2013/12/17 19:59:57, scottmg wrote:
> > > > > lgtm
> > > > > 
> > > > > https://codereview.chromium.org/111723007/diff/60001/icu.gyp
> > > > > File icu.gyp (right):
> > > > > 
> > > > > https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode92
> > > > > icu.gyp:92: 'target_conditions': [
> > > > > I don't think this target_conditions block necessary? Just a regular
> > > > conditions?
> > > > 
> > > > iOS and Android builds may need that, no?
> > > 
> > > I don't know. I'm not really familiar with target_conditions, but from
> looking
> > > at https://code.google.com/p/gyp/wiki/InputFormatReference it doesn't seem
> > like
> > > that sort of delayed processing is necessary. So if you believe there's
some
> > > reason, it should probably be commented here.
> > 
> > Ahah.. Thank you for the pointer. I totally misunderstood it. I guess
> > target_conditions was used with an intention to add icud*dat to
> > mac_resource_bundle *only* when 'mac_bundle' is set to 1 in target for which
> > icu.gyp is included (directly or indirectly?). With 'OS="mac" and
_mac_bundle'
> > removed, it appears unnecessary. 
> > 
> > However, if it works as intended, it'd do exactly what I wanted to do.
> > Unfortunately, it didn't work that way. 
> > 
> > Mark wrote, "none of the targets in icu.gyp are themselves mac_bundle. The
> > chrome_dll target is, and that’s the bundle where you’d need to get the ICU
> data
> > copied." ( https://code.google.com/p/chromium/issues/detail?id=72633#c41 )
> 
> Sorry, I might have misunderstood.
> 
> The goal is to get targets that depend on 'icudata' to have that data file
> copied to <(PRODUCT_DIR), right?
> 
> I guess target_conditions is an attempt to defer the evaluation of
PRODUCT_DIR,
> and push it into the one that depends on this one, so that it's copied to the
> PRODUCT_DIR of chrome_dll, not PRODUCT_DIR of icudata. Is that right? I don't
> know if it works like that, but I would be somewhat surprised if
> target_conditions wasn't scoped to the target/file where it was used.

I guess you're right. Although on Mac (unbundled binary - e.g. base_unittests)
and Linux, 'conditions' or 'target_condtions' does not
seem to make any difference when it comes to where 'icudt*dat' is copied. (in
both cases, it's copied to the top of the build directory). 
I haven't tested it on Windows, yet. 

 
> I think the normal way to "export" functionality like this would be to have an
> icudata.gypi file that targets that want this functionality would add to an
> 'includes' block. This block would have the copies block or bundle.

I also suspect that that is an usual (or potentially cleaner) way, but I'd
rather defer that change for the future. 

 
> If you're still not sure by testing with some trybots, 

I can't test with trybots without landing a CL for third_party/icu and then
making a CL to roll icu to the version. So, I've been testing on multiple
platforms (linux, mac, windows) myself. I don't have a build set up for iOS and
Android. Therefore, I like to avoid a change affecting them if possible for the
moment. 

> I would suggest
> describing the problem to gyp-developer@ as someone there will probably be
more
> able to help.

I solved almost all the problems on Mac except that 1) a redundant copy of
icudt*.dat is added to the top of the build directory when building bundled mac
targets (e.g. Google Chrome/Chromium or Content Shell) 2) I have to add
icudt*dat to the bundle list in a couple of gypi files for bundled mac targets.

[jungshik at Google, 2013-12-18 00:47:36]

On 2013/12/17 23:49:27, scottmg wrote:
> On 2013/12/17 23:28:17, Jungshik Shin wrote:
> > On 2013/12/17 23:03:48, scottmg wrote:
> > > On 2013/12/17 22:42:27, Jungshik Shin wrote:
> > > > On 2013/12/17 19:59:57, scottmg wrote:
> > > > > lgtm
> > > > > 
> > > > > https://codereview.chromium.org/111723007/diff/60001/icu.gyp
> > > > > File icu.gyp (right):
> > > > > 
> > > > > https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode92
> > > > > icu.gyp:92: 'target_conditions': [
> > > > > I don't think this target_conditions block necessary? Just a regular
> > > > conditions?
> > > > 
> > > > iOS and Android builds may need that, no?
> > > 
> > > I don't know. I'm not really familiar with target_conditions, but from
> looking
> > > at https://code.google.com/p/gyp/wiki/InputFormatReference it doesn't seem
> > like
> > > that sort of delayed processing is necessary. So if you believe there's
some
> > > reason, it should probably be commented here.
> > 
> > Ahah.. Thank you for the pointer. I totally misunderstood it. I guess
> > target_conditions was used with an intention to add icud*dat to
> > mac_resource_bundle *only* when 'mac_bundle' is set to 1 in target for which
> > icu.gyp is included (directly or indirectly?). With 'OS="mac" and
_mac_bundle'
> > removed, it appears unnecessary. 
> > 
> > However, if it works as intended, it'd do exactly what I wanted to do.
> > Unfortunately, it didn't work that way. 
> > 
> > Mark wrote, "none of the targets in icu.gyp are themselves mac_bundle. The
> > chrome_dll target is, and that’s the bundle where you’d need to get the ICU
> data
> > copied." ( https://code.google.com/p/chromium/issues/detail?id=72633#c41 )
> 
> Sorry, I might have misunderstood.
> 
> The goal is to get targets that depend on 'icudata' to have that data file
> copied to <(PRODUCT_DIR), right?
> 
> I guess target_conditions is an attempt to defer the evaluation of
PRODUCT_DIR,
> and push it into the one that depends on this one, so that it's copied to the
> PRODUCT_DIR of chrome_dll, not PRODUCT_DIR of icudata. Is that right? I don't
> know if it works like that, but I would be somewhat surprised if
> target_conditions wasn't scoped to the target/file where it was used.

Am I right to interpret the above as saying that 'target_conditions' has to be
kept instead of being replaced by 'conditions'?  That's what I like to do here
because I don't want to venture to affect platforms I can't test right now
(Android in this case. iOS is less likely to be affected, I suspect).

[scottmg, 2013-12-18 00:49:57]

On 2013/12/18 00:47:36, Jungshik Shin wrote:
> On 2013/12/17 23:49:27, scottmg wrote:
> > On 2013/12/17 23:28:17, Jungshik Shin wrote:
> > > On 2013/12/17 23:03:48, scottmg wrote:
> > > > On 2013/12/17 22:42:27, Jungshik Shin wrote:
> > > > > On 2013/12/17 19:59:57, scottmg wrote:
> > > > > > lgtm
> > > > > > 
> > > > > > https://codereview.chromium.org/111723007/diff/60001/icu.gyp
> > > > > > File icu.gyp (right):
> > > > > > 
> > > > > >
https://codereview.chromium.org/111723007/diff/60001/icu.gyp#newcode92
> > > > > > icu.gyp:92: 'target_conditions': [
> > > > > > I don't think this target_conditions block necessary? Just a regular
> > > > > conditions?
> > > > > 
> > > > > iOS and Android builds may need that, no?
> > > > 
> > > > I don't know. I'm not really familiar with target_conditions, but from
> > looking
> > > > at https://code.google.com/p/gyp/wiki/InputFormatReference it doesn't
seem
> > > like
> > > > that sort of delayed processing is necessary. So if you believe there's
> some
> > > > reason, it should probably be commented here.
> > > 
> > > Ahah.. Thank you for the pointer. I totally misunderstood it. I guess
> > > target_conditions was used with an intention to add icud*dat to
> > > mac_resource_bundle *only* when 'mac_bundle' is set to 1 in target for
which
> > > icu.gyp is included (directly or indirectly?). With 'OS="mac" and
> _mac_bundle'
> > > removed, it appears unnecessary. 
> > > 
> > > However, if it works as intended, it'd do exactly what I wanted to do.
> > > Unfortunately, it didn't work that way. 
> > > 
> > > Mark wrote, "none of the targets in icu.gyp are themselves mac_bundle. The
> > > chrome_dll target is, and that’s the bundle where you’d need to get the
ICU
> > data
> > > copied." ( https://code.google.com/p/chromium/issues/detail?id=72633#c41 )
> > 
> > Sorry, I might have misunderstood.
> > 
> > The goal is to get targets that depend on 'icudata' to have that data file
> > copied to <(PRODUCT_DIR), right?
> > 
> > I guess target_conditions is an attempt to defer the evaluation of
> PRODUCT_DIR,
> > and push it into the one that depends on this one, so that it's copied to
the
> > PRODUCT_DIR of chrome_dll, not PRODUCT_DIR of icudata. Is that right? I
don't
> > know if it works like that, but I would be somewhat surprised if
> > target_conditions wasn't scoped to the target/file where it was used.
> 
> Am I right to interpret the above as saying that 'target_conditions' has to be
> kept instead of being replaced by 'conditions'?  That's what I like to do here
> because I don't want to venture to affect platforms I can't test right now
> (Android in this case. iOS is less likely to be affected, I suspect).

I think it should just be conditions. I don't know of any reason it should be
target_conditions. Maybe you'll find one if something breaks. :)

[Mark Mentovai, 2013-12-18 17:03:03]

LGTM with this one change.

I haven’t looked at your Chrome-side Mac CL yet. I assume because you haven’t
sent it out for review that you still consider it a work in progress and aren’t
ready for me to look.

https://codereview.chromium.org/111723007/diff/100001/icu.gyp
File icu.gyp (right):

https://codereview.chromium.org/111723007/diff/100001/icu.gyp#newcode92
icu.gyp:92: 'target_conditions': [
conditions should be fine here. The OS won’t change inside of a target.

Sometimes, target_conditions is used not because a variable is only defined
within a target, or differently-defined within a target, but because someone’s
trying to influence the order of GYP operations, since target_conditions are
processed after regular conditions. That’s not the case here.

[jungshik at Google, 2013-12-18 18:03:02]

On 2013/12/18 17:03:03, Mark Mentovai wrote:
> LGTM with this one change.
> 
> I haven’t looked at your Chrome-side Mac CL yet. I assume because you haven’t
> sent it out for review that you still consider it a work in progress and
aren’t
> ready for me to look.
> 
> https://codereview.chromium.org/111723007/diff/100001/icu.gyp
> File icu.gyp (right):
> 
> https://codereview.chromium.org/111723007/diff/100001/icu.gyp#newcode92
> icu.gyp:92: 'target_conditions': [
> conditions should be fine here. The OS won’t change inside of a target.
> 
> Sometimes, target_conditions is used not because a variable is only defined
> within a target, or differently-defined within a target, but because someone’s
> trying to influence the order of GYP operations, since target_conditions are
> processed after regular conditions. That’s not the case here.

Thank you, Mark. I'm going to land with 'conditions' instead of
'target_conditions'. I still have an issue with a Mac CL. Can you take a look
and 'enlighten' me (I just published it). Thanks again.

[jungshik at Google, 2013-12-18 18:03:36]

Committed patchset #7 manually as r241597 (presubmit successful).

[jungshik at Google, 2013-12-18 21:33:36]

Mark and Pinkerton, do you have a idea as to ios_debug_simulator failure
mentioned below? 
I'm trying ios_rel_device trybot as well.

https://codereview.chromium.org/111723007/diff/120001/icu.gyp
File icu.gyp (right):

https://codereview.chromium.org/111723007/diff/120001/icu.gyp#newcode97
icu.gyp:97: }, {
Trybot failed for a CL at https://codereview.chromium.org/118313004/ that rolls
icu to this version. It's ios_debug_simulator. base_unittests or net_unittests
failed to find icudtl.dat.  See
http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...

I wonder which of two changes, removing link_settings and replacing
target_conditions with conditions is to blame for the failure.

[Mark Mentovai, 2013-12-18 21:58:27]

https://codereview.chromium.org/111723007/diff/120001/icu.gyp
File icu.gyp (right):

https://codereview.chromium.org/111723007/diff/120001/icu.gyp#newcode94
icu.gyp:94: 'mac_bundle_resources': [
This is a static_library, but a static_library can’t be a mac_bundle, so this is
meaningless. The file won’t be copied anywhere.

[jungshik at Google, 2013-12-18 22:06:13]

https://codereview.chromium.org/111723007/diff/120001/icu.gyp
File icu.gyp (right):

https://codereview.chromium.org/111723007/diff/120001/icu.gyp#newcode94
icu.gyp:94: 'mac_bundle_resources': [
On 2013/12/18 21:58:28, Mark Mentovai wrote:
> This is a static_library, but a static_library can’t be a mac_bundle, so this
is
> meaningless. The file won’t be copied anywhere.

Hmm.. Then, I wonder how iOS could have worked so far? iOS has had
icu_use_data_file_flag set to 1 for a long while (before I did anything).  In
this CL, I removed link_settings and replaced 'target_conditions' with
'conditions'.

[Mark Mentovai, 2013-12-18 22:07:40]

The link_settings is what made it work. That would push the mac_bundle_settings
down to the dependent shared_library or executable, which would be a mac_bundle,
and would get the file copied into its Resources directory.

[jungshik at Google, 2013-12-18 22:10:27]

On 2013/12/18 22:07:40, Mark Mentovai wrote:
> The link_settings is what made it work. That would push the
mac_bundle_settings
> down to the dependent shared_library or executable, which would be a
mac_bundle,
> and would get the file copied into its Resources directory.

Thanks a lot. In that case, I'll use link_settings only on iOS because
link_settings break component=static_library builds elsewhere.

[Mark Mentovai, 2013-12-18 22:10:36]

My advice:

Keep link_settings for iOS where it’s needed to make mac_bundle_resources work.
Leave the copies section outside of link_settings for other platforms, because
you only need to get the copy to happen once in this target, not once in each
linkable dependent (shared_library or executable), when you’re just copying to
<(PRODUCT_DIR). You don’t need link_settings there because <(PRODUCT_DIR) is the
same for all targets including this one.

Sorry I didn’t catch this during review.

[jungshik at Google, 2013-12-18 23:09:04]

On 2013/12/18 22:10:36, Mark Mentovai wrote:
> My advice:
> 
> Keep link_settings for iOS where it’s needed to make mac_bundle_resources
work.
> Leave the copies section outside of link_settings for other platforms, because
> you only need to get the copy to happen once in this target, not once in each
> linkable dependent (shared_library or executable), when you’re just copying to
> <(PRODUCT_DIR). You don’t need link_settings there because <(PRODUCT_DIR) is
the
> same for all targets including this one.
> 
> Sorry I didn’t catch this during review.

Don't worry. Thanks a lot for the help. A new CL to take care of iOS is up at
https://codereview.chromium.org/118373006/