Add bundle_deps_filter to create_bundle targets.

Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to the
create_bundle target used to filter targets when recursively
resolving the deps. Any target matching one of the labels in
bundle_deps_filter is not added as a dependency.

BUG=615058
Committed: https://crrev.com/7ac497647684c5e90020f2917b2d849b9056826a
Cr-Commit-Position: refs/heads/master@{#410281}

[sdefresne, 2016-08-03 00:30:53]

The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run

[sdefresne, 2016-08-03 00:31:05]

sdefresne@chromium.org changed reviewers:
+ brettw@chromium.org

[sdefresne, 2016-08-03 00:31:06]

Please take a look.

[commit-bot: I haz the power, 2016-08-03 00:32:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 01:25:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-03 01:25:04]

Dry run: This issue passed the CQ dry run.

[brettw, 2016-08-03 19:25:20]

I still don't quite understand how this will be used. Can you give an explicit
example?

https://codereview.chromium.org/2205693005/diff/1/tools/gn/bundle_data.h
File tools/gn/bundle_data.h (right):

https://codereview.chromium.org/2205693005/diff/1/tools/gn/bundle_data.h#newc...
tools/gn/bundle_data.h:135: std::vector<LabelPattern> deps_filter_;
I suggested renaming the variable below, and this and the getters above should
be changed to match.

https://codereview.chromium.org/2205693005/diff/1/tools/gn/variables.cc
File tools/gn/variables.cc (right):

https://codereview.chromium.org/2205693005/diff/1/tools/gn/variables.cc#newco...
tools/gn/variables.cc:973: "deps_filter: [label list] A list of labels that must
not be linked.\n"
Your implementation takes a list of label patterns, the documentation should
make that clearn.

https://codereview.chromium.org/2205693005/diff/1/tools/gn/variables.cc#newco...
tools/gn/variables.cc:977: "  Specifies target that must be ignored when copying
data into a bundle.\n"
Can you fix the grammar here? I think you mean "specifies targets" and "It is
usually used...". Can you also expand on this paragraph quite a bit and supply
some background for people that aren't familiar with ios bundles?

https://codereview.chromium.org/2205693005/diff/1/tools/gn/variables.h
File tools/gn/variables.h (right):

https://codereview.chromium.org/2205693005/diff/1/tools/gn/variables.h#newcod...
tools/gn/variables.h:194: extern const char kDepsFilter[];
This name implies more general behavior that what you're doing. Perhaps
"bundle_deps_filter" or something would make it more clear that this only
applies to the deps for bundle stuff?

[sdefresne, 2016-08-03 19:52:13]

You can find some example here (sorry downstream only CL):
https://chromereviews.googleplex.com/482907013/

The idea is that the two application extension shipped with Chrome on iOS uses
base, thus depends on //third_party/icu:icudata and ends up copying the
icudtl.dat file in both the application extension and the application bundle,
but the application extension is using the icudtl.dat file from the application
bundle, so it is unnecessary to copy the file into the application extension
bundle.

For most of the file, this is not a problem, but icudtl.dat is 15Mb large, so 
not copying it to the application extension save 30Mb from the application
bundle (since the application bundle contains the application extension bundle).

Basically, we want to avoid finding three files when running the following
command:

$  find Chromium.app -name icudtl.dat
Chromium.app/icudtl.dat
Chromium.app/PlugIns/share_extension.appex/icudtl.dat
Chromium.app/PlugIns/today_extension.appex/icudtl.dat

So we would do the following in when defining the share_extension.appex target:

ios_appex_bundle("share_extension") {
  bundle_deps_filter = [ "//third_party/icu:icudata" ]
}

[sdefresne, 2016-08-03 20:48:43]

The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-03 20:49:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sdefresne, 2016-08-03 20:51:22]

Please take another look.

https://codereview.chromium.org/2205693005/diff/20001/tools/gn/command_gen.cc
File tools/gn/command_gen.cc (right):

https://codereview.chromium.org/2205693005/diff/20001/tools/gn/command_gen.cc...
tools/gn/command_gen.cc:351: "  script on generated file.\n"
I can split this to another CL if you want, it is just cleanup that I found
while regenerating the documentation.

https://codereview.chromium.org/2205693005/diff/20001/tools/gn/functions_targ...
File tools/gn/functions_target.cc (right):

https://codereview.chromium.org/2205693005/diff/20001/tools/gn/functions_targ...
tools/gn/functions_target.cc:282: "    sources = [ \"test/data\" ]\n"
This is removing an invisible control character.

[commit-bot: I haz the power, 2016-08-03 20:52:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-03 20:52:11]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sdefresne, 2016-08-03 20:54:06]

The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-03 20:54:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sdefresne, 2016-08-03 20:58:00]

The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-03 20:58:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 22:12:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-03 22:12:42]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2016-08-03 22:48:11]

