[Mac/GN] Re-do framework packaging to fix framework versioning.

[Mac/GN] Re-do framework packaging to fix framework versioning.

Rather than trying to symlink as an action after producing the bundle, do it
as an early-stage action. As part of this, mac_framework_bundle callers need
to explicitly specify the top-level symlinks (framework_contents) that should
be created. Frameworks are now packaged like so:

1. At `gn gen` time, an exec_script runs to write the framework_version to a
   file. If the previous value written does not match the new value, the entire
   framework output directory is clobbered. This must be done at gen-time to
   ensure nothing tries to clean the framework while also attempting to copy
   to it.
2. Also at `gn gen` time, a TOC file for the framework_contents is written.
   This allows the build to emulate depending on the presence of the top-level
   symlinks, since ninja does not stat symlinks correctly.
   https://github.com/ninja-build/ninja/issues/1186
3. The package_framework.py action now runs before the main create_bundle().
   This action depends on the TOC file from (2) and will create all the
   required symlinks in the bundle. At the time this runs, the symlink target
   may not yet exist.
4. The create_bundle target is now always the leaf edge for
   mac_framework_bundle, and it copies all bundle contents to the fully
   versioned path (rather than through a symlink).

This should resolve the issue of the build not stabilizing between specifying
a framework_version and not.

BUG=648757
Committed: https://crrev.com/9522ddcc745f739414607811da8a2c6a17dbb17a
Cr-Commit-Position: refs/heads/master@{#430778}

[Robert Sesek, 2016-11-08 17:51:05]

Description was changed from

==========
[Mac/GN] Re-do framework packaging to fix framework versioning.

Rather than trying to symlink as an action after producing the bundle, do it
as an early-stage action. As part of this, mac_framework_bundle callers need
to explicitly specify the top-level symlinks that should be created.
Frameworks are now packaged like so:

1. At `gn gen` time, an exec_script runs to write the framework_version to a
   file. If the previous value written does not match the new value, the entire
   framework output directory is clobbered.
2. Also at `gn gen` time, a TOC file for the framework_contents is written.
   This allows the build to depend on the presence of the top-level symlinks,
   since ninja does not stat symlinks correctly.
   https://github.com/ninja-build/ninja/issues/1186
3. The package_framework.py action now runs before the main create_bundle().
   This action depends on the TOC file from (2) and will create all the
   required symlinks in the bundle. At the time this runs, the symlink target
   may not yet exist.
4. The create_bundle target is the leaf node for mac_framework_bundle, and it
   copies all bundle contents to the fully versioned path (rather than through
   a symlink).

This should resolve the issue of the build not stabilizing between specifying
a framework_version and not.

BUG=648757
==========

to

==========
[Mac/GN] Re-do framework packaging to fix framework versioning.

Rather than trying to symlink as an action after producing the bundle, do it
as an early-stage action. As part of this, mac_framework_bundle callers need
to explicitly specify the top-level symlinks (framework_contents) that should
be created. Frameworks are now packaged like so:

1. At `gn gen` time, an exec_script runs to write the framework_version to a
   file. If the previous value written does not match the new value, the entire
   framework output directory is clobbered. This must be done at gen-time to
   ensure nothing tries to clean the framework while also attempting to copy
   to it.
2. Also at `gn gen` time, a TOC file for the framework_contents is written.
   This allows the build to emulate depending on the presence of the top-level
   symlinks, since ninja does not stat symlinks correctly.
   https://github.com/ninja-build/ninja/issues/1186
3. The package_framework.py action now runs before the main create_bundle().
   This action depends on the TOC file from (2) and will create all the
   required symlinks in the bundle. At the time this runs, the symlink target
   may not yet exist.
4. The create_bundle target is now always the leaf edge for
   mac_framework_bundle, and it copies all bundle contents to the fully
   versioned path (rather than through a symlink).

This should resolve the issue of the build not stabilizing between specifying
a framework_version and not.

BUG=648757
==========

[Robert Sesek, 2016-11-08 19:19:41]

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

[Dirk Pranke, 2016-11-08 21:41:10]

lgtm, I think, though it's hard for me to keep all of the subtleties in my head
:).

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/package_fr...
File build/config/mac/package_framework.py (right):

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/package_fr...
build/config/mac/package_framework.py:16: parser.add_argument('--stamp',
action='store', type=str)
Maybe make --framework and --stamp required as well?

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/package_fr...
build/config/mac/package_framework.py:35: if args.contents:
nit: you don't need the `if` here, just the for loop is sufficient.

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

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/rules.gni#...
build/config/mac/rules.gni:310: _framework_toc_file,
I don't see where package_framework.py is actually reading the .toc ? Is it
supposed to be, or are you listing it as an input just to trigger the rule to be
re-run? If the latter, we should probably add a comment to that effect.

[Robert Sesek, 2016-11-08 21:55:33]

Thanks!

On 2016/11/08 21:41:10, Dirk Pranke wrote:
> lgtm, I think, though it's hard for me to keep all of the subtleties in my
head
> :).

