WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
203970
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
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
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Keith Rollin
Comment 1
2019-11-07 11:14:36 PST
<
rdar://problem/55917748
>
Keith Rollin
Comment 2
2019-11-07 11:33:51 PST
Created
attachment 383063
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug