REGRESSION (r229930, r231063): WebKitLegacy.xcconfig overwrites OTHER_CFLAGS, OTHER_CPLUSPLUSFLAGS and OTHER_LDFLAGS from Base.xcconfig

[David Kilzer (:ddkilzer)]

WebKitLegacy.xcconfig overwrites OTHER_CFLAGS, OTHER_CPLUSPLUSFLAGS and OTHER_LDFLAGS from Base.xcconfig.

These Xcode variables are defined like this in WebKitLegacy.xcconfig:

OTHER_CFLAGS = $(OTHER_CFLAGS_COCOA_TOUCH_$(WK_IS_COCOA_TOUCH));
[...]
OTHER_CPLUSPLUSFLAGS = $(OTHER_CFLAGS);
[...]
OTHER_LDFLAGS = -lobjc -lsqlite3 -framework CFNetwork -framework CoreFoundation -framework CoreGraphics -framework CoreText -framework Foundation -framework ImageIO -framework IOKit $(WK_APPKIT_LDFLAGS) $(WK_CARBON_LDFLAGS) $(WK_GRAPHICS_SERVICES_LDFLAGS) $(WK_MOBILE_CORE_SERVICES_LDFLAGS) $(WK_MOBILE_GESTALT_LDFLAGS) $(WK_SECURITY_INTERFACE_LDFLAGS) $(WK_WEBINSPECTORUI_LDFLAGS);

This overwrites the values defined in Base.xcconfig:

OTHER_CFLAGS = $(ASAN_OTHER_CFLAGS);
OTHER_CPLUSPLUSFLAGS = $(ASAN_OTHER_CPLUSPLUSFLAGS);
OTHER_LDFLAGS = $(ASAN_OTHER_LDFLAGS);

I found this because I was (ab)using Tools/asan/asan.xcconfig to set a compiler flag in every project by doing it in one place:

-ASAN_OTHER_CFLAGS = -fno-omit-frame-pointer -g;
+ASAN_OTHER_CFLAGS = -fno-omit-frame-pointer -g -O0 -DRELEASE_WITHOUT_OPTIMIZATIONS=1;

Then the WebKitLegacy project failed to compile because -DRELEASE_WITHOUT_OPTIMIZATIONS=1 was not set when building source with a Release configuration and compiler optimizations disabled.

[David Kilzer (:ddkilzer)]

The OTHER_C[PLUSPLUS]FLAGS change regressed in r231063 for Bug 185006.

The OTHER_LDFLAGS change regressed in r229930 for Bug 183948.

[David Kilzer (:ddkilzer)]

Created attachment 385458 [details]
Patch v1

[Alexey Proskuryakov]

Comment on attachment 385458 [details]
Patch v1

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

> Source/WebKitLegacy/mac/Configurations/WebKitLegacy.xcconfig:47
> +OTHER_CFLAGS_COCOA_TOUCH = $(OTHER_CFLAGS_COCOA_TOUCH_$(WK_IS_COCOA_TOUCH));

Is it always YES or NO, or can it be empty? If the latter, then we also need OTHER_CFLAGS_COCOA_TOUCH_ - which is typical for most variables in xcconfigs.

Your patch doesn't change this aspect, but worth taking the opportunity to confirm.

[David Kilzer (:ddkilzer)]

Comment on attachment 385458 [details]
Patch v1

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

>> Source/WebKitLegacy/mac/Configurations/WebKitLegacy.xcconfig:47
>> +OTHER_CFLAGS_COCOA_TOUCH = $(OTHER_CFLAGS_COCOA_TOUCH_$(WK_IS_COCOA_TOUCH));
> 
> Is it always YES or NO, or can it be empty? If the latter, then we also need OTHER_CFLAGS_COCOA_TOUCH_ - which is typical for most variables in xcconfigs.
> 
> Your patch doesn't change this aspect, but worth taking the opportunity to confirm.

WK_IS_COCOA_TOUCH is defined in Source/WebKitLegacy/mac/Configurations/Base.xcconfig:

WK_IS_COCOA_TOUCH = $(WK_NOT_$(WK_EMPTY_$(WK_COCOA_TOUCH)));

And the other macros are defined in Source/WebKitLegacy/mac/Configurations/SDKVariant.xcconfig:

WK_EMPTY_ = YES;
WK_NOT_ = YES;
WK_NOT_YES = NO;

So WK_IS_COCOA_TOUCH is always defined as YES or NO.

[WebKit Commit Bot]

Comment on attachment 385458 [details]
Patch v1

Clearing flags on attachment: 385458

Committed r253439: <https://trac.webkit.org/changeset/253439>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/57883449>