Summary: | [Mac] WebCore.framework is missing a symlink from Frameworks to Versions/Current/Frameworks when the latter exists | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | mitz | ||||||||||
Component: | WebCore Misc. | Assignee: | Joseph Pecoraro <joepeck> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, buildbot, joepeck, jonlee, rniwa, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | Other | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
mitz
2017-03-01 20:38:36 PST
I see WebKit has a Make Frameworks Symbolic Link build phase that this can mimic. Should we be making libwebrtc's install path go to WebCore.frameworks/Versions/Current/Frameworks instead of to WebCore.frameworks/Frameworks which will now be a symlink? (In reply to comment #1) > I see WebKit has a Make Frameworks Symbolic Link build phase that this can > mimic. > > Should we be making libwebrtc's install path go to > WebCore.frameworks/Versions/Current/Frameworks instead of to > WebCore.frameworks/Frameworks which will now be a symlink? libwebrtc’s own install path is, and should remain, …/WebCore.framework/Versions/A/Frameworks . Ahh yes, sorry. iOS has shallow bundles, and we would only want to be adding the symlink for non-shallow bundle cases like macOS. (In reply to comment #3) > Ahh yes, sorry. iOS has shallow bundles, and we would only want to be adding > the symlink for non-shallow bundle cases like macOS. That’s why the bug title says [Mac] :| Created attachment 303219 [details]
[PATCH] Proposed Fix
Comment on attachment 303219 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=303219&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:29344 > + shellScript = "if [[ ${PLATFORM_NAME} != \"macosx\" ]]; then\n exit 0\nfi\n\nif [[ ${WK_USE_OVERRIDE_FRAMEWORKS_DIR} == \"YES\" ]]; then\n exit 0\nfi\n\nln -shf Versions/Current/Frameworks \"$TARGET_BUILD_DIR/WebCore.framework/Frameworks\"\n"; The script is: if [[ ${PLATFORM_NAME} != "macosx" ]]; then exit 0 fi if [[ ${WK_USE_OVERRIDE_FRAMEWORKS_DIR} == "YES" ]]; then exit 0 fi ln -shf Versions/Current/Frameworks "$TARGET_BUILD_DIR/WebCore.framework/Frameworks" This still creates a symlink in engineering Release/Debug builds, but I think thats harmless. Comment on attachment 303219 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=303219&action=review >> Source/WebCore/WebCore.xcodeproj/project.pbxproj:29344 >> + shellScript = "if [[ ${PLATFORM_NAME} != \"macosx\" ]]; then\n exit 0\nfi\n\nif [[ ${WK_USE_OVERRIDE_FRAMEWORKS_DIR} == \"YES\" ]]; then\n exit 0\nfi\n\nln -shf Versions/Current/Frameworks \"$TARGET_BUILD_DIR/WebCore.framework/Frameworks\"\n"; > > The script is: > > if [[ ${PLATFORM_NAME} != "macosx" ]]; then > exit 0 > fi > > if [[ ${WK_USE_OVERRIDE_FRAMEWORKS_DIR} == "YES" ]]; then > exit 0 > fi > > ln -shf Versions/Current/Frameworks "$TARGET_BUILD_DIR/WebCore.framework/Frameworks" > > This still creates a symlink in engineering Release/Debug builds, but I think thats harmless. Creating the symlink in engineering builds seems fine. But you should not create it when ENABLE_WEB_RTC is empty. Created attachment 303335 [details]
[PATCH] Proposed Patch
Since this was a non-obvious review comment to address I'm putting up a new PFR.
Created attachment 303336 [details]
[PATCH] Proposed Patch
Rebaselined.
Comment on attachment 303336 [details] [PATCH] Proposed Patch View in context: https://bugs.webkit.org/attachment.cgi?id=303336&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:29367 > + shellScript = "if [[ ${PLATFORM_NAME} != \"macosx\" ]]; then\n exit 0\nfi\n\nif [[ ${WK_USE_OVERRIDE_FRAMEWORKS_DIR} == \"YES\" ]]; then\n exit 0\nfi\n\n# ENABLE_WEB_RTC cannot be relied on because of its conditionals.\nif [[ ${CONFIGURATION} == \"Production\" && ${TARGET_MAC_OS_X_VERSION_MAJOR} != \"101300\" ]]; then\n exit 0\nfi\n\nln -shf Versions/Current/Frameworks \"$TARGET_BUILD_DIR/WebCore.framework/Frameworks\"\n"; This is now: if [[ ${PLATFORM_NAME} != "macosx" ]]; then exit 0 fi if [[ ${WK_USE_OVERRIDE_FRAMEWORKS_DIR} == "YES" ]]; then exit 0 fi # ENABLE_WEB_RTC cannot be relied on because of its conditionals. if [[ ${CONFIGURATION} == "Production" && ${TARGET_MAC_OS_X_VERSION_MAJOR} != "101300" ]]; then exit 0 fi ln -shf Versions/Current/Frameworks "$TARGET_BUILD_DIR/WebCore.framework/Frameworks" Comment on attachment 303336 [details] [PATCH] Proposed Patch Attachment 303336 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3231559 New failing tests: media/modern-media-controls/tracks-support/tracks-support-show-panel-after-dragging-controls.html Created attachment 303344 [details]
Archive of layout-test-results from ews100 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 303336 [details] [PATCH] Proposed Patch View in context: https://bugs.webkit.org/attachment.cgi?id=303336&action=review >> Source/WebCore/WebCore.xcodeproj/project.pbxproj:29367 >> + shellScript = "if [[ ${PLATFORM_NAME} != \"macosx\" ]]; then\n exit 0\nfi\n\nif [[ ${WK_USE_OVERRIDE_FRAMEWORKS_DIR} == \"YES\" ]]; then\n exit 0\nfi\n\n# ENABLE_WEB_RTC cannot be relied on because of its conditionals.\nif [[ ${CONFIGURATION} == \"Production\" && ${TARGET_MAC_OS_X_VERSION_MAJOR} != \"101300\" ]]; then\n exit 0\nfi\n\nln -shf Versions/Current/Frameworks \"$TARGET_BUILD_DIR/WebCore.framework/Frameworks\"\n"; > > This is now: > > if [[ ${PLATFORM_NAME} != "macosx" ]]; then > exit 0 > fi > > if [[ ${WK_USE_OVERRIDE_FRAMEWORKS_DIR} == "YES" ]]; then > exit 0 > fi > > # ENABLE_WEB_RTC cannot be relied on because of its conditionals. > if [[ ${CONFIGURATION} == "Production" && ${TARGET_MAC_OS_X_VERSION_MAJOR} != "101300" ]]; then > exit 0 > fi > > ln -shf Versions/Current/Frameworks "$TARGET_BUILD_DIR/WebCore.framework/Frameworks" Someone might forget to update this when WebCore starts targeting newer macOS versions. Instead, I suggest that you use an inequality like ${TARGET_MAC_OS_X_VERSION_MAJOR} < 101300 . |