Mac/iOS/GN: Generate Info.plist files for Mac apps and frameworks with gn.

Mac/iOS/GN: Generate Info.plist files for Mac apps and frameworks with gn.

Refactor out common code between Mac and iOS Info.plist generation using gn and
make separate mac_info_plist and ios_info_plist templates for use with existing
mac_framework_bundle, mac_app_bundle, and ios_app_bundle templates.

BUG=297668
Committed: https://crrev.com/7df5f898fc2ccd951ac36fb18a485775586b4dcf
Cr-Commit-Position: refs/heads/master@{#389682}

[Patti Lor, 2016-04-19 05:38:45]

Patchset #3 (id:40001) has been deleted

[Patti Lor, 2016-04-19 05:43:06]

Description was changed from

==========
Generate .plist files for Mac applications. WIP! Do not submit.
==========

to

==========
Mac: Generate Info.plist files for Mac apps and frameworks with gn. WIP!

Add new template for generating Info.plist using gn and integrate with existing
mac_framework_bundle and mac_app_bundle templates.

BUG=
==========

[Patti Lor, 2016-04-19 05:53:26]

patricialor@chromium.org changed reviewers:
+ rsesek@chromium.org

[Patti Lor, 2016-04-19 05:53:27]

Hi Robert, PTAL. It's almost there (I think?) but gn is complaining about
duplicate Info.plist files being generated for apps (and frameworks as well I
think) and I'm not sure where the <target_name>_info_plist_bundle_data
dependency is being added in again. Thanks!

[Robert Sesek, 2016-04-19 14:49:04]

Thanks for working on this! With the changes we discussed over email, this
should work.

https://codereview.chromium.org/1877523002/diff/60001/build/config/ios/rules.gni
File build/config/ios/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/60001/build/config/ios/rules....
build/config/ios/rules.gni:48: action(_generate_info_plist) {
It would be good to re-implement this in terms of the new info_plist template
you created. That'd mean refactoring out the common substitutions below into the
base template info_plist, with the iOS or Mac specific substitutions passed as
extra_substitutions to it. I'd create a mac_info_plist template that calls the
base info_plist template with the Mac-specific substitutions. You could do the
same for an ios_info_plist template, but since this would be the only use,
that's not yet necessary.

I did something very similar for XIB files here:
https://codereview.chromium.org/1897853003

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules.gni
File build/config/mac/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:9: # Used to generate an Info.plist file for iOS and
Mac apps and frameworks.
Also document the Arguments as the other templates do below.

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:196: # TODO(rsesek): Process Info.plist variables.
Remove, since you're doing it :).

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:281: # TODO(rsesek): Process Info.plist variables.
Remove.

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:284: if (!defined(extra_substitutions)) {
Since you're not forward_varaibles_from()-ing extra_substitutions, this will
never be hit. I don't think there's a need yet for targets to define their own
substitutions, so you can remove this if block.

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:288: "PRODUCT_NAME=$_output_name",
These subs are needed in the mac_framework_bundle version as well.

[Patti Lor, 2016-04-21 07:55:36]

Description was changed from

==========
Mac: Generate Info.plist files for Mac apps and frameworks with gn. WIP!

Add new template for generating Info.plist using gn and integrate with existing
mac_framework_bundle and mac_app_bundle templates.

BUG=
==========

to

==========
Mac: Generate Info.plist files for Mac apps and frameworks with gn.

Refactor out common code between Mac and iOS Info.plist generation using gn and
make separate mac_info_plist and ios_info_plist templates for use with existing
mac_framework_bundle, mac_app_bundle, and ios_app_bundle templates.

BUG=297668
==========

[Patti Lor, 2016-04-21 08:00:41]

Description was changed from

==========
Mac: Generate Info.plist files for Mac apps and frameworks with gn.

Refactor out common code between Mac and iOS Info.plist generation using gn and
make separate mac_info_plist and ios_info_plist templates for use with existing
mac_framework_bundle, mac_app_bundle, and ios_app_bundle templates.

BUG=297668
==========

to

==========
Mac/iOS/GN: Generate Info.plist files for Mac apps and frameworks with gn.

Refactor out common code between Mac and iOS Info.plist generation using gn and
make separate mac_info_plist and ios_info_plist templates for use with existing
mac_framework_bundle, mac_app_bundle, and ios_app_bundle templates.

BUG=297668
==========

[Patti Lor, 2016-04-21 08:02:09]

Thanks Robert, here are the changes you suggested! I went ahead and created a
ios_info_plist template as well.

https://codereview.chromium.org/1877523002/diff/60001/build/config/ios/rules.gni
File build/config/ios/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/60001/build/config/ios/rules....
build/config/ios/rules.gni:48: action(_generate_info_plist) {
On 2016/04/19 14:49:04, Robert Sesek wrote:
> It would be good to re-implement this in terms of the new info_plist template
> you created. That'd mean refactoring out the common substitutions below into
the
> base template info_plist, with the iOS or Mac specific substitutions passed as
> extra_substitutions to it. I'd create a mac_info_plist template that calls the
> base info_plist template with the Mac-specific substitutions. You could do the
> same for an ios_info_plist template, but since this would be the only use,
> that's not yet necessary.
> 
> I did something very similar for XIB files here:
> https://codereview.chromium.org/1897853003

Done.

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules.gni
File build/config/mac/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:9: # Used to generate an Info.plist file for iOS and
Mac apps and frameworks.
On 2016/04/19 14:49:04, Robert Sesek wrote:
> Also document the Arguments as the other templates do below.

Done.

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:196: # TODO(rsesek): Process Info.plist variables.
On 2016/04/19 14:49:04, Robert Sesek wrote:
> Remove, since you're doing it :).

Done.

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:281: # TODO(rsesek): Process Info.plist variables.
On 2016/04/19 14:49:04, Robert Sesek wrote:
> Remove.

Done.

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:284: if (!defined(extra_substitutions)) {
On 2016/04/19 14:49:04, Robert Sesek wrote:
> Since you're not forward_varaibles_from()-ing extra_substitutions, this will
> never be hit. I don't think there's a need yet for targets to define their own
> substitutions, so you can remove this if block.

Done.

https://codereview.chromium.org/1877523002/diff/60001/build/config/mac/rules....
build/config/mac/rules.gni:288: "PRODUCT_NAME=$_output_name",
On 2016/04/19 14:49:04, Robert Sesek wrote:
> These subs are needed in the mac_framework_bundle version as well.

Done.

[Robert Sesek, 2016-04-21 15:36:21]

https://codereview.chromium.org/1877523002/diff/140001/build/config/ios/rules...
File build/config/ios/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/140001/build/config/ios/rules...
build/config/ios/rules.gni:9: assert(defined(invoker.info_plist),
I think you can rely on the asserts in the info_plist template to handle this.

https://codereview.chromium.org/1877523002/diff/140001/build/config/mac/rules...
File build/config/mac/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/140001/build/config/mac/rules...
build/config/mac/rules.gni:17: #     _output_name:
_ prefix is usually for private variables. Call this executable_name instead,
since that'll be more clear in the context of info_plist.

https://codereview.chromium.org/1877523002/diff/140001/build/config/mac/rules...
build/config/mac/rules.gni:75: assert(defined(invoker.info_plist),
I think you can rely on the asserts in the base info_plist template to handle
this.

[Patti Lor, 2016-04-22 02:49:55]

https://codereview.chromium.org/1877523002/diff/140001/build/config/ios/rules...
File build/config/ios/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/140001/build/config/ios/rules...
build/config/ios/rules.gni:9: assert(defined(invoker.info_plist),
On 2016/04/21 15:36:21, Robert Sesek wrote:
> I think you can rely on the asserts in the info_plist template to handle this.

Done.

https://codereview.chromium.org/1877523002/diff/140001/build/config/mac/rules...
File build/config/mac/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/140001/build/config/mac/rules...
build/config/mac/rules.gni:17: #     _output_name:
On 2016/04/21 15:36:21, Robert Sesek wrote:
> _ prefix is usually for private variables. Call this executable_name instead,
> since that'll be more clear in the context of info_plist.

Done.

https://codereview.chromium.org/1877523002/diff/140001/build/config/mac/rules...
build/config/mac/rules.gni:75: assert(defined(invoker.info_plist),
On 2016/04/21 15:36:21, Robert Sesek wrote:
> I think you can rely on the asserts in the base info_plist template to handle
> this.

Done.

[Robert Sesek, 2016-04-22 04:34:40]

LGTM w/ two nits.

https://codereview.chromium.org/1877523002/diff/160001/build/config/mac/rules...
File build/config/mac/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/160001/build/config/mac/rules...
build/config/mac/rules.gni:28: "The output_name file must be specified for
$target_name")
"output_name file" -> "executable_name"

https://codereview.chromium.org/1877523002/diff/160001/build/config/mac/rules...
build/config/mac/rules.gni:49: "-s=VERSION=$chrome_version_full",
Remove VERSION and VERSION_BUILD from the template, since those should be set
only by the chrome target (when it uses this).

[Patti Lor, 2016-04-26 03:25:16]

https://codereview.chromium.org/1877523002/diff/160001/build/config/mac/rules...
File build/config/mac/rules.gni (right):

https://codereview.chromium.org/1877523002/diff/160001/build/config/mac/rules...
build/config/mac/rules.gni:28: "The output_name file must be specified for
$target_name")
On 2016/04/22 04:34:40, Robert Sesek wrote:
> "output_name file" -> "executable_name"

Done.

https://codereview.chromium.org/1877523002/diff/160001/build/config/mac/rules...
build/config/mac/rules.gni:49: "-s=VERSION=$chrome_version_full",
On 2016/04/22 04:34:40, Robert Sesek wrote:
> Remove VERSION and VERSION_BUILD from the template, since those should be set
> only by the chrome target (when it uses this).

Done.

[Patti Lor, 2016-04-26 03:25:26]

The CQ bit was checked by patricialor@chromium.org

[Patti Lor, 2016-04-26 03:25:27]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org
Link to the patchset: https://codereview.chromium.org/1877523002/#ps180001
(title: "Delete version information to be populated elsewhere, fix assert
string.")

[commit-bot: I haz the power, 2016-04-26 03:25:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1877523002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1877523002/180001

[commit-bot: I haz the power, 2016-04-26 03:30:01]

Description was changed from

==========
Mac/iOS/GN: Generate Info.plist files for Mac apps and frameworks with gn.

Refactor out common code between Mac and iOS Info.plist generation using gn and
make separate mac_info_plist and ios_info_plist templates for use with existing
mac_framework_bundle, mac_app_bundle, and ios_app_bundle templates.

BUG=297668
==========

to

==========
Mac/iOS/GN: Generate Info.plist files for Mac apps and frameworks with gn.

Refactor out common code between Mac and iOS Info.plist generation using gn and
make separate mac_info_plist and ios_info_plist templates for use with existing
mac_framework_bundle, mac_app_bundle, and ios_app_bundle templates.

BUG=297668
==========

[commit-bot: I haz the power, 2016-04-26 03:30:03]

Committed patchset #9 (id:180001)

[commit-bot: I haz the power, 2016-04-26 03:31:21]

Description was changed from

==========
Mac/iOS/GN: Generate Info.plist files for Mac apps and frameworks with gn.

Refactor out common code between Mac and iOS Info.plist generation using gn and
make separate mac_info_plist and ios_info_plist templates for use with existing
mac_framework_bundle, mac_app_bundle, and ios_app_bundle templates.

BUG=297668
==========

to

==========
Mac/iOS/GN: Generate Info.plist files for Mac apps and frameworks with gn.

Refactor out common code between Mac and iOS Info.plist generation using gn and
make separate mac_info_plist and ios_info_plist templates for use with existing
mac_framework_bundle, mac_app_bundle, and ios_app_bundle templates.

BUG=297668

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

[commit-bot: I haz the power, 2016-04-26 03:31:22]

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