Bug 182268 – Add a build step to copy resources from WebKitAdditions as bundle resources in WebKit

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Wenson Hsieh]

<rdar://problem/37003784>

[Wenson Hsieh]

Created attachment 332597 [details]
Patch

Comment on attachment 332597 [details]
Patch

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

> Source/WebKit/WebKit.xcodeproj/project.pbxproj:10022
> +			name = "Copy additional resources";

Why not use the same capitalization style as the other build phases?

> Source/WebKit/WebKit.xcodeproj/project.pbxproj:10027
> +			shellScript = "SOURCE=\"$SDKROOT/usr/local/include/WebKitAdditions/WebKit/AdditionalResources\"\nDESTINATION=\"$BUILT_PRODUCTS_DIR/WebKit.framework\"\nif [ -d \"$SOURCE\" ]\nthen\n    rsync -aq --exclude \".svn\" --exclude \".DS_Store\" $SOURCE/* \"$DESTINATION\"\nfi";

Some ideas on how to improve this:
- use $SDK_DIR instead of $SDKROOT
- account for local builds of WebKitAdditions in the built products directories before falling back on the SDK
- use ditto
- don’t behave as if the SDK may contain items named .svn or .DS_Store
- copy into $UNLOCALIZED_RESOURCES_FOLDER_PATH so that it doesn’t depend on the iOS shallow bundle structure
- use modern [[ instead of quirky [
- set -e

[Wenson Hsieh]

Comment on attachment 332597 [details]
Patch

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

>> Source/WebKit/WebKit.xcodeproj/project.pbxproj:10022
>> +			name = "Copy additional resources";
> 
> Why not use the same capitalization style as the other build phases?

Good catch! Should definitely be capitalized.

>> Source/WebKit/WebKit.xcodeproj/project.pbxproj:10027
>> +			shellScript = "SOURCE=\"$SDKROOT/usr/local/include/WebKitAdditions/WebKit/AdditionalResources\"\nDESTINATION=\"$BUILT_PRODUCTS_DIR/WebKit.framework\"\nif [ -d \"$SOURCE\" ]\nthen\n    rsync -aq --exclude \".svn\" --exclude \".DS_Store\" $SOURCE/* \"$DESTINATION\"\nfi";
> 
> Some ideas on how to improve this:
> - use $SDK_DIR instead of $SDKROOT
> - account for local builds of WebKitAdditions in the built products directories before falling back on the SDK
> - use ditto
> - don’t behave as if the SDK may contain items named .svn or .DS_Store
> - copy into $UNLOCALIZED_RESOURCES_FOLDER_PATH so that it doesn’t depend on the iOS shallow bundle structure
> - use modern [[ instead of quirky [
> - set -e

- switched to $SDK_DIR
- changed to try and find additional resources in the built products directory first, then fall back to SDK
- changed to use ditto.
- removed unneeded .svn and .DS_Store exclusion
- switched to use $UNLOCALIZED_RESOURCES_FOLDER_PATH
- changed to [[
- added `set -e` at the beginning

Thanks for the suggestions!

[Wenson Hsieh]

Created attachment 332624 [details]
Patch

Comment on attachment 332624 [details]
Patch

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

> Source/WebKit/WebKit.xcodeproj/project.pbxproj:10022
> +			shellScript = "set -e\n\nRELATIVE_SOURCE_PATH=\"usr/local/include/WebKitAdditions/WebKit/AdditionalResources\"\nSOURCE_PATH=\"$BUILT_PRODUCTS_DIR/$RELATIVE_SOURCE_PATH\"\n\nif [[ ! -d \"$SOURCE_PATH\" ]]; then\n    SOURCE_PATH=\"$SDK_DIR/$RELATIVE_SOURCE_PATH\"\nfi\n\nif [[ -d \"$SOURCE_PATH\" ]]; then\n    ditto $SOURCE_PATH/* \"$BUILT_PRODUCTS_DIR/$UNLOCALIZED_RESOURCES_FOLDER_PATH\"\nfi";

I think the * there is unnecessary (and harmful if the source directory ever contained subdirectories).

[Wenson Hsieh]

(In reply to mitz from comment #5)
> Comment on attachment 332624 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332624&action=review
> 
> > Source/WebKit/WebKit.xcodeproj/project.pbxproj:10022
> > +			shellScript = "set -e\n\nRELATIVE_SOURCE_PATH=\"usr/local/include/WebKitAdditions/WebKit/AdditionalResources\"\nSOURCE_PATH=\"$BUILT_PRODUCTS_DIR/$RELATIVE_SOURCE_PATH\"\n\nif [[ ! -d \"$SOURCE_PATH\" ]]; then\n    SOURCE_PATH=\"$SDK_DIR/$RELATIVE_SOURCE_PATH\"\nfi\n\nif [[ -d \"$SOURCE_PATH\" ]]; then\n    ditto $SOURCE_PATH/* \"$BUILT_PRODUCTS_DIR/$UNLOCALIZED_RESOURCES_FOLDER_PATH\"\nfi";
> 
> I think the * there is unnecessary (and harmful if the source directory ever
> contained subdirectories).

Ah, I wasn't aware that ditto had different semantics than rsync when the source is a folder. Fixed!

[Wenson Hsieh]

Created attachment 332627 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 332627 [details]
Patch for landing

Clearing flags on attachment: 332627

Committed r227771: <https://trac.webkit.org/changeset/227771>

[Radar WebKit Bug Importer]

<rdar://problem/37015528>