Bug 223803 – [Cocoa] Add WebKitAdditions sources to project

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[Cocoa] Add WebKitAdditions sources to project

[Jer Noble]

Created attachment 424369 [details]
WIP

[Jer Noble]

Created attachment 424372 [details]
WIP

[Jer Noble]

Created attachment 424385 [details]
WIP

[Jer Noble]

Created attachment 424408 [details]
WIP

[Alexey Proskuryakov]

Comment on attachment 424408 [details]
WIP

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

> Source/WebKit/WebKit.xcodeproj/project.pbxproj:13777
> +			inputPaths = (
> +			);

Please define inputPaths and outputPaths, so that this phase doesn't have to run every time in incremental builds.

> Source/WebKit/WebKit.xcodeproj/project.pbxproj:13785
> +			shellScript = "set -e\n\nRELATIVE_SOURCE_PATH=\"usr/local/include/WebKitAdditions\"\nSOURCE_PATH=\"$SDK_DIR/$RELATIVE_SOURCE_PATH\"\nDESTINATION_PATH=\"$BUILT_PRODUCTS_DIR/$RELATIVE_SOURCE_PATH\"\n\nfor SOURCE in WKCoordinator.h WKCoordinator.mm WKGroupSession.h WKGroupSession.mm WKGroupSessionWrapper.swift; do\n    if [[ \"$SOURCE_PATH/$SOURCE\" -nt \"$DESTINATION_PATH/$SOURCE\" ]]; then\n        ditto \"$SOURCE_PATH/$SOURCE\" \"$DESTINATION_PATH/$SOURCE\"\n    fi\ndone\n";

I don't understand why we need to copy source files into BUILT_PRODUCTS_DIR, or why this cannot be done with a copy files phase.

[Jer Noble]

(In reply to Alexey Proskuryakov from comment #5)
> Comment on attachment 424408 [details]
> WIP
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=424408&action=review
> 
> > Source/WebKit/WebKit.xcodeproj/project.pbxproj:13777
> > +			inputPaths = (
> > +			);
> 
> Please define inputPaths and outputPaths, so that this phase doesn't have to
> run every time in incremental builds.

The issue here is the same as the one below:

> > Source/WebKit/WebKit.xcodeproj/project.pbxproj:13785
> > +			shellScript = "set -e\n\nRELATIVE_SOURCE_PATH=\"usr/local/include/WebKitAdditions\"\nSOURCE_PATH=\"$SDK_DIR/$RELATIVE_SOURCE_PATH\"\nDESTINATION_PATH=\"$BUILT_PRODUCTS_DIR/$RELATIVE_SOURCE_PATH\"\n\nfor SOURCE in WKCoordinator.h WKCoordinator.mm WKGroupSession.h WKGroupSession.mm WKGroupSessionWrapper.swift; do\n    if [[ \"$SOURCE_PATH/$SOURCE\" -nt \"$DESTINATION_PATH/$SOURCE\" ]]; then\n        ditto \"$SOURCE_PATH/$SOURCE\" \"$DESTINATION_PATH/$SOURCE\"\n    fi\ndone\n";
> 
> I don't understand why we need to copy source files into BUILT_PRODUCTS_DIR,
> or why this cannot be done with a copy files phase.

Project files are subject to the following limitation: their locations must not be variable, outside of the standard enumerated list of variable file locations: BUILT_PRODUCTS_DIR, SDK_DIR, PROJECT_DIR, etc. The issue with these files in particular is that they are either in BUILT_PRODUCTS_DIR or SDK_DIR depending on configuration and internal SDK status.  Hence both not being able to put those files directly in the project in their original location. This solution intends to work around that problem by collapsing the two possible locations of these files (SDK_DIR and BUILT_PRODUCTS_DIR) into one by copying the files in SDK_DIR into the BUILT_PRODUCTS_DIR only if they are newer.

A Copy Files build step would not be appropriate here, because these files should only be copied in particular configurations, and that's not something Copy Files can do.

I can try to define input and output files  (the output side is easier), but input files may need to be in the project, which would lead to duplicate filenames inside the project. But I'll see what I can do.

[Jer Noble]

(In reply to Jer Noble from comment #6)
> Project files are subject to the following limitation: their locations must
> not be variable, outside of the standard enumerated list of variable file
> locations: BUILT_PRODUCTS_DIR, SDK_DIR, PROJECT_DIR, etc. The issue with
> these files in particular is that they are either in BUILT_PRODUCTS_DIR or
> SDK_DIR depending on configuration and internal SDK status.  Hence both not
> being able to put those files directly in the project in their original
> location. This solution intends to work around that problem by collapsing
> the two possible locations of these files (SDK_DIR and BUILT_PRODUCTS_DIR)
> into one by copying the files in SDK_DIR into the BUILT_PRODUCTS_DIR only if
> they are newer.

Oh, and we can't just put both files in the project, because the technique of using EXCLUDED_SOURCE_FILE_NAMES depends on having uniquely named source files (not unique paths), and these would have the same names.

On the other hand, the inputPaths and outputPaths just take arbitrary strings with fully generated build variable support, so I've added these to the Copy step, and verified that the script does not run if the files haven't changed.

[Jer Noble]

Created attachment 424511 [details]
WIP

[Radar WebKit Bug Importer]

<rdar://problem/76004506>

[Jer Noble]

Created attachment 424692 [details]
Patch

[Jer Noble]

Created attachment 425338 [details]
Patch

[Eric Carlson]

Comment on attachment 425338 [details]
Patch

r=me once the bots are happy

[Jer Noble]

Created attachment 425407 [details]
Patch for landing

[Jer Noble]

Created attachment 425416 [details]
Patch for landing

[EWS]

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

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