Bug 237112 – [XCBuild] libwebrtc's headers are copied via rsync and do not emit task outputs

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Elliott Williams]

<rdar://89053575>

[Elliott Williams]

Created attachment 453043 [details]
Copy headers natively, add headers-only absl target

[Elliott Williams]

Created attachment 453046 [details]
Copy headers natively, add headers-only absl target r2

[Elliott Williams]

Created attachment 453047 [details]
/usr/local/include diff for r2

The pbxproj changes are too messy to meaningfully review, because of all the untracked files I had to add. To help reviewers, here's a diff of /usr/local/include after building libwebrtc, before and after this patch.

There are some header directories that I didn't bother adding to the project. I tried to limit this to directories that were obviously test-only or for non-Apple platforms. We'll see in testing if any of those headers *are* actually meaningful.

[Elliott Williams]

Created attachment 453069 [details]
Copy headers natively, add headers-only absl target r2

Removing a directory which got accidentally added twice.

[Elliott Williams]

Created attachment 453072 [details]
Copy headers natively, add headers-only absl target r3

[Elliott Williams]

Comment on attachment 453072 [details]
Copy headers natively, add headers-only absl target r3

Just rebased and rebuilt locally, and this is still fresh. cq+

[EWS]

Committed r290966 (248146@main): <https://commits.webkit.org/248146@main>

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

[Alex Christensen]

After this change, doing a clean build on Monterey I hit this issue using the public SDK:

/Users/alexchristensen/code/OpenSource/WebKitBuild/Debug/usr/local/include/webrtc/api/peer_connection_interface.h:78:10: fatal error: 'absl/base/attributes.h' file not found
#include "absl/base/attributes.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

[Alex Christensen]

I see that header in WebKitBuild/Debug/usr/local/include/base/attributes.h

[Elliott Williams]

(In reply to Alex Christensen from comment #8)
> After this change, doing a clean build on Monterey I hit this issue using
> the public SDK:
> 
> /Users/alexchristensen/code/OpenSource/WebKitBuild/Debug/usr/local/include/
> webrtc/api/peer_connection_interface.h:78:10: fatal error:
> 'absl/base/attributes.h' file not found
> #include "absl/base/attributes.h"
>          ^~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.

I can reproduce this too, looks like my testing was not sufficient. Reverting for now and will investigate more tomorrow. Thanks for letting me know!

[WebKit Commit Bot]

Re-opened since this is blocked by bug 237582

[Alex Christensen]

Reverted r290966 for reason:

Broke ews and make

Committed r290974 (248152@trunk): <https://commits.webkit.org/248152@trunk>

[Elliott Williams]

Created attachment 454171 [details]
Patch for relanding

libabsl.xcconfig was not setting PUBLIC_HEADERS_FOLDER_PATH, due to a bad copy-paste. This was causing build failures. I'm guessing I forgot to clean when I tested the patch yesterday, so I didn't notice the headers in the wrong location.

This patch runs s/PREFIX/PATH/g on libabsl.xcconfig, which fixes the issue:

	diff --git a/Source/ThirdParty/libwebrtc/Configurations/libabsl.xcconfig b/Source/ThirdParty/libwebrtc/Configurations/libabsl.xcconfig
	index 896b098ddc7d..57d61fe0fd01 100644
	--- a/Source/ThirdParty/libwebrtc/Configurations/libabsl.xcconfig
	+++ b/Source/ThirdParty/libwebrtc/Configurations/libabsl.xcconfig
	@@ -24,6 +24,6 @@
	 PRODUCT_NAME = absl;
	 
	 APPLY_RULES_IN_COPY_HEADERS = $(WK_USE_NEW_BUILD_SYSTEM);
	-PUBLIC_HEADERS_FOLDER_PREFIX = $(PUBLIC_HEADERS_FOLDER_PREFIX_$(WK_WHICH_BUILD_SYSTEM));
	-PUBLIC_HEADERS_FOLDER_PREFIX_not_legacy = /usr/local/include/absl;
	-PUBLIC_HEADERS_FOLDER_PREFIX_legacy = /usr/local/include/absl_flattened;
	+PUBLIC_HEADERS_FOLDER_PATH = $(PUBLIC_HEADERS_FOLDER_PATH_$(WK_WHICH_BUILD_SYSTEM));
	+PUBLIC_HEADERS_FOLDER_PATH_not_legacy = /usr/local/include/absl;
	+PUBLIC_HEADERS_FOLDER_PATH_legacy = /usr/local/include/absl_flattened;

[EWS]

Committed r291239 (248394@main): <https://commits.webkit.org/248394@main>

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

[Elliott Williams]

Reverted r291239 for reason:

Some builds failing with "fatal error: 'absl/utility/utility.h' file not found"

Committed r291249 (248402@trunk): <https://commits.webkit.org/248402@trunk>

[Elliott Williams]

My understanding of the problem is that libwebrtc has many quote-style includes of absl headers. This is fine when webrtc *itself* is building, because it puts absl on its HEADER_SEARCH_PATHS, but it's a problem for *other* project which import webrtc through a bracket-style include.

This causes WebCore to fail with an include trace like:

    In file included from Sources/WebKit/Source/WebCore/platform/graphics/cocoa/VP9UtilitiesCocoa.mm:32:
    In file included from Sources/WebKit/Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:44:
    In file included from SDKs/iPhoneOS16.0.Internal.sdk/usr/local/include/webrtc/api/peer_connection_interface.h:79:
    /usr/local/include/absl/types/optional.h:39:10: fatal error: 'absl/utility/utility.h' file not found

…because it doesn't have /usr/local/include on its include search path, only its *library* search path.

As I understand it, this is really just a disagreement between the two projects as to how we differentiate between "project" and "library" headers. The right move is probably to add /usr/local/include to WebCore's HEADER_SEARCH_PATHS, or to add absl headers back to the `libwebrtc` target, so that Xcode puts them into the appropriate header map.

[Elliott Williams]

Created attachment 456646 [details]
Patch

[Elliott Williams]

(In reply to Elliott Williams from comment #16)
> My understanding of the problem is that libwebrtc has many quote-style
> includes of absl headers. This is fine when webrtc *itself* is building,
> because it puts absl on its HEADER_SEARCH_PATHS, but it's a problem for
> *other* project which import webrtc through a bracket-style include.
> 
> This causes WebCore to fail with an include trace like:
> 
>     In file included from
> Sources/WebKit/Source/WebCore/platform/graphics/cocoa/VP9UtilitiesCocoa.mm:
> 32:
>     In file included from
> Sources/WebKit/Source/WebCore/platform/mediastream/libwebrtc/
> LibWebRTCProvider.h:44:
>     In file included from
> SDKs/iPhoneOS16.0.Internal.sdk/usr/local/include/webrtc/api/
> peer_connection_interface.h:79:
>     /usr/local/include/absl/types/optional.h:39:10: fatal error:
> 'absl/utility/utility.h' file not found
> 
> …because it doesn't have /usr/local/include on its include search path, only
> its *library* search path.

This was incorrect: By default, HEADER_SEARCH_PATHS variables (and -I flags) apply to both quote and bracket style includes. The former _also_ search the local directory and -iquote flags, which is what makes them different.

The real problem appears to be that I missed some headers which needed to be added, and didn't notice on engineering builds because of absl headers in the SDK.

[Alexey Proskuryakov]

Comment on attachment 456646 [details]
Patch

Sounds like this is the same approach as before but with more headers, rs=me.

[EWS]

Committed r292411 (249274@main): <https://commits.webkit.org/249274@main>

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