Bug 222346 – Fix linker warnings building gtest for macCatalyst

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

Fix linker warnings building gtest for macCatalyst

[Tim Horton]

Created attachment 421388 [details]
Patch

[Tim Horton]

<rdar://problem/74405116>

[Alexey Proskuryakov]

Comment on attachment 421388 [details]
Patch

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

r=me too, with comments.

> Source/ThirdParty/gtest/xcode/Config/FrameworkTarget.xcconfig:22
> +INSTALL_PATH_SDK_VARIANT_macos = $(MACOS_UNZIPPERED_TWIN_PRIVATE_FRAMEWORK_INSTALL_PATH);
> +INSTALL_PATH_SDK_VARIANT_iosmac = $(IOS_UNZIPPERED_TWIN_PRIVATE_FRAMEWORK_INSTALL_PATH);

This is not how we do this elsewhere (cf. WK_ALTERNATE_FRAMEWORKS_DIR). Do we need to modernize all other projects to use these constants? Do they exist on all OS versions we need?

> Source/ThirdParty/gtest/xcode/Config/General.xcconfig:11
> +#include "<DEVELOPER_DIR>/AppleInternal/XcodeConfig/PlatformSupport.xcconfig"

Shouldn't this be "include?"? It is in other projects, and obviously not everyone has it.

[Alexey Proskuryakov]

Also, I don't imagine that removing INSTALL_PATH altogether fixes the problem - or does it?

[Tim Horton]

Comment on attachment 421388 [details]
Patch

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

>> Source/ThirdParty/gtest/xcode/Config/FrameworkTarget.xcconfig:22
>> +INSTALL_PATH_SDK_VARIANT_iosmac = $(IOS_UNZIPPERED_TWIN_PRIVATE_FRAMEWORK_INSTALL_PATH);
> 
> This is not how we do this elsewhere (cf. WK_ALTERNATE_FRAMEWORKS_DIR). Do we need to modernize all other projects to use these constants? Do they exist on all OS versions we need?

They do exist where we need them, and I don't think any of the other parts of this matter?

>> Source/ThirdParty/gtest/xcode/Config/General.xcconfig:11
>> +#include "<DEVELOPER_DIR>/AppleInternal/XcodeConfig/PlatformSupport.xcconfig"
> 
> Shouldn't this be "include?"? It is in other projects, and obviously not everyone has it.

Yes, this is a good point.

[Tim Horton]

(In reply to Alexey Proskuryakov from comment #4)
> Also, I don't imagine that removing INSTALL_PATH altogether fixes the
> problem - or does it?

I have not tried that!

[Tim Horton]

(In reply to Tim Horton from comment #6)
> (In reply to Alexey Proskuryakov from comment #4)
> > Also, I don't imagine that removing INSTALL_PATH altogether fixes the
> > problem - or does it?
> 
> I have not tried that!

It does!

[Tim Horton]

Created attachment 421435 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 421435 [details]
Patch

Yay.

[EWS]

Committed r273430: <https://commits.webkit.org/r273430>

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

[Alexey Proskuryakov]

> They do exist where we need them, and I don't think any of the other parts of this matter?

Doesn't matter for landing the original patch, definitely doesn't matter for the final patch, matters for understanding what future cleanup is ahead of us. Using multiple ways to do the same thing forever wouldn't have been great.