Bug 203970 - Move jsc from Resources to Helpers
Summary: Move jsc from Resources to Helpers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Rollin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-11-07 11:14 PST by Keith Rollin
Modified: 2019-11-18 13:48 PST (History)
14 users (show)

See Also:


Attachments
Patch (6.20 KB, patch)
2019-11-07 11:33 PST, Keith Rollin
no flags Details | Formatted Diff | Diff
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 Details | Formatted Diff | Diff
Fixed problem with running jsc stress tests. (11.57 KB, patch)
2019-11-14 16:09 PST, 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-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>
Comment 1 Keith Rollin 2019-11-07 11:14:36 PST
<rdar://problem/55917748>
Comment 2 Keith Rollin 2019-11-07 11:33:51 PST
Created attachment 383063 [details]
Patch
Comment 3 Mark Lam 2019-11-07 11:34:49 PST
Comment on attachment 383063 [details]
Patch

r=me if EWS bots are happy.
Comment 4 Keith Miller 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.
Comment 5 Alexey Proskuryakov 2019-11-07 12:47:50 PST
I think that we also need to update the symlink in /usr/local/bin
Comment 6 Keith Rollin 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?
Comment 7 Keith Rollin 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?
Comment 8 Keith Rollin 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.
Comment 9 Keith Rollin 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.
Comment 10 Keith Rollin 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);
Comment 11 Alexey Proskuryakov 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.
Comment 12 Keith Rollin 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.
Comment 13 Keith Rollin 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.
Comment 14 Aakash Jain 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
Comment 15 Keith Rollin 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.
Comment 16 Keith Rollin 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.
Comment 17 Keith Rollin 2019-11-13 10:58:11 PST
I don't know what that ".vm" is about. Is that right?
Comment 18 Keith Rollin 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.
Comment 19 Keith Rollin 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.
Comment 20 Keith Rollin 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.
Comment 21 Keith Rollin 2019-11-14 16:09:40 PST
Created attachment 383581 [details]
Fixed problem with running jsc stress tests.
Comment 22 Keith Rollin 2019-11-15 10:53:35 PST
jsc EWS is passing this time, so the patch looks good.
Comment 23 Keith Rollin 2019-11-18 12:42:01 PST
This patch is still available for review.
Comment 24 Keith Miller 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. :(
Comment 25 WebKit Commit Bot 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>
Comment 26 WebKit Commit Bot 2019-11-18 13:48:09 PST
All reviewed patches have been landed.  Closing bug.