Bug 223803 - [Cocoa] Add WebKitAdditions sources to project
Summary: [Cocoa] Add WebKitAdditions sources to project
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-26 10:04 PDT by Jer Noble
Modified: 2021-04-07 11:20 PDT (History)
2 users (show)

See Also:


Attachments
WIP (12.75 KB, patch)
2021-03-26 10:05 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
WIP (12.75 KB, patch)
2021-03-26 10:17 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
WIP (12.75 KB, patch)
2021-03-26 11:47 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
WIP (13.94 KB, patch)
2021-03-26 14:45 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
WIP (15.66 KB, patch)
2021-03-28 15:11 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (17.87 KB, patch)
2021-03-30 14:04 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (31.66 KB, patch)
2021-04-06 17:10 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (28.80 KB, patch)
2021-04-07 09:12 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (32.50 KB, patch)
2021-04-07 10:25 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2021-03-26 10:04:56 PDT
[Cocoa] Add WebKitAdditions sources to project
Comment 1 Jer Noble 2021-03-26 10:05:48 PDT
Created attachment 424369 [details]
WIP
Comment 2 Jer Noble 2021-03-26 10:17:25 PDT
Created attachment 424372 [details]
WIP
Comment 3 Jer Noble 2021-03-26 11:47:47 PDT
Created attachment 424385 [details]
WIP
Comment 4 Jer Noble 2021-03-26 14:45:33 PDT
Created attachment 424408 [details]
WIP
Comment 5 Alexey Proskuryakov 2021-03-26 16:43:58 PDT
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.
Comment 6 Jer Noble 2021-03-28 14:42:56 PDT
(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.
Comment 7 Jer Noble 2021-03-28 15:11:19 PDT
(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.
Comment 8 Jer Noble 2021-03-28 15:11:38 PDT
Created attachment 424511 [details]
WIP
Comment 9 Radar WebKit Bug Importer 2021-03-30 08:34:57 PDT
<rdar://problem/76004506>
Comment 10 Jer Noble 2021-03-30 14:04:03 PDT
Created attachment 424692 [details]
Patch
Comment 11 Jer Noble 2021-04-06 17:10:30 PDT
Created attachment 425338 [details]
Patch
Comment 12 Eric Carlson 2021-04-06 17:16:14 PDT
Comment on attachment 425338 [details]
Patch

r=me once the bots are happy
Comment 13 Jer Noble 2021-04-07 09:12:28 PDT
Created attachment 425407 [details]
Patch for landing
Comment 14 Jer Noble 2021-04-07 10:25:43 PDT
Created attachment 425416 [details]
Patch for landing
Comment 15 EWS 2021-04-07 11:20:32 PDT
Committed r275614: <https://commits.webkit.org/r275614>

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