[webkit-reviews] review denied: [Bug 61073] Teach WebKit2 about window.internals : [Attachment 97230] Patch that uses @rpath

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has denied Dominic Cooney
<dominicc at chromium.org>'s request for review:
Bug 61073: Teach WebKit2 about window.internals
https://bugs.webkit.org/show_bug.cgi?id=61073

Attachment 97230: Patch that uses @rpath
https://bugs.webkit.org/attachment.cgi?id=97230&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=97230&action=review

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:26099
> +				INSTALL_PATH =
"$(INSTALL_PATH_$(REAL_PLATFORM_NAME))";

These should all be set in the .xcconfig file.	It’s not clear to me why the
Production one is set to this value either, when the others are set to @rpath.

> Tools/DumpRenderTree/DumpRenderTree.xcodeproj/project.pbxproj:939
> +				LD_RUNPATH_SEARCH_PATHS = "@executable_path/.";


As I mentioned in an earlier comment, it’s almost never useful to use
@executable_path instead of @loader_path.  Given that I think it would be
clearer to use @loader_path here.

These should live in the .xcconfig file as well.

> Tools/WebKitTestRunner/WebKitTestRunner.xcodeproj/project.pbxproj:98
> +		A664BC6813A5F18F009A7B25 /* libWebCoreTestSupport.dylib */ =
{isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; path =
libWebCoreTestSupport.dylib; sourceTree = BUILT_PRODUCTS_DIR; };

It looks like you’ve added libWebCoreTestSupport.dylib to the Xcode project
multiple times?

> Tools/WebKitTestRunner/WebKitTestRunner.xcodeproj/project.pbxproj:535
> +				LD_RUNPATH_SEARCH_PATHS =
"@loader_path/../../..";

These should live in the .xcconfig file.