Bug 144762 - Fix internal build configuration issues
Summary: Fix internal build configuration issues
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-05-07 14:45 PDT by Jake Nielsen
Modified: 2015-05-12 12:25 PDT (History)
4 users (show)

See Also:


Attachments
Cleans up xcconfig files in DumpRenderTree (11.21 KB, patch)
2015-05-07 14:47 PDT, Jake Nielsen
joepeck: review-
Details | Formatted Diff | Diff
Changes DebugRelease.xcconfig to be consistent with that of WebCore (11.23 KB, patch)
2015-05-07 16:56 PDT, Jake Nielsen
no flags Details | Formatted Diff | Diff
More bug fixes (thanks Joe!) (11.09 KB, patch)
2015-05-07 17:05 PDT, Jake Nielsen
ddkilzer: review-
ddkilzer: commit-queue-
Details | Formatted Diff | Diff
Addresses further comments (10.99 KB, patch)
2015-05-08 11:32 PDT, Jake Nielsen
no flags Details | Formatted Diff | Diff
Fixes a missed MAYBE -> PLATFORM (10.99 KB, patch)
2015-05-08 13:25 PDT, Jake Nielsen
ddkilzer: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Fixes the rest of the configuration issues with TestWebKitAPI and WebKitTestRunner (9.08 KB, patch)
2015-05-11 13:49 PDT, Jake Nielsen
no flags Details | Formatted Diff | Diff
Adds Changelog (12.10 KB, patch)
2015-05-11 14:03 PDT, Jake Nielsen
no flags Details | Formatted Diff | Diff
Also adds changelog (10.11 KB, patch)
2015-05-11 14:04 PDT, Jake Nielsen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jake Nielsen 2015-05-07 14:45:32 PDT
see rdar://problem/20641901 for details.
Comment 1 Jake Nielsen 2015-05-07 14:47:38 PDT
Created attachment 252631 [details]
Cleans up xcconfig files in DumpRenderTree

changelog not yet added. Posting for EWS to pick up.
Comment 2 Joseph Pecoraro 2015-05-07 16:54:57 PDT
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;
Comment 3 Jake Nielsen 2015-05-07 16:56:04 PDT
Created attachment 252651 [details]
Changes DebugRelease.xcconfig to be consistent with that of WebCore

Again, let's see what EWS thinks of this.
Comment 4 Joseph Pecoraro 2015-05-07 16:56:18 PDT
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?
Comment 5 Jake Nielsen 2015-05-07 17:04:31 PDT
(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.
Comment 6 Jake Nielsen 2015-05-07 17:05:55 PDT
Created attachment 252655 [details]
More bug fixes (thanks Joe!)
Comment 7 David Kilzer (:ddkilzer) 2015-05-08 09:27:47 PDT
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.
Comment 8 Jake Nielsen 2015-05-08 11:32:57 PDT
Created attachment 252733 [details]
Addresses further comments
Comment 9 Jake Nielsen 2015-05-08 13:20:05 PDT
(In reply to comment #8)
> Created attachment 252733 [details]
> Addresses further comments

Doh. jake-- for attention to detail.
Comment 10 Jake Nielsen 2015-05-08 13:25:02 PDT
Created attachment 252742 [details]
Fixes a missed MAYBE -> PLATFORM
Comment 11 David Kilzer (:ddkilzer) 2015-05-08 16:40:46 PDT
Comment on attachment 252742 [details]
Fixes a missed MAYBE -> PLATFORM

r=me
Comment 12 WebKit Commit Bot 2015-05-08 17:27:11 PDT
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
Comment 13 Jake Nielsen 2015-05-11 13:49:48 PDT
Created attachment 252888 [details]
Fixes the rest of the configuration issues with TestWebKitAPI and WebKitTestRunner
Comment 14 Jake Nielsen 2015-05-11 13:50:20 PDT
I'll also go rebase the first patch.
Comment 15 Jake Nielsen 2015-05-11 13:53:13 PDT
Oh... I see. I just didn't update the change logs.
Comment 16 Jake Nielsen 2015-05-11 14:03:32 PDT
Created attachment 252889 [details]
Adds Changelog
Comment 17 Jake Nielsen 2015-05-11 14:04:05 PDT
Created attachment 252890 [details]
Also adds changelog
Comment 18 WebKit Commit Bot 2015-05-11 18:56:18 PDT
Comment on attachment 252890 [details]
Also adds changelog

Clearing flags on attachment: 252890

Committed r184146: <http://trac.webkit.org/changeset/184146>
Comment 19 David Kilzer (:ddkilzer) 2015-05-12 11:37:56 PDT
Comment on attachment 252889 [details]
Adds Changelog

r=me
Comment 20 David Kilzer (:ddkilzer) 2015-05-12 11:39:32 PDT
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.
Comment 21 WebKit Commit Bot 2015-05-12 12:25:18 PDT
Comment on attachment 252889 [details]
Adds Changelog

Clearing flags on attachment: 252889

Committed r184209: <http://trac.webkit.org/changeset/184209>
Comment 22 WebKit Commit Bot 2015-05-12 12:25:22 PDT
All reviewed patches have been landed.  Closing bug.