Bug 198984 - Relocate some test tools in non-mac builds
Summary: Relocate some test tools in non-mac builds
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Rollin
Keywords: InRadar
Depends on:
Reported: 2019-06-18 17:02 PDT by Keith Rollin
Modified: 2019-06-19 17:14 PDT (History)
6 users (show)

See Also:

Patch (3.48 KB, patch)
2019-06-18 17:07 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff
Patch (8.08 KB, patch)
2019-06-19 15:16 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Rollin 2019-06-18 17:02:46 PDT
The tools DumpRenderTree, WebKitTestRunner, LayoutTestHelper, and TestNetscapePlugin get created in WebKit.framework/Versions/A/Resources on non-mac builds. This is incorrect, as those bundles are shallow bundles that don't use the Versions hierarchy. Instead, store these files directly in WebKit.framework. (Note that getting rid of just the "Versions/A" path components and putting the files in WebKit.framework/Resources doesn't work as `codesign` treats this as an invalid layout.)
Comment 1 Radar WebKit Bug Importer 2019-06-18 17:03:01 PDT
Comment 2 Keith Rollin 2019-06-18 17:07:32 PDT
Created attachment 372408 [details]
Comment 3 Andy Estes 2019-06-18 17:30:36 PDT
Comment on attachment 372408 [details]

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

> Tools/DumpRenderTree/mac/Configurations/Base.xcconfig:116
> +WEBKIT_FRAMEWORK_RESOURCES_PATH[sdk=macosx*] = WebKit.framework/Versions/A/Resources;

I see some other places in WebKit that use the value of $(SHALLOW_BUNDLE) rather than checking for specific SDKs. Can we do that here too?
Comment 4 Keith Rollin 2019-06-18 17:37:05 PDT
Not really. I tried that. The value of $(SHALLOW_BUNDLE) bundle that we'd want would be that used by the WebKit.framework build. That's not available when building WebKitTestRunner, et al. And, it turns out $(SHALLOW_BUNDLE) can be set to NO for some of these projects, which are strict executables and not bundles themselves.
Comment 5 Andy Estes 2019-06-18 17:57:19 PDT
Comment on attachment 372408 [details]

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

> Tools/DumpRenderTree/DumpRenderTree.xcodeproj/project.pbxproj:-1389
> -				WEBKIT_FRAMEWORK_RESOURCES_PATH = WebKit.framework/Versions/A/Resources;

It'd be nice to also move INSTALL_PATH from the pbxproj to an .xcconfig file. WEBKIT_FRAMEWORK_RESOURCES_PATH's effect is a little mysterious otherwise (at least it was to me while reviewing).
Comment 6 Alexey Proskuryakov 2019-06-19 00:59:39 PDT
IIRC the location was chosen for Mac as one that didn't make code signing get unhappy about unexpected or missing binaries. Not sure if this is still relevant as we tend to re-sign a lot more eagerly, but anyway - how did you test this?
Comment 7 Keith Rollin 2019-06-19 11:57:49 PDT
> how did you test this?

I ran it through spade to create a temporary-install. I modified spade to allow me to specify local roots. See Safari pull request 7255. This is how I found out about codesign complaining about my trying to use a Resources directory.
Comment 8 Keith Rollin 2019-06-19 15:16:16 PDT
Created attachment 372495 [details]
Comment 9 WebKit Commit Bot 2019-06-19 17:14:20 PDT
Comment on attachment 372495 [details]

Clearing flags on attachment: 372495

Committed r246617: <https://trac.webkit.org/changeset/246617>
Comment 10 WebKit Commit Bot 2019-06-19 17:14:22 PDT
All reviewed patches have been landed.  Closing bug.