Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1030)

Issue 2118563002: Revert of [Mac/GN] Explicitly link ApplicationServices wherever CoreGraphics is linked. (Closed)

Created:
4 years, 5 months ago by Robert Sesek
Modified:
4 years, 5 months ago
Reviewers:
Mark Mentovai
CC:
chromium-reviews, piman+watch_chromium.org, miu+watch_chromium.org, tfarina, mcasas+watch+vc_chromium.org, feature-media-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of [Mac/GN] Explicitly link ApplicationServices wherever CoreGraphics is linked. (patchset #2 id:20001 of https://codereview.chromium.org/2092513002/ ) Reason for revert: After https://crrev.com/42e3c3e7125d the build warns if it detects this incompatibility. Original issue's description: > [Mac/GN] Explicitly link ApplicationServices wherever CoreGraphics is linked. > > The 10.11 SDK has an incompatibility with a OS X 10.7 deployment target. > ApplicationServices re-exports CoreGraphics, but due to a bug, the dylib > compatibility version from the re-exported framework gets confused with > the version of the framework doing the re-export. > > This only manifests itself in the component build because individual > components depend on CoreGraphics directly instead of ApplicationServices. > In the static library build, the transitive collection of libs ensures that > ApplicationServices gets linked before CoreGraphics when linking the > Chromium Framework, so this doesn't occur. > > To hack around the issue, specify ApplicationServices in libs ahead of > CoreGraphics so that the correct compatibility version is picked up. After > the deployment is updated to 10.8+ (https://crbug.com/580152) these hacks > can be removed. > > BUG=620127 > R=mark@chromium.org > CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel > > Committed: https://chromium.googlesource.com/chromium/src/+/5f7bc190c7ffeda2a2c56161b371bc16750fac2c TBR=mark@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=620127 Committed: https://crrev.com/ac799c2fd50b8fb62b7a8186ff78b025de5b8718 Cr-Commit-Position: refs/heads/master@{#403392}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -12 lines) Patch
M gpu/ipc/service/BUILD.gn View 1 chunk +1 line, -2 lines 0 comments Download
M media/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M media/capture/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M skia/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M ui/accelerated_widget_mac/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M ui/display/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M ui/gfx/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M ui/native_theme/BUILD.gn View 1 chunk +1 line, -2 lines 0 comments Download
M ui/snapshot/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M ui/views/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 15 (6 generated)
Robert Sesek
Created Revert of [Mac/GN] Explicitly link ApplicationServices wherever CoreGraphics is linked.
4 years, 5 months ago (2016-06-30 20:22:50 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2118563002/1
4 years, 5 months ago (2016-06-30 20:23:49 UTC) #3
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/253467)
4 years, 5 months ago (2016-06-30 21:12:58 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2118563002/1
4 years, 5 months ago (2016-06-30 21:28:21 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/248757)
4 years, 5 months ago (2016-06-30 22:51:49 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2118563002/1
4 years, 5 months ago (2016-07-01 00:44:58 UTC) #11
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 5 months ago (2016-07-01 02:19:07 UTC) #12
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/ac799c2fd50b8fb62b7a8186ff78b025de5b8718 Cr-Commit-Position: refs/heads/master@{#403392}
4 years, 5 months ago (2016-07-01 02:21:10 UTC) #14
Mark Mentovai
4 years, 5 months ago (2016-07-01 12:49:59 UTC) #15
Message was sent while issue was closed.
LGTM

Powered by Google App Engine
This is Rietveld 408576698