Bug 144762 – Fix internal build configuration issues

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jake Nielsen]

see rdar://problem/20641901 for details.

[Jake Nielsen]

Created attachment 252631 [details]
Cleans up xcconfig files in DumpRenderTree

changelog not yet added. Posting for EWS to pick up.

[Joseph Pecoraro]

Comment on attachment 252631 [details]
Cleans up xcconfig files in DumpRenderTree

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

> Tools/DumpRenderTree/mac/Configurations/DebugRelease.xcconfig:36
> +MACOSX_DEPLOYMENT_TARGET[sdk=iphone*] = 10.9;

We should be able to drop this for iOS. I don't see it in WebCore for iOS and it doesn't really make sense to have this on iOS.

> Tools/DumpRenderTree/mac/Configurations/DebugRelease.xcconfig:48
> -SDKROOT = $(SDKROOT_$(PLATFORM_NAME));
> -SDKROOT_iphoneos = $(SDKROOT);
> -SDKROOT_iphonesimulator = $(SDKROOT);
> -SDKROOT_macosx = $(SDKROOT_macosx_$(USE_INTERNAL_SDK));
> +SDKROOT[sdk=macosx*] = $(SDKROOT_macosx_$(USE_INTERNAL_SDK));
>  SDKROOT_macosx_ = macosx;
>  SDKROOT_macosx_YES = macosx.internal;

I don't know why this doesn't work (mac builders failing). But the same change was made to WebCore's DebugRelease.xcconfig ended up with this:

    SDKROOT[sdk=iphone*] = $(SDKROOT);
    SDKROOT = $(SDKROOT_$(PLATFORM_NAME)_$(USE_INTERNAL_SDK));
    SDKROOT_macosx_ = macosx;
    SDKROOT_macosx_YES = macosx.internal;

Should work here too then!

> Tools/DumpRenderTree/mac/Configurations/LayoutTestHelper.xcconfig:29
> -OTHER_LDFLAGS_macosx = -framework Carbon -framework Cocoa -framework OpenGL -framework IOKit;
> +OTHER_LDFLAGS = $(inherited) $(MAYBE_OTHER_LDFLAGS);
> +MAYBE_OTHER_LDFLAGS[sdk=macosx*] = -framework Carbon -framework Cocoa -framework OpenGL -framework IOKit;

I think you can simplify this to just:

    OTHER_LDFLAGS[sdk=macosx*] = $(inherited) -framework Carbon -framework Cocoa -framework OpenGL -framework IOKit;

[Jake Nielsen]

Created attachment 252651 [details]
Changes DebugRelease.xcconfig to be consistent with that of WebCore

Again, let's see what EWS thinks of this.

[Joseph Pecoraro]

Comment on attachment 252631 [details]
Cleans up xcconfig files in DumpRenderTree

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

> Tools/DumpRenderTree/mac/Configurations/Base.xcconfig:32
> +FRAMEWORK_SEARCH_PATHS[sdk=macosx*] = $(interited);

Interesting.... "interited" is a typo that existed before your change. Maybe we can just drop this for macosx?

[Jake Nielsen]

