Bug 168819 - Simplify EXPORTED_SYMBOLS_FILE variables in WebKitLegacy.xcconfig
Summary: Simplify EXPORTED_SYMBOLS_FILE variables in WebKitLegacy.xcconfig
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-23 20:26 PST by Aakash Jain
Modified: 2017-02-26 16:18 PST (History)
6 users (show)

See Also:

Attachments
Proposed patch (1.85 KB, patch)
2017-02-23 20:31 PST, Aakash Jain 		mitz: review+
Details | Formatted Diff | Diff
Updated patch (1.85 KB, patch)
2017-02-23 21:08 PST, Aakash Jain 		no flags Details | Formatted Diff | Diff
Updated patch (3.36 KB, patch)
2017-02-23 22:07 PST, Aakash Jain 		mitz: review+
Details | Formatted Diff | Diff
Updated patch (3.37 KB, patch)
2017-02-24 00:36 PST, Aakash Jain 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 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.
Comment 1 Aakash Jain 2017-02-23 20:31:38 PST 
Created attachment 302633 [details]
Proposed patch
Comment 2 Aakash Jain 2017-02-23 21:08:01 PST 
Created attachment 302636 [details]
Updated patch
Comment 3 mitz 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;
Comment 4 Aakash Jain 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]).
Comment 5 mitz 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.
Comment 6 Aakash Jain 2017-02-24 00:36:11 PST 
Created attachment 302656 [details]
Updated patch
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2017-02-26 16:18:46 PST 
All reviewed patches have been landed.  Closing bug.