Summary: | Add a build step to copy resources from WebKitAdditions as bundle resources in WebKit | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
Component: | WebKit Misc. | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aestes, commit-queue, joepeck, mitz, thorton, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Wenson Hsieh
2018-01-29 15:55:42 PST
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 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! 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). (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! Created attachment 332627 [details]
Patch for landing
Comment on attachment 332627 [details] Patch for landing Clearing flags on attachment: 332627 Committed r227771: <https://trac.webkit.org/changeset/227771> |