Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2453043002:
[Mac/GN] Fix rebuilds when changing framework_version of a mac_framework_bundle. (Closed)
|
Created:
4 years, 1 month ago by Robert Sesek Modified:
4 years, 1 month ago CC:
chromium-reviews, mac-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
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
Originally Committed: https://crrev.com/fed4fc1df8bab7159f19e04ed658c1f417e4e499
Reverted: https://crrev.com/d76ba00e6cc2625000d495e79bdc8d747645405e
Committed: https://crrev.com/a74b313db5f7444667bcf6530f1b2740884f4bfd
Cr-Commit-Position: refs/heads/master@{#428366}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Re-land #
Messages
Total messages: 34 (16 generated)
The CQ bit was checked by rsesek@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Why can't you just modify the mac_framework_bundle() template to make the shared_library() depend on the action()? Do you need to ensure that the action() also runs before any of the dependencies of the shared_library() ?
On 2016/10/26 19:15:15, Dirk Pranke (slow) wrote: > Why can't you just modify the mac_framework_bundle() template to make the > shared_library() depend on the action()? Do you need to ensure that the action() > also runs before any of the dependencies of the shared_library() ? shared_library() is just an example. Every file that gets copied into the bundle via bundle_data would need to depend on the framework version stamp because they're all copied via hard links. E.g., we'd need to be able to add the 'X' dependency from the 'create_bundle' target, for every bundle_data in the create_bundle. create_bundle --> bundle_data --> real_dep --> X
Right, I was guessing it'd be something like this. Let me think about this a bit more ...
On 2016/10/26 19:21:39, Dirk Pranke wrote: > Right, I was guessing it'd be something like this. Let me think about this a bit > more ... If you have ideas I'd be interested because I agree this isn't the best. But I really can't think of another way to do it. (The bug has some other things I considered).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
I'm still thinking about / trying to understand this a bit better. Where do we actually use framework_version? I'm not seeing any references to it anywhere.
On 2016/10/26 22:13:49, Dirk Pranke wrote: > I'm still thinking about / trying to understand this a bit better. Where do we > actually use framework_version? I'm not seeing any references to it anywhere. We're not using it yet. It's for the blocked bugs.
dpranke@chromium.org changed reviewers: + brettw@chromium.org
lgtm, now that I understand things better. It's not terribly obvious how to do this any better. landmines would work if we thought we would only make this switch once, but I'd prefer to not rely on them, and running the script should be very fast. However, you need brettw's approval as well, for the change to //.gn :).
LGTM https://codereview.chromium.org/2453043002/diff/1/build/config/mac/rules.gni File build/config/mac/rules.gni (right): https://codereview.chromium.org/2453043002/diff/1/build/config/mac/rules.gni#... build/config/mac/rules.gni:317: "'$_version_arg'", Are you sure you need to use single-quotes here? I thought GN should quote as necessary to avoid the shell breaking args.
Thanks. https://codereview.chromium.org/2453043002/diff/1/build/config/mac/rules.gni File build/config/mac/rules.gni (right): https://codereview.chromium.org/2453043002/diff/1/build/config/mac/rules.gni#... build/config/mac/rules.gni:317: "'$_version_arg'", On 2016/10/27 19:30:16, brettw (ping on IM after 24h) wrote: > Are you sure you need to use single-quotes here? I thought GN should quote as > necessary to avoid the shell breaking args. GN's quoting is correct. But the framework_version may be "" and it makes it easier to see on the command line if it's explicitly quoted.
The CQ bit was checked by rsesek@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from
==========
[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
==========
to
==========
[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}
==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/fed4fc1df8bab7159f19e04ed658c1f417e4e499 Cr-Commit-Position: refs/heads/master@{#428183}
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2453933006/ by hongchan@chromium.org. The reason for reverting is: 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..
Message was sent while issue was closed.
On 2016/10/27 23:31:14, hongchan wrote: > A revert of this CL (patchset #1 id:1) has been created in > https://codereview.chromium.org/2453933006/ by mailto:hongchan@chromium.org. > > The reason for reverting is: 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.. Sorry for the revert. Also, it's helpful if you include a link to the breakage for easier location later. https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac/b... Traceback (most recent call last): File "/b/c/b/Google_Chrome_Mac/src/build/config/mac/prepare_framework_version.py", line 34, in <module> PrepareFrameworkVersion(sys.argv[1], sys.argv[2], sys.argv[3]) File "/b/c/b/Google_Chrome_Mac/src/build/config/mac/prepare_framework_version.py", line 28, in PrepareFrameworkVersion f = open(version_file, 'w+') IOError: [Errno 2] No such file or directory: '/b/c/b/Google_Chrome_Mac/src/out/Release/obj/ui/base/ui_unittests_framework_version' Looks like we need to `mkdir -p` before writing the file.
Message was sent while issue was closed.
Description was changed from
==========
[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}
==========
to
==========
[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
==========
The CQ bit was checked by rsesek@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by rsesek@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org, brettw@chromium.org Link to the patchset: https://codereview.chromium.org/2453043002/#ps20001 (title: "Re-land")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from
==========
[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
==========
to
==========
[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
==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from
==========
[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
==========
to
==========
[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
Committed: https://crrev.com/a74b313db5f7444667bcf6530f1b2740884f4bfd
Cr-Commit-Position: refs/heads/master@{#428366}
==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/a74b313db5f7444667bcf6530f1b2740884f4bfd Cr-Commit-Position: refs/heads/master@{#428366} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
