GN: Flipping between versioned and un-versioned framework bundles is broken |
||||
Issue descriptionThe mac_framework_bundle has support for both versioned and un-versioned frameworks, as specified in the rule here: https://chromium.googlesource.com/chromium/src/+/9d5d972/build/config/mac/rules.gni#355 Currently the chrome_framework target is un-versioned, giving it this structure: Chromium Framework.framework/ Chromium Framework Helpers/ Resources/ To use XPC ( bug 604809 ), we need to make the framework versioned, which would produce this structure (with -> being a symlink): Chromium Framework.framework/ Chromium Framework -> Versions/Current/Chromium Framework Helpers -> Versions/Current/Helpers Resources -> Versions/Current/Resources Versions/ A/ Chromium Framework Helpers/ Resources/ Current -> A The issue is that flipping between a versioned and un-versioned framework can lead to the bundle_data copies going into the wrong place (if the symlinks exist) or existing with an invalid copy destination. To resolve this, I've come up with a couple of ideas: 1) Create an inputdeps on all the copy_bundle_data rules consisting of the non-bundle_data deps on the create_bundle target. This does not work, though, because we use hard links to "copy" the bundle data, and then the stamp from the inputdeps is always more recent compared to the hardlinked files. 2) Add a create_bundle_script and create_bundle_script_args to the create_bundle tool. These would operate much like how the code_signing_script/code_signing_args work, except the script would need to run before the copies. I'm not sure if this would have the same issue as #1 does with the hardlinks though. 3) Make create_bundle an actual tool, which pre-creates all the bundle_*_dirs before executing the copy_bundle_data steps. If it detects that a bundle_*_dir is a bad symlink, it removes it and replaces it with a real dir. 4) Assume that we won't be flipping between versioned and un-versioned frameworks, and just use landmines to work around this. This is not an ideal solution, since we may indeed need to land/revert the change.
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d76ba00e6cc2625000d495e79bdc8d747645405e commit d76ba00e6cc2625000d495e79bdc8d747645405e Author: hongchan <hongchan@chromium.org> Date: Thu Oct 27 23:33:19 2016 Revert of [Mac/GN] Fix rebuilds when changing framework_version of a mac_framework_bundle. (patchset #1 id:1 of https://codereview.chromium.org/2453043002/ ) Reason for revert: rules.gni file is causing the build failure. ------ /b/c/b/Google_Chrome_Mac/src/buildtools/mac/gn gen //out/Release --check -> returned 1 ERROR at //build/config/mac/rules.gni:313:3: Script returned non-zero exit code. exec_script("//build/config/mac/prepare_framework_version.py", ^---------- Current dir: /b/c/b/Google_Chrome_Mac/src/out/Release/ Command: python -- /b/c/b/Google_Chrome_Mac/src/build/config/mac/prepare_framework_version.py /b/c/b/Google_Chrome_Mac/src/out/Release/obj/ui/base/ui_unittests_framework_version /b/c/b/Google_Chrome_Mac/src/out/Release/ui_unittests Framework.framework '' Returned 1. Original issue's description: > [Mac/GN] Fix rebuilds when changing framework_version of a mac_framework_bundle. > > Write the current framework_version to a file at gen-time, and clobber the > entire framework bundle if it differs from the current value in the file. > > This has to be done at gen-time because it is not possible to run a script at > the create_bundle stage before any other dependencies in its tree run. Take > this sample graph: > > bundle_data --> shared_library > / > mac_framework_bundle > \ > action("clean_framework_version") > > It is not possible, from a mac_framework_bundle, to force the > clean_framework_version action to run before the shared_library. When the > action does run, its stamp will have a newer mtime than the shared_library. > Because bundle_data are hard linked into place, the link source file will > have an older mtime than the action, and the build will never stabilize. > > BUG= 648757 > R=dpranke@chromium.org > > Committed: https://crrev.com/fed4fc1df8bab7159f19e04ed658c1f417e4e499 > Cr-Commit-Position: refs/heads/master@{#428183} TBR=dpranke@chromium.org,brettw@chromium.org,rsesek@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 648757 Review-Url: https://codereview.chromium.org/2453933006 Cr-Commit-Position: refs/heads/master@{#428212} [modify] https://crrev.com/d76ba00e6cc2625000d495e79bdc8d747645405e/.gn [delete] https://crrev.com/b3e637eba149cbfc8506c1accf8b3820af348f1a/build/config/mac/prepare_framework_version.py [modify] https://crrev.com/d76ba00e6cc2625000d495e79bdc8d747645405e/build/config/mac/rules.gni
,
Oct 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a74b313db5f7444667bcf6530f1b2740884f4bfd commit a74b313db5f7444667bcf6530f1b2740884f4bfd Author: rsesek <rsesek@chromium.org> Date: Fri Oct 28 14:42:10 2016 [Mac/GN] Fix rebuilds when changing framework_version of a mac_framework_bundle. Write the current framework_version to a file at gen-time, and clobber the entire framework bundle if it differs from the current value in the file. This has to be done at gen-time because it is not possible to run a script at the create_bundle stage before any other dependencies in its tree run. Take this sample graph: bundle_data --> shared_library / mac_framework_bundle \ action("clean_framework_version") It is not possible, from a mac_framework_bundle, to force the clean_framework_version action to run before the shared_library. When the action does run, its stamp will have a newer mtime than the shared_library. Because bundle_data are hard linked into place, the link source file will have an older mtime than the action, and the build will never stabilize. BUG= 648757 R=dpranke@chromium.org Originally Committed: https://crrev.com/fed4fc1df8bab7159f19e04ed658c1f417e4e499 Reverted: https://crrev.com/d76ba00e6cc2625000d495e79bdc8d747645405e Review-Url: https://codereview.chromium.org/2453043002 Cr-Commit-Position: refs/heads/master@{#428366} [modify] https://crrev.com/a74b313db5f7444667bcf6530f1b2740884f4bfd/.gn [add] https://crrev.com/a74b313db5f7444667bcf6530f1b2740884f4bfd/build/config/mac/prepare_framework_version.py [modify] https://crrev.com/a74b313db5f7444667bcf6530f1b2740884f4bfd/build/config/mac/rules.gni
,
Oct 28 2016
,
Nov 8 2016
This is still broken. The build failed to stabilize after flipping versions: ninja: Entering directory `/b/c/b/mac/src/out/Debug' ninja explain: output Chromium.app/Contents/Versions/56.0.2906.0/Chromium Framework.framework older than most recent input Chromium Framework.framework (1477926303 vs 1477926518) ninja explain: Chromium.app/Contents/Versions/56.0.2906.0/Chromium Framework.framework is dirty ninja explain: obj/chrome/chrome_app.stamp is dirty ninja explain: obj/chrome/chrome_app.stamp is dirty ninja explain: obj/chrome/chrome.stamp is dirty ninja explain: obj/chrome/chrome.stamp is dirty [1/4] COPY_BUNDLE_DATA 'Chromium Framework.framework' 'Chromium.app/Contents/Versions/56.0.2906.0/Chromium Framework.framework' [2/4] STAMP obj/chrome/chrome_app.stamp [3/4] STAMP obj/chrome/chrome.stamp [4/4] STAMP obj/chrome/test/browser_tests.inputdeps.stamp <Thread(Thread-1, started 4386082816)> ProcessRead: proc.stdout finished. <Thread(Thread-1, started 4386082816)> ProcessRead: cleaning up. <Thread(Thread-2, started daemon 4390289408)> TimedFlush: Finished <Thread(Thread-1, started 4386082816)> ProcessRead: finished. Failing build because ninja reported work to do.
,
Nov 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9522ddcc745f739414607811da8a2c6a17dbb17a commit 9522ddcc745f739414607811da8a2c6a17dbb17a Author: rsesek <rsesek@chromium.org> Date: Wed Nov 09 00:04:46 2016 [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 Review-Url: https://codereview.chromium.org/2487763002 Cr-Commit-Position: refs/heads/master@{#430778} [modify] https://crrev.com/9522ddcc745f739414607811da8a2c6a17dbb17a/build/config/mac/package_framework.py [modify] https://crrev.com/9522ddcc745f739414607811da8a2c6a17dbb17a/build/config/mac/rules.gni [modify] https://crrev.com/9522ddcc745f739414607811da8a2c6a17dbb17a/chrome/BUILD.gn
,
Nov 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7accb69c5cc3c08b6c36b2d616cf4a38892ade55 commit 7accb69c5cc3c08b6c36b2d616cf4a38892ade55 Author: rsesek <rsesek@chromium.org> Date: Thu Nov 10 00:30:03 2016 [Mac/GN] Don't list any symlinks in the outputs of mac_framework_bundle. Despite the comment left in 9522ddcc745f, the bots do not agree that this is stable. BUG= 648757 Review-Url: https://codereview.chromium.org/2492493002 Cr-Commit-Position: refs/heads/master@{#431098} [modify] https://crrev.com/7accb69c5cc3c08b6c36b2d616cf4a38892ade55/build/config/mac/rules.gni
,
Nov 10 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by bugdroid1@chromium.org
, Oct 27 2016