(In reply to comment #4)
> Comment on attachment 252631 [details]
> Cleans up xcconfig files in DumpRenderTree
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=252631&action=review
> 
> > Tools/DumpRenderTree/mac/Configurations/Base.xcconfig:32
> > +FRAMEWORK_SEARCH_PATHS[sdk=macosx*] = $(interited);
> 
> Interesting.... "interited" is a typo that existed before your change. Maybe
> we can just drop this for macosx?
Hahaha, nice.

[Jake Nielsen]

Created attachment 252655 [details]
More bug fixes (thanks Joe!)

[David Kilzer (:ddkilzer)]

Comment on attachment 252655 [details]
More bug fixes (thanks Joe!)

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

Please make corrections above and post a new patch.

> Tools/DumpRenderTree/mac/Configurations/Base.xcconfig:30
> +HEADER_SEARCH_PATHS = $(BUILT_PRODUCTS_DIR)/usr/local/include $(BUILT_PRODUCTS_DIR)/WebCoreTestSupport ForwardingHeaders $(MAYBE_HEADER_SEARCH_PATHS) $(SRCROOT)/../../Source/JavaScriptCore/icu;
> +MAYBE_HEADER_SEARCH_PATHS[sdk=iphone*] = $(SDKROOT)/usr/local/include $(SDKROOT)/usr/local/include/WebCoreTestSupport $(SRCROOT)/../../Source/WebKit2/Platform/spi/ios;

Please rename MAYBE_HEADER_SEARCH_PATHS to PLATFORM_HEADER_SEARCH_PATHS.  I think "PLATFORM" is more descriptive than "MAYBE" here.

> Tools/DumpRenderTree/mac/Configurations/Base.xcconfig:31
> +MAYBE_HEADER_SEARCH_PATHS[sdk=macosx*] = mac/InternalHeaders $(NEXT_ROOT)/usr/local/include/WebCoreTestSupport $(BUILT_PRODUCTS_DIR)/usr/local/include $(BUILT_PRODUCTS_DIR)/WebCoreTestSupport ForwardingHeaders $(SRCROOT)/../../Source/JavaScriptCore/icu;

This doesn't need to be this long; it's duplicating paths in HEADER_SEARCH_PATHS:

PLATFORM_HEADER_SEARCH_PATHS[sdk=macosx*] = mac/InternalHeaders $(NEXT_ROOT)/usr/local/include/WebCoreTestSupport;

> Tools/DumpRenderTree/mac/Configurations/DebugRelease.xcconfig:31
> +MACOSX_DEPLOYMENT_TARGET[sdk=macosx*] = $(MACOSX_DEPLOYMENT_TARGET_$(PLATFORM_NAME));

It's redundant to use $(PLATFORM_NAME) here when [sdk=macosx*] will ensure that PLATFORM_NAME=macosx.  I would just say:

MACOSX_DEPLOYMENT_TARGET[sdk=macosx*] = $(MACOSX_DEPLOYMENT_TARGET_macosx);

And, in fact, you can merge this line with the next line to make:

MACOSX_DEPLOYMENT_TARGET[sdk=macosx*] = $(MACOSX_DEPLOYMENT_TARGET_macosx_$(TARGET_MAC_OS_X_VERSION_MAJOR));

> Tools/DumpRenderTree/mac/Configurations/DebugRelease.xcconfig:37
> +WEBKIT_SYSTEM_INTERFACE_LIBRARY[sdk=macosx*] = $(WEBKIT_SYSTEM_INTERFACE_LIBRARY_$(PLATFORM_NAME));

Similarly here you can merge this line with the next line:

WEBKIT_SYSTEM_INTERFACE_LIBRARY[sdk=macosx*] = $(WEBKIT_SYSTEM_INTERFACE_LIBRARY_macosx_$(TARGET_MAC_OS_X_VERSION_MAJOR));

> Tools/DumpRenderTree/mac/Configurations/DumpRenderTreeApp.xcconfig:30
> +OTHER_LDFLAGS = $(MAYBE_OTHER_LDFLAGS) $(ASAN_OTHER_LDFLAGS);

Please use PLATFORM_OTHER_LDFLAGS instead of MAYBE_OTHER_LDFLAGS.

> Tools/DumpRenderTree/mac/Configurations/DumpRenderTreeApp.xcconfig:31
> +MAYBE_OTHER_LDFLAGS[sdk=iphone*] = -l$(WEBKIT_SYSTEM_INTERFACE_LIBRARY) -lWebCoreTestSupport -force_load $(BUILT_PRODUCTS_DIR)/libDumpRenderTree.a -framework QuartzCore -framework CFNetwork -framework CoreFoundation -framework CoreGraphics -framework Foundation -framework GraphicsServices -framework ImageIO -framework MobileCoreServices -framework UIKit -framework WebCore -framework JavaScriptCore -framework WebKit $(OTHER_LDFLAGS_FONTS) $(ASAN_OTHER_LDFLAGS);

No need to include $(ASAN_OTHER_LDFLAGS) here as it's redundant with OTHER_LDFLAGS.

[Jake Nielsen]

Created attachment 252733 [details]
Addresses further comments

[Jake Nielsen]

(In reply to comment #8)
> Created attachment 252733 [details]
> Addresses further comments

Doh. jake-- for attention to detail.

[Jake Nielsen]

Created attachment 252742 [details]
Fixes a missed MAYBE -> PLATFORM

[David Kilzer (:ddkilzer)]

Comment on attachment 252742 [details]
Fixes a missed MAYBE -> PLATFORM

r=me

[WebKit Commit Bot]

Comment on attachment 252742 [details]
Fixes a missed MAYBE -> PLATFORM

Rejecting attachment 252742 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 252742, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
0-ab3c-d52691b4dbfc ...
Currently at 184028 = d808ebf589728ffc009c6c687539c55e5d5ec7a3
r184029 = eea6ff1eb5f67df8590c635db01097dba1419f4a
r184030 = d04196d1994efc793d5ef27392294683cb123f2c
r184031 = 351bad2a2f389c25da626b242206567204b5914d
r184032 = 661530a58a219e86c5eb19db94607bed1d83a9c1
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.appspot.com/results/6342543956508672

[Jake Nielsen]

Created attachment 252888 [details]
Fixes the rest of the configuration issues with TestWebKitAPI and WebKitTestRunner

[Jake Nielsen]

I'll also go rebase the first patch.

[Jake Nielsen]

Oh... I see. I just didn't update the change logs.

[Jake Nielsen]

Created attachment 252889 [details]
Adds Changelog

[Jake Nielsen]

Created attachment 252890 [details]
Also adds changelog

[WebKit Commit Bot]

Comment on attachment 252890 [details]
Also adds changelog

Clearing flags on attachment: 252890

Committed r184146: <http://trac.webkit.org/changeset/184146>

[David Kilzer (:ddkilzer)]

Comment on attachment 252889 [details]
Adds Changelog

r=me

[David Kilzer (:ddkilzer)]

Comment on attachment 252889 [details]
Adds Changelog

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

> Tools/DumpRenderTree/mac/Configurations/DumpRenderTreeApp.xcconfig:31
> -OTHER_LDFLAGS = $(OTHER_LDFLAGS_$(PLATFORM_NAME)) $(ASAN_OTHER_LDFLAGS);
> -OTHER_LDFLAGS_iphoneos = $(inherited) -l$(WEBKIT_SYSTEM_INTERFACE_LIBRARY) -lWebCoreTestSupport -force_load $(BUILT_PRODUCTS_DIR)/libDumpRenderTree.a -framework QuartzCore -framework CFNetwork -framework CoreFoundation -framework CoreGraphics -framework Foundation -framework GraphicsServices -framework ImageIO -framework MobileCoreServices -framework UIKit -framework WebCore -framework JavaScriptCore -framework WebKit $(OTHER_LDFLAGS_FONTS);
> -OTHER_LDFLAGS_iphonesimulator = $(OTHER_LDFLAGS_iphoneos);
> +OTHER_LDFLAGS = $(PLATFORM_OTHER_LDFLAGS) $(ASAN_OTHER_LDFLAGS);
> +PLATFORM_OTHER_LDFLAGS[sdk=iphone*] = -l$(WEBKIT_SYSTEM_INTERFACE_LIBRARY) -lWebCoreTestSupport -force_load $(BUILT_PRODUCTS_DIR)/libDumpRenderTree.a -framework QuartzCore -framework CFNetwork -framework CoreFoundation -framework CoreGraphics -framework Foundation -framework GraphicsServices -framework ImageIO -framework MobileCoreServices -framework UIKit -framework WebCore -framework JavaScriptCore -framework WebKit $(OTHER_LDFLAGS_FONTS);

OTHER_LDFLAGS should probably include $(inherited) here to match other *.xcconfig files (and to pick up OTHER_LDFLAGS settings prior to this xcconfig file):

OTHER_LDFLAGS = $(inherited) $(PLATFORM_OTHER_LDFLAGS) $(ASAN_OTHER_LDFLAGS);

This can be fixed in a follow-up patch.

[WebKit Commit Bot]

Comment on attachment 252889 [details]
Adds Changelog

Clearing flags on attachment: 252889

Committed r184209: <http://trac.webkit.org/changeset/184209>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.