You're not the only one ;).

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/package_fr...
File build/config/mac/package_framework.py (right):

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/package_fr...
build/config/mac/package_framework.py:16: parser.add_argument('--stamp',
action='store', type=str)
On 2016/11/08 21:41:10, Dirk Pranke wrote:
> Maybe make --framework and --stamp required as well?

Done.

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/package_fr...
build/config/mac/package_framework.py:35: if args.contents:
On 2016/11/08 21:41:10, Dirk Pranke wrote:
> nit: you don't need the `if` here, just the for loop is sufficient.

contents can be None if it's not passed, so that would result in "None not
iterable" error.

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

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/rules.gni#...
build/config/mac/rules.gni:310: _framework_toc_file,
On 2016/11/08 21:41:10, Dirk Pranke wrote:
> I don't see where package_framework.py is actually reading the .toc ? Is it
> supposed to be, or are you listing it as an input just to trigger the rule to
be
> re-run? If the latter, we should probably add a comment to that effect.

Just the latter; we're only using it as an input dependency. Added a comment.

[Robert Sesek, 2016-11-08 21:55:37]

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

[commit-bot: I haz the power, 2016-11-08 21:56:07]

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

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

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/package_fr...
File build/config/mac/package_framework.py (right):

https://codereview.chromium.org/2487763002/diff/1/build/config/mac/package_fr...
build/config/mac/package_framework.py:35: if args.contents:
On 2016/11/08 21:55:33, Robert Sesek wrote:
> On 2016/11/08 21:41:10, Dirk Pranke wrote:
> > nit: you don't need the `if` here, just the for loop is sufficient.
> 
> contents can be None if it's not passed, so that would result in "None not
> iterable" error.

Ah, you're right, I thought it defaults to [] if not passed for `nargs='+'`.

[Robert Sesek, 2016-11-08 23:30:12]

The CQ bit was checked by rsesek@chromium.org

[Robert Sesek, 2016-11-08 23:30:13]

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/2487763002/#ps20001
(title: "address comments")

[commit-bot: I haz the power, 2016-11-08 23:30:27]

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-11-09 00:05:23]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-09 00:08:21]

Description was changed from

==========
[Mac/GN] Re-do framework packaging to fix framework versioning.

Rather than trying to symlink as an action after producing the bundle, do it
as an early-stage action. As part of this, mac_framework_bundle callers need
to explicitly specify the top-level symlinks (framework_contents) that should
be created. Frameworks are now packaged like so:

1. At `gn gen` time, an exec_script runs to write the framework_version to a
   file. If the previous value written does not match the new value, the entire
   framework output directory is clobbered. This must be done at gen-time to
   ensure nothing tries to clean the framework while also attempting to copy
   to it.
2. Also at `gn gen` time, a TOC file for the framework_contents is written.
   This allows the build to emulate depending on the presence of the top-level
   symlinks, since ninja does not stat symlinks correctly.
   https://github.com/ninja-build/ninja/issues/1186
3. The package_framework.py action now runs before the main create_bundle().
   This action depends on the TOC file from (2) and will create all the
   required symlinks in the bundle. At the time this runs, the symlink target
   may not yet exist.
4. The create_bundle target is now always the leaf edge for
   mac_framework_bundle, and it copies all bundle contents to the fully
   versioned path (rather than through a symlink).

This should resolve the issue of the build not stabilizing between specifying
a framework_version and not.

BUG=648757
==========

to

==========
[Mac/GN] Re-do framework packaging to fix framework versioning.

Rather than trying to symlink as an action after producing the bundle, do it
as an early-stage action. As part of this, mac_framework_bundle callers need
to explicitly specify the top-level symlinks (framework_contents) that should
be created. Frameworks are now packaged like so:

1. At `gn gen` time, an exec_script runs to write the framework_version to a
   file. If the previous value written does not match the new value, the entire
   framework output directory is clobbered. This must be done at gen-time to
   ensure nothing tries to clean the framework while also attempting to copy
   to it.
2. Also at `gn gen` time, a TOC file for the framework_contents is written.
   This allows the build to emulate depending on the presence of the top-level
   symlinks, since ninja does not stat symlinks correctly.
   https://github.com/ninja-build/ninja/issues/1186
3. The package_framework.py action now runs before the main create_bundle().
   This action depends on the TOC file from (2) and will create all the
   required symlinks in the bundle. At the time this runs, the symlink target
   may not yet exist.
4. The create_bundle target is now always the leaf edge for
   mac_framework_bundle, and it copies all bundle contents to the fully
   versioned path (rather than through a symlink).

This should resolve the issue of the build not stabilizing between specifying
a framework_version and not.

BUG=648757

Committed: https://crrev.com/9522ddcc745f739414607811da8a2c6a17dbb17a
Cr-Commit-Position: refs/heads/master@{#430778}
==========

[commit-bot: I haz the power, 2016-11-09 00:08:22]

Patchset 2 (id:??) landed as
https://crrev.com/9522ddcc745f739414607811da8a2c6a17dbb17a
Cr-Commit-Position: refs/heads/master@{#430778}