RESOLVED FIXED203970
Move jsc from Resources to Helpers
https://bugs.webkit.org/show_bug.cgi?id=203970
Summary Move jsc from Resources to Helpers
Keith Rollin
Reported 2019-11-07 11:14:18 PST
'jsc' is a supporting application or tool, not a resource. As such, it should go into Helpers, per <https://developer.apple.com/library/archive/technotes/tn2206/_index.html#//apple_ref/doc/uid/DTS40007919-CH1-TNTAG201>
Attachments
Patch (6.20 KB, patch)
2019-11-07 11:33 PST, Keith Rollin
no flags
Mark had r+'d the previous patch, but I've made enough changes in the update that someone should probably look them over again. (11.21 KB, patch)
2019-11-13 10:10 PST, Keith Rollin
no flags
Fixed problem with running jsc stress tests. (11.57 KB, patch)
2019-11-14 16:09 PST, Keith Rollin
no flags
Keith Rollin
Comment 1 2019-11-07 11:14:36 PST
Keith Rollin
Comment 2 2019-11-07 11:33:51 PST
Mark Lam
Comment 3 2019-11-07 11:34:49 PST
Comment on attachment 383063 [details] Patch r=me if EWS bots are happy.
Keith Miller
Comment 4 2019-11-07 12:23:16 PST
Do we know that this won't break a bunch of existing scripts and such? I know I'll have to update my $PATH.
Alexey Proskuryakov
Comment 5 2019-11-07 12:47:50 PST
I think that we also need to update the symlink in /usr/local/bin
Keith Rollin
Comment 6 2019-11-07 13:12:13 PST
(In reply to Alexey Proskuryakov from comment #5) > I think that we also need to update the symlink in /usr/local/bin In what way? The link will already be created to point to the new location for jsc (the script that creates the link uses INSTALL_PATH, which was updated to point to Helpers). Is there some other updating that needs to be done?
Keith Rollin
Comment 7 2019-11-07 13:13:49 PST
(In reply to Keith Miller from comment #4) > Do we know that this won't break a bunch of existing scripts and such? I > know I'll have to update my $PATH. Anyone know what scripts might be affected and that I should look into?
Keith Rollin
Comment 8 2019-11-07 13:14:17 PST
(In reply to Keith Rollin from comment #7) > (In reply to Keith Miller from comment #4) > > Do we know that this won't break a bunch of existing scripts and such? I > > know I'll have to update my $PATH. > > Anyone know what scripts might be affected and that I should look into? Looks like run-jsc-stress-tests, for one.
Keith Rollin
Comment 9 2019-11-07 13:17:33 PST
(In reply to Keith Rollin from comment #8) > (In reply to Keith Rollin from comment #7) > > (In reply to Keith Miller from comment #4) > > > Do we know that this won't break a bunch of existing scripts and such? I > > > know I'll have to update my $PATH. > > > > Anyone know what scripts might be affected and that I should look into? > > Looks like run-jsc-stress-tests, for one. And sunspider-compare-results. Which highlights for me that we probably need a link from the framework root directory to the right Versions directory on macOS.
Keith Rollin
Comment 10 2019-11-07 13:18:57 PST
(In reply to Keith Rollin from comment #9) > (In reply to Keith Rollin from comment #8) > > (In reply to Keith Rollin from comment #7) > > > (In reply to Keith Miller from comment #4) > > > > Do we know that this won't break a bunch of existing scripts and such? I > > > > know I'll have to update my $PATH. > > > > > > Anyone know what scripts might be affected and that I should look into? > > > > Looks like run-jsc-stress-tests, for one. > > And sunspider-compare-results. Which highlights for me that we probably need > a link from the framework root directory to the right Versions directory on > macOS. I'm going with this set, unless anyone else knows others that should be added: % rg "Resource.*jsc" run-jsc-stress-tests 309: jscArg = Pathname.new(output).join("JavaScriptCore.framework", "Resources", "jsc") if !File.file?(jscArg) 328: $jscPath = $bundle + ".vm" + "JavaScriptCore.framework" + "Resources" + "jsc" 538: pathToBundleResourceFromBenchmarkDirectory($jscPath) 1743: $jscPath = destinationFrameworkPath + "Resources" + "jsc" webkitdirs.pm 479: return "$productDir/JavaScriptCore.framework/Resources/$jscName"; sunspider-compare-results 101: my $path = "/System/Library/Frameworks/JavaScriptCore.framework/Resources/jsc"; run-layout-jsc 28:jscCmd="/System/Library/Frameworks/JavaScriptCore.framework/Resources/jsc" test262/Runner.pm 607: $jsc = $jscDir . '/JavaScriptCore.framework/Resources/jsc' if (! -e $jsc);
Alexey Proskuryakov
Comment 11 2019-11-07 13:35:23 PST
> The link will already be created to point to the new location for jsc Then it should be all good for the symlink.
Keith Rollin
Comment 12 2019-11-11 12:25:32 PST
Moving "jsc" to Helpers also has the effect of moving other tools to Helpers: % ll ./macos-production-default-default-default/webkit-build/SafariRoot/System/Library/Frameworks/JavaScriptCore.framework/Versions/A/Helpers total 9488 -rwxr-xr-x 1 keith wheel 50K Nov 8 15:17 JSCLLIntOffsetsExtractor* -rwxr-xr-x 1 keith wheel 139K Nov 8 15:17 JSCLLIntSettingsExtractor* -rwxr-xr-x 1 keith wheel 67K Nov 8 15:18 dynbench* -rwxr-xr-x 1 keith wheel 263K Nov 8 15:17 jsc* -rwxr-xr-x 1 keith wheel 24K Nov 8 15:18 minidom* -rwxr-xr-x 1 keith wheel 34K Nov 8 15:18 testRegExp* -rwxr-xr-x 1 keith wheel 492K Nov 8 15:18 testair* -rwxr-xr-x 1 keith wheel 358K Nov 8 15:18 testapi* -rw-r--r-- 1 keith wheel 11K Nov 8 15:18 testapi.js -rwxr-xr-x 1 keith wheel 3.0M Nov 8 15:18 testb3* -rwxr-xr-x 1 keith wheel 18K Nov 8 15:18 testdfg* -rwxr-xr-x 1 keith wheel 155K Nov 8 15:18 testmasm* -rwxr-xr-x 1 keith wheel 19K Nov 8 15:18 testmem* I think it is what we want. But I'll check all our scripts to see if this move will break any of them.
Keith Rollin
Comment 13 2019-11-13 10:10:58 PST
Created attachment 383465 [details] Mark had r+'d the previous patch, but I've made enough changes in the update that someone should probably look them over again.
Aakash Jain
Comment 14 2019-11-13 10:39:23 PST
jsc EWS seems to be failing with 69441 failures. Please wait for ews to complete https://webkit-queues.webkit.org/patch/383465/jsc-ews
Keith Rollin
Comment 15 2019-11-13 10:49:18 PST
Failed to run "['Tools/Scripts/run-javascriptcore-tests', '--no-fail-fast', '--release', u'--json-output=/Volumes/Data/EWS/WebKit/WebKitBuild/Release/jsc_test_results.json']" exit_code: 1 Maybe run-javascriptcore-tests can't find one of the tools I moved. I'll check.
Keith Rollin
Comment 16 2019-11-13 10:57:16 PST
I *am* seeing lots of errors like: stress/async-await-mozilla.js.ftl-no-cjit-validate-sampling-profiler: test_script_1109: line 2: ../../.vm/JavaScriptCore.framework/Helpers/jsc: No such file or directory Which initially seems odd, since "Helpers" should be the right directory. I'll check it out.
Keith Rollin
Comment 17 2019-11-13 10:58:11 PST
I don't know what that ".vm" is about. Is that right?
Keith Rollin
Comment 18 2019-11-13 11:00:04 PST
Regardless of that ".vm", the WebKitBuild/Release/JavaScriptCore.framework directory does not have a Helpers directory when building for jsc testing. I have no idea where the "jsc" tool ended up. Or the other tools. They aren't in Resources, either.
Keith Rollin
Comment 19 2019-11-13 11:03:27 PST
During a Release build, should any of those JSC tools end up in the .framework bundle at all? They're in the root Release directory, which I think is where I'd expect them in a Release build.
Keith Rollin
Comment 20 2019-11-13 16:54:13 PST
OK, I see the problem. Trying to copy 'jsc' to 'Resources' works because Xcode pre-creates that and other standard directories. However, trying to copy 'jsc' to 'Helpers' doesn't work because that directory is not pre-created. Since the destination directory didn't exist, the copy command failed. Fixed this by adding a line to ensure that the destination directory exists.
Keith Rollin
Comment 21 2019-11-14 16:09:40 PST
Created attachment 383581 [details] Fixed problem with running jsc stress tests.
Keith Rollin
Comment 22 2019-11-15 10:53:35 PST
jsc EWS is passing this time, so the patch looks good.
Keith Rollin
Comment 23 2019-11-18 12:42:01 PST
This patch is still available for review.
Keith Miller
Comment 24 2019-11-18 13:09:12 PST
Comment on attachment 383581 [details] Fixed problem with running jsc stress tests. Seems fine to me. Although, I have a feeling you're going to have a long tail of people filing bugs that their scripts don't work anymore. :(
WebKit Commit Bot
Comment 25 2019-11-18 13:48:06 PST
Comment on attachment 383581 [details] Fixed problem with running jsc stress tests. Clearing flags on attachment: 383581 Committed r252564: <https://trac.webkit.org/changeset/252564>
WebKit Commit Bot
Comment 26 2019-11-18 13:48:09 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.