Bug 236720

Summary: Add LC_SOURCE_VERSION to compile targets
Product: WebKit Reporter: Saam Barati <saam>
Component: WebKit Misc.Assignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dino, emw, eric.carlson, ews-watchlist, hi, kbr, keith_miller, kondapallykalyan, mark.lam, msaboff, pangle, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
patch
msaboff: review+, ews-feeder: commit-queue-
patch for landing none

Description Saam Barati 2022-02-16 10:46:50 PST 
...
Comment 1 Saam Barati 2022-02-16 10:51:15 PST 
Created attachment 452212 [details]
WIP

Needs some reworking, as I misunderstood what "$(inherited)" does. "$(inherited)" doesn't inherit from something if it's defined in a file that's included by the filed you're in. It only inherits from the thing that precedes in the xcconfig hierarchy defined by the Xcode GUI.
Comment 2 Saam Barati 2022-02-16 10:51:39 PST 
<rdar://84984140>
Comment 3 Saam Barati 2022-02-17 14:55:03 PST 
Created attachment 452435 [details]
patch
Comment 4 EWS Watchlist 2022-02-17 14:56:41 PST 
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment 5 Michael Saboff 2022-02-17 15:30:07 PST 
Comment on attachment 452435 [details]
patch

r+

Check on the EWS build failures to make sure they are related to the patch.
Comment 6 Alexey Proskuryakov 2022-02-17 15:31:50 PST 
Comment on attachment 452435 [details]
patch

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

> Source/JavaScriptCore/Configurations/TestAPI.xcconfig:50
> +OTHER_LDFLAGS = $(inherited) $(SOURCE_VERSION_LDFLAGS);

The only thing that could be of concern AFAICT is whether any of these are overridden by OTHER_LDFLAGS variables lacking $(inherited) in other xcconfigs. I didn't immediately spot any.
Comment 7 Saam Barati 2022-02-17 17:15:36 PST 
Comment on attachment 452435 [details]
patch

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

>> Source/JavaScriptCore/Configurations/TestAPI.xcconfig:50
>> +OTHER_LDFLAGS = $(inherited) $(SOURCE_VERSION_LDFLAGS);
> 
> The only thing that could be of concern AFAICT is whether any of these are overridden by OTHER_LDFLAGS variables lacking $(inherited) in other xcconfigs. I didn't immediately spot any.

Right. I tried to apply my change in leaf xcconfig files, unless there was an "OTHER_LDFLAGS" already defined in a non leaf. In which case, I used that prior definition.
Comment 8 EWS 2022-02-17 22:40:56 PST 
ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.
Comment 9 Saam Barati 2022-02-17 23:13:28 PST 
Created attachment 452488 [details]
patch for landing
Comment 10 EWS 2022-02-18 00:18:11 PST 
Committed r290117 (247461@main): <https://commits.webkit.org/247461@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452488 [details].