dpranke@chromium.org changed reviewers:
+ dpranke@chromium.org

[Dirk Pranke, 2016-08-03 22:48:12]

I think the general concept makes sense, and I think we'll end up wanting
something like it on android as well as we try to figure out how to consollidate
android isolates and ios builds.

I wonder if at some point we'll want a way to filter files as well as labels,
but if we need to choose just one labels is the right choice.

So, basically lgtm but brettw@ should review as well.

[sdefresne, 2016-08-03 22:50:25]

Description was changed from

==========
Add deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new deps_filter property to create_bundle
target used to filter targets when recursively resolving the deps.
Any target matching one of the labels in deps_filter is not added
as a dependency.

BUG=615058
==========

to

==========
Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to create_bundle
target used to filter targets when recursively resolving the deps.
Any target matching one of the labels in bundle_deps_filter is not added
as a dependency.

BUG=615058
==========

[sdefresne, 2016-08-03 22:50:51]

Description was changed from

==========
Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to create_bundle
target used to filter targets when recursively resolving the deps.
Any target matching one of the labels in bundle_deps_filter is not added
as a dependency.

BUG=615058
==========

to

==========
Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to the
create_bundle target used to filter targets when recursively resolving
the deps. Any target matching one of the labels in bundle_deps_filter
is not added as a dependency.

BUG=615058
==========

[sdefresne, 2016-08-03 22:51:06]

Description was changed from

==========
Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to the
create_bundle target used to filter targets when recursively resolving
the deps. Any target matching one of the labels in bundle_deps_filter
is not added as a dependency.

BUG=615058
==========

to

==========
Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to the
create_bundle target used to filter targets when recursively
resolving the deps. Any target matching one of the labels in
bundle_deps_filter is not added as a dependency.

BUG=615058
==========

[brettw, 2016-08-05 18:24:44]

https://codereview.chromium.org/2205693005/diff/60001/tools/gn/variables.cc
File tools/gn/variables.cc (right):

https://codereview.chromium.org/2205693005/diff/60001/tools/gn/variables.cc#n...
tools/gn/variables.cc:611: const char kBundleDepsFilter[] = "deps_filter";
This should also be "bundle_deps_filter"

[sdefresne, 2016-08-05 23:52:39]

The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run

[sdefresne, 2016-08-05 23:53:03]

Thank you for the review, please take another look.

https://codereview.chromium.org/2205693005/diff/60001/tools/gn/variables.cc
File tools/gn/variables.cc (right):

https://codereview.chromium.org/2205693005/diff/60001/tools/gn/variables.cc#n...
tools/gn/variables.cc:611: const char kBundleDepsFilter[] = "deps_filter";
On 2016/08/05 18:24:44, brettw (ping on IM after 24h) wrote:
> This should also be "bundle_deps_filter"

Done.

[commit-bot: I haz the power, 2016-08-05 23:53:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-06 00:51:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-06 00:51:47]

Dry run: This issue passed the CQ dry run.

[brettw, 2016-08-06 04:02:39]

Sorry, I meant to push lgtm last time.

[sdefresne, 2016-08-07 07:26:37]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2016-08-07 07:26:38]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/2205693005/#ps80001
(title: "Fix value of kBundleDepsFilter.")

[commit-bot: I haz the power, 2016-08-07 07:26:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-07 08:18:02]

Description was changed from

==========
Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to the
create_bundle target used to filter targets when recursively
resolving the deps. Any target matching one of the labels in
bundle_deps_filter is not added as a dependency.

BUG=615058
==========

to

==========
Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to the
create_bundle target used to filter targets when recursively
resolving the deps. Any target matching one of the labels in
bundle_deps_filter is not added as a dependency.

BUG=615058
==========

[commit-bot: I haz the power, 2016-08-07 08:18:03]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-07 08:19:34]

Description was changed from

==========
Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to the
create_bundle target used to filter targets when recursively
resolving the deps. Any target matching one of the labels in
bundle_deps_filter is not added as a dependency.

BUG=615058
==========

to

==========
Add bundle_deps_filter to create_bundle targets.

On iOS application extension can share runtime data with the main
application and thus it is not necessary to copy the runtime data
into the application extension bundle.

To implement this add a new bundle_deps_filter property to the
create_bundle target used to filter targets when recursively
resolving the deps. Any target matching one of the labels in
bundle_deps_filter is not added as a dependency.

BUG=615058

Committed: https://crrev.com/7ac497647684c5e90020f2917b2d849b9056826a
Cr-Commit-Position: refs/heads/master@{#410281}
==========

[commit-bot: I haz the power, 2016-08-07 08:19:37]

Patchset 5 (id:??) landed as
https://crrev.com/7ac497647684c5e90020f2917b2d849b9056826a
Cr-Commit-Position: refs/heads/master@{#410281}