Bug 183576 - Add and adopt WK_ALTERNATE_FRAMEWORKS_DIR in WTF and bmalloc
Summary: Add and adopt WK_ALTERNATE_FRAMEWORKS_DIR in WTF and bmalloc
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-03-12 13:40 PDT by Tim Horton
Modified: 2018-03-13 16:18 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.39 KB, patch)
2018-03-12 16:54 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (4.22 KB, patch)
2018-03-13 12:10 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (7.22 KB, patch)
2018-03-13 14:41 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (7.21 KB, patch)
2018-03-13 16:03 PDT, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2018-03-12 13:40:28 PDT
Add and adopt WK_ALTERNATE_FRAMEWORKS_DIR in WTF
Comment 1 Radar WebKit Bug Importer 2018-03-12 16:46:17 PDT
<rdar://problem/38396766>
Comment 2 Tim Horton 2018-03-12 16:54:40 PDT
Created attachment 335664 [details]
Patch
Comment 3 Tim Horton 2018-03-12 19:14:02 PDT
This isn’t quite sufficient.
Comment 4 Lucas Forschler 2018-03-13 09:31:04 PDT
WKBI was failing to import smart quotes u2019. Attempted fix, let's see if this comment goes through now.
Comment 5 Tim Horton 2018-03-13 12:10:32 PDT
Created attachment 335709 [details]
Patch
Comment 6 mitz 2018-03-13 12:13:08 PDT
Comment on attachment 335709 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335709&action=review

> Source/WTF/Configurations/Base.xcconfig:137
> +WTF_INSTALL_PATH_PREFIX = $(WTF_INSTALL_PATH_PREFIX_$(WK_USE_ALTERNATE_FRAMEWORKS_DIR));
> +WTF_INSTALL_PATH_PREFIX_YES = $(WK_ALTERNATE_FRAMEWORKS_DIR)/;

Is this going to be defined differently in other projects in the stack? If not, then maybe this can be WK_INSTALL_PATH_PREFIX?
Comment 7 Tim Horton 2018-03-13 12:14:12 PDT
(In reply to mitz from comment #6)
> Comment on attachment 335709 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335709&action=review
> 
> > Source/WTF/Configurations/Base.xcconfig:137
> > +WTF_INSTALL_PATH_PREFIX = $(WTF_INSTALL_PATH_PREFIX_$(WK_USE_ALTERNATE_FRAMEWORKS_DIR));
> > +WTF_INSTALL_PATH_PREFIX_YES = $(WK_ALTERNATE_FRAMEWORKS_DIR)/;
> 
> Is this going to be defined differently in other projects in the stack? If
> not, then maybe this can be WK_INSTALL_PATH_PREFIX?

The other projects will probably do some dance with WK_OVERRIDE_FRAMEWORKS_DIR instead of this.
Comment 8 mitz 2018-03-13 12:59:52 PDT
Comment on attachment 335709 [details]
Patch

I think this is wrong for bmalloc reasons which we discussed in person.
Comment 9 Tim Horton 2018-03-13 14:41:21 PDT
Created attachment 335733 [details]
Patch
Comment 10 mitz 2018-03-13 14:48:00 PDT
Comment on attachment 335733 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335733&action=review

> Source/WTF/Configurations/Base.xcconfig:93
> +HEADER_SEARCH_PATHS = $(BUILT_PRODUCTS_DIR)/$(WTF_INSTALL_PATH_PREFIX)/usr/local/include $(DSTROOT)/$(WTF_INSTALL_PATH_PREFIX)/usr/local/include $(HEADER_SEARCH_PATHS);

I think you can count on WTF_INSTALL_PATH_PREFIX to begin with a / if it’s nonempty. And you may change $(HEADER_SEARCH_PATHS) to $(inherited)!

> Source/WTF/Configurations/Base.xcconfig:94
> +SYSTEM_HEADER_SEARCH_PATHS = $(inherited) $(SDK_DIR)/$(WTF_INSTALL_PATH_PREFIX)/usr/local/include;

Don’t we want to search in the alternate location first? Things may exist in both locations. Ditto about the leading /.

> Source/WTF/Configurations/Base.xcconfig:95
> +LIBRARY_SEARCH_PATHS = $(inherited) $(SDK_DIR)/$(WTF_INSTALL_PATH_PREFIX)/usr/local/lib;

Ditto on both counts.
Comment 11 Tim Horton 2018-03-13 16:03:03 PDT
Created attachment 335740 [details]
Patch
Comment 12 WebKit Commit Bot 2018-03-13 16:18:45 PDT
Comment on attachment 335740 [details]
Patch

Clearing flags on attachment: 335740

Committed r229591: <https://trac.webkit.org/changeset/229591>
Comment 13 WebKit Commit Bot 2018-03-13 16:18:47 PDT
All reviewed patches have been landed.  Closing bug.