Bug 235663 – Generated xcfilelists contain symlinks when building for macOS

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

[Elliott Williams]

Our “Derived Sources” targets declare their input files through an automatically generated xcfilelist. (The filelist is created by examining Make’s database, and it “discovers” dependencies used to generate derived sources in a successful build.) These filelists may refer to headers from other targets. For example, here’s a generated xcfilelist with dependencies on scripts copied to WebCore’s private headers directory:

    > cat Tools/WebKitTestRunner/DerivedSources-input.xcfilelist | tail
    $(PROJECT_DIR)/InjectedBundle/Bindings/CodeGeneratorTestRunner.pm
    $(PROJECT_DIR)/InjectedBundle/Bindings/EventSendingController.idl
    $(PROJECT_DIR)/InjectedBundle/Bindings/GCController.idl
    $(PROJECT_DIR)/InjectedBundle/Bindings/TestRunner.idl
    $(PROJECT_DIR)/InjectedBundle/Bindings/TextInputController.idl
    $(PROJECT_DIR)/Scripts/PreferencesTemplates/TestOptionsGeneratedKeys.h.erb
    $(WEBCORE_PRIVATE_HEADERS_DIR)/CodeGenerator.pm
    $(WEBCORE_PRIVATE_HEADERS_DIR)/IDLAttributes.json
    $(WEBCORE_PRIVATE_HEADERS_DIR)/IDLParser.pm
    $(WEBCORE_PRIVATE_HEADERS_DIR)/generate-bindings.pl

At build time, $(WEBCORE_PRIVATE_HEADERS_DIR) is expanded like:

    Tools/WebKitTestRunner/Configurations/Base.xcconfig
    112:WEBCORE_PRIVATE_HEADERS_DIR = $(WEBCORE_PRIVATE_HEADERS_DIR_$(CONFIGURATION));
    113:WEBCORE_PRIVATE_HEADERS_DIR_Release = $(WEBCORE_PRIVATE_HEADERS_DIR_engineering);
    114:WEBCORE_PRIVATE_HEADERS_DIR_Debug = $(WEBCORE_PRIVATE_HEADERS_DIR_engineering);
    115:WEBCORE_PRIVATE_HEADERS_DIR_Production = $(PRODUCTION_FRAMEWORKS_DIR)/WebCore.framework/PrivateHeaders;
    116:WEBCORE_PRIVATE_HEADERS_DIR_engineering = $(BUILT_PRODUCTS_DIR)/WebCore.framework/PrivateHeaders;

The problem is that `WebCore.framework/PrivateHeaders` is a symlink to `WebCore.framework/Versions/A/PrivateHeaders` when building for macOS. XCBuild doesn’t appear to resolve symlinks, so it can’t find a task which produces the depended-on files, and the downstream target fails to build.

[Elliott Williams]

rdar://88054903

[Elliott Williams]

Created attachment 450057 [details]
Patch

[Elliott Williams]

Comment on attachment 450057 [details]
Patch

r-, need to resolve conflicts

[Elliott Williams]

Created attachment 450061 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 450061 [details]
Patch

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

> Source/WebCore/Configurations/Base.xcconfig:152
> +WK_VERSIONED_BUNDLE[sdk=macosx*] = Versions/A;

I wonder if it would make more sense to name this WK_CONTENT_PATH_IN_FRAMEWORK or something.

Also, if you make it "Versions/A/" then you can avoid having "//" in paths in the embedded case.

[Elliott Williams]

(In reply to Alexey Proskuryakov from comment #5)
> I wonder if it would make more sense to name this
> WK_CONTENT_PATH_IN_FRAMEWORK or something.
> 
> Also, if you make it "Versions/A/" then you can avoid having "//" in paths
> in the embedded case.

Bikeshedding, but would you be ok with WK_FRAMEWORK_VERSION_PREFIX? If I implement the latter suggestion, we end up with declarations like

    INSTALL_PATH = $(WEBKIT_FRAMEWORKS_DIR)/WebKit.framework/$(WK_FRAMEWORK_VERSION_PREFIX)XPCServices

and I feel like "prefix" reassures that we aren't missing a path separator there :)

[Alexey Proskuryakov]

Seems ok to me too. Probably the main aspect to consider is whether similarity in name  to WK_INSTALL_PATH_PREFIX makes it harder to think about these. But my proposal has the same problem, colliding with SYSTEM_CONTENT_PATH.

[Elliott Williams]

Created attachment 450151 [details]
Patch

[Elliott Williams]

Comment on attachment 450151 [details]
Patch

This patch renames the variable and implements your feedback; it shouldn't need additional review.

[EWS]

Committed r288692 (246490@main): <https://commits.webkit.org/246490@main>

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