Bug 135006

Summary:

SECTORDER_FLAGS should be defined in target's xcconfig file, not Base.xcconfig

Product:

WebKit

Reporter:

David Kilzer (:ddkilzer) <ddkilzer>

Component:

Tools / Tests

Assignee:

David Kilzer (:ddkilzer) <ddkilzer>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, beidson, commit-queue, darin, ddkilzer, dino, mitz, mrowe, pdr, psolanki

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch v1	none

Description David Kilzer (:ddkilzer) 2014-07-16 20:18:33 PDT

Building all targets with the same SECTORDER_FLAGS makes no sense because *.order files are specific to a target.

We should consider moving the SECTORDER_FLAGS variables to the target xcconfig in each project that will use them.

Comment 1 David Kilzer (:ddkilzer) 2014-07-17 15:29:10 PDT

Created attachment 235095 [details]
Patch v1

Comment 2 WebKit Commit Bot 2014-07-17 21:00:07 PDT

Comment on attachment 235095 [details]
Patch v1

Clearing flags on attachment: 235095

Committed r171212: <http://trac.webkit.org/changeset/171212>

Comment 3 WebKit Commit Bot 2014-07-17 21:00:12 PDT

All reviewed patches have been landed.  Closing bug.

Comment 4 Philip Rogers 2014-07-20 10:28:09 PDT

No review?

I think this breaks the external build:
=== BUILD TARGET llvmForJSC OF PROJECT JavaScriptCore WITH CONFIGURATION Debug ===

Check dependencies
Base.xcconfig line 23: Unable to find included file "iOS.xcconfig"
Base.xcconfig line 24: Unable to find included file "../../../../Internal/Configurations/UseInternalSDK.xcconfig"
The file “LLVMForJSC.xcconfig” couldn’t be opened because there is no such file. (/Users/pdr/Desktop/WebKit/Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig)
Build setting PRODUCT_NAME undefined

** BUILD FAILED **

Comment 5 mitz 2014-07-20 10:33:40 PDT

(In reply to comment #4)
> No review?

<https://bugs.webkit.org/show_activity.cgi?id=135006> shows that Darin Adler has reviewed the patch.

> 
> I think this breaks the external build:
> === BUILD TARGET llvmForJSC OF PROJECT JavaScriptCore WITH CONFIGURATION Debug ===
> 
> Check dependencies
> Base.xcconfig line 23: Unable to find included file "iOS.xcconfig"
> Base.xcconfig line 24: Unable to find included file "../../../../Internal/Configurations/UseInternalSDK.xcconfig"
> The file “LLVMForJSC.xcconfig” couldn’t be opened because there is no such file. (/Users/pdr/Desktop/WebKit/Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig)
> Build setting PRODUCT_NAME undefined
> 
> ** BUILD FAILED **

The above failure doesn’t appear to be related to the patch. It’s failing because it can’t find LLVMForJSC.xcconfig, but that file wasn’t touched.

Comment 6 Darin Adler 2014-07-20 10:46:52 PDT

(In reply to comment #4)
> No review?

I reviewed this.

> I think this breaks the external build:

I don’t think so. As Dan says, that problem you are seeing is something different.

Comment 7 Philip Rogers 2014-07-20 11:08:36 PDT

(In reply to comment #6)
> (In reply to comment #4)
> > No review?
> 
> I reviewed this.
> 
> > I think this breaks the external build:
> 
> I don’t think so. As Dan says, that problem you are seeing is something different.

I apologize, this patch was fine. I blame jetlag :(

I clicked "formatted diff" instead of the patch that landed, and had a local change which caused the failure.