Bug 168819

Summary: Simplify EXPORTED_SYMBOLS_FILE variables in WebKitLegacy.xcconfig
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: WebKit Misc.Assignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, commit-queue, juergen, mitz, thorton
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=168758
Attachments:
Description Flags
Proposed patch
mitz: review+
Updated patch
none
Updated patch
mitz: review+
Updated patch none

Aakash Jain
Reported 2017-02-23 20:26:56 PST
Mac 32-bit (i386) can not use Objective C symbols (starting with _OBJC_IVAR) in the exp file. Therefore it uses separate symbol file WebKitLegacy.i386.exp. However iphonesimulator i386 doesn't have any restriction and it can use the regular WebKitLegacy.generated.exp file.
Attachments
Proposed patch (1.85 KB, patch)
2017-02-23 20:31 PST, Aakash Jain
mitz: review+
Updated patch (1.85 KB, patch)
2017-02-23 21:08 PST, Aakash Jain
no flags
Updated patch (3.36 KB, patch)
2017-02-23 22:07 PST, Aakash Jain
mitz: review+
Updated patch (3.37 KB, patch)
2017-02-24 00:36 PST, Aakash Jain
no flags
Aakash Jain
Comment 1 2017-02-23 20:31:38 PST
Created attachment 302633 [details] Proposed patch
Aakash Jain
Comment 2 2017-02-23 21:08:01 PST
Created attachment 302636 [details] Updated patch
mitz
Comment 3 2017-02-23 21:15:38 PST
Comment on attachment 302636 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=302636&action=review > Source/WebKit/mac/Configurations/WebKitLegacy.xcconfig:39 > EXPORTED_SYMBOLS_FILE_armv7k = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.generated.exp; > EXPORTED_SYMBOLS_FILE_armv7s = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.generated.exp; > EXPORTED_SYMBOLS_FILE_arm64 = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.generated.exp; > -EXPORTED_SYMBOLS_FILE_i386 = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.i386.exp; > +EXPORTED_SYMBOLS_FILE_i386[sdk=*simulator*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.generated.exp; > +EXPORTED_SYMBOLS_FILE_i386[sdk=macosx*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.i386.exp; > EXPORTED_SYMBOLS_FILE_x86_64[sdk=iphonesimulator*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.generated.exp; > EXPORTED_SYMBOLS_FILE_x86_64[sdk=macosx*] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.LP64.exp; I know that you are just trying to fix a small problem, but looking at this, I notice that the list doesn’t cover 64-bit non-iPhone simulators, and that the entire construct is way over-complicated. If I’m reading this correctly, there are three exports files we use: WebKitLegacy.generated.exp, WebKitLegacy.i386.exp, and WebKitLegacy.LP64.exp. Weird naming scheme. It also looks like WebKitLegacy.generated.exp is the default. Here’s how I’d rewrite this entire section of the .xcconfig file (lines 30-39): EXPORTED_SYMBOLS_FILE = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.generated.exp; EXPORTED_SYMBOLS_FILE[sdk=macosx] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.LP64.exp; EXPORTED_SYMBOLS_FILE[sdk=macosx][arch=i386] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.i386.exp;
Aakash Jain
Comment 4 2017-02-23 22:07:10 PST
Created attachment 302643 [details] Updated patch Agree with you. It simplifies the code quite a bit and make it more readable as well. I made a small change for WebKitLegacy.LP64.exp file (added [arch=x86_64]).
mitz
Comment 5 2017-02-23 22:09:23 PST
Comment on attachment 302643 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=302643&action=review > Source/WebKit/mac/Configurations/WebKitLegacy.xcconfig:32 > +EXPORTED_SYMBOLS_FILE[sdk=macosx][arch=x86_64] = $(BUILT_PRODUCTS_DIR)/DerivedSources/WebKitLegacy/WebKitLegacy.LP64.exp; Neat! r=me once you see that the bots are OK with this.
Aakash Jain
Comment 6 2017-02-24 00:36:11 PST
Created attachment 302656 [details] Updated patch
WebKit Commit Bot
Comment 7 2017-02-26 16:18:40 PST
Comment on attachment 302656 [details] Updated patch Clearing flags on attachment: 302656 Committed r213017: <http://trac.webkit.org/changeset/213017>
WebKit Commit Bot
Comment 8 2017-02-26 16:18:46 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.