Bug 230218

Summary: [JSC] run-javascriptcore-tests script fails on linux
Product: WebKit Reporter: Phillip Mates <pmates>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Trivial CC: angelos, ews-watchlist, jbedard, webkit-bug-importer
Priority: P4 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
fix for the bug
none
Patch none

Description Phillip Mates 2021-09-13 07:17:32 PDT
Created attachment 438038 [details]
fix for the bug

When I run 
```
$ Tools/Scripts/run-javascriptcore-tests --jsc-only --debug
```

I see one warning:
```
Can't exec "xcodebuild": No such file or directory at /home/mates/igalia/WebKit/Tools/Scripts/webkitdirs.pm line 634.
```

and one error:
```
Unrecognized option `--64-bit'
```

adding `--no-build` allows me to run the tests, so it seems that `run-javascriptcore-tests`, has issues when dispatching to the `build-jsc` command.


Attached is a fix for this issue though keep in mind that I'm very new to the build tooling.
I also removed the `determineIsWin64FromArchitecture` function, which is unused as far as I can tell from grepping the codebase and right below some code other code I changed in the patch
Comment 1 Jonathan Bedard 2021-09-13 07:31:04 PDT
Comment on attachment 438038 [details]
fix for the bug

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

> Tools/Scripts/webkitdirs.pm:-1483
> -    $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || ((isAnyWindows() || isJSCOnly()) && !shouldBuild32Bit());

I notice we're stripping the isJSCOnly() call, is that required?
Comment 2 Phillip Mates 2021-09-13 07:38:36 PDT
(In reply to Jonathan Bedard from comment #1)
> Comment on attachment 438038 [details]
> fix for the bug
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438038&action=review
> 
> > Tools/Scripts/webkitdirs.pm:-1483
> > -    $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || ((isAnyWindows() || isJSCOnly()) && !shouldBuild32Bit());
> 
> I notice we're stripping the isJSCOnly() call, is that required?

I think so, given that we want the whole condition to be `false` to exclude the problematic `--64-bit` flag. In this context `!shouldBuild32Bit()` is `true` and `isAnyWindows()` is `false` so we'd need `isJSCOnly()` to be `false` to exclude the `--64-bit` flag, but it is `true`; hence I just dropped it. Dropping it might be the wrong way to resolve this, wasn't totally sure.
Comment 3 Radar WebKit Bug Importer 2021-09-20 07:18:16 PDT
<rdar://problem/83305636>
Comment 4 Phillip Mates 2021-09-21 01:49:04 PDT
Hi jbedard, could you also set cq+ when you have the chance? thanks!
Comment 5 EWS 2021-09-21 01:55:44 PDT
Unable to find any modified ChangeLog in Attachment 438038 [details]
Comment 6 Phillip Mates 2021-09-21 02:11:10 PDT
Comment on attachment 438038 [details]
fix for the bug

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 675f6cdf35a6..215167b3fb5b 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,24 @@
+2021-09-21  Phillip Mates  <pmates@igalia.com>
+
+        Fix JSC test runner warnings and errors on linux
+        https://bugs.webkit.org/show_bug.cgi?id=230218
+
+        Reviewed by Jonathan Bedard.
+
+        Fixed the following warning that arises in many scripts when they are
+        run on Linux:
+        `Can't exec "xcodebuild": No such file or directory at
+        /home/mates/igalia/WebKit/Tools/Scripts/webkitdirs.pm line 634.`
+
+        Fixed `run-javascriptcore-tests` on Linux, which was failing to invoke
+        builds before running the test suite with the error:
+        `Unrecognized option `--64-bit'`
+
+        * Scripts/webkitdirs.pm:
+        (XcodeOptions):
+        (determineIsWin64):
+        (determineIsWin64FromArchitecture): Deleted.
+
 2021-09-20  Chris Dumez  <cdumez@apple.com>
 
         Reduce use of makeRefPtr() and use RefPtr { } directly
diff --git a/Tools/Scripts/webkitdirs.pm b/Tools/Scripts/webkitdirs.pm
index a56fe7ecb944..eb61a3ec298b 100755
--- a/Tools/Scripts/webkitdirs.pm
+++ b/Tools/Scripts/webkitdirs.pm
@@ -1008,7 +1008,9 @@ sub XcodeOptions
     determineForceOptimizationLevel();
     determineCoverageIsEnabled();
     determineLTOMode();
-    determineXcodeSDK();
+    if (isAppleCocoaWebKit()) {
+      determineXcodeSDK();
+    }
 
     my @options;
     push @options, "-UseSanitizedBuildSystemEnvironment=YES";
@@ -1480,14 +1482,7 @@ sub isWin64()
 sub determineIsWin64()
 {
     return if defined($isWin64);
-    $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || ((isAnyWindows() || isJSCOnly()) && !shouldBuild32Bit());
-}
-
-sub determineIsWin64FromArchitecture($)
-{
-    my $arch = shift;
-    $isWin64 = ($arch eq "x86_64");
-    return $isWin64;
+    $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || (isAnyWindows() && !shouldBuild32Bit());
 }
 
 sub isCygwin()
Comment 7 EWS 2021-09-21 02:11:25 PDT
pmates@igalia.com does not have committer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/metadata/contributors.json.

Rejecting attachment 438038 [details] from commit queue.
Comment 8 Phillip Mates 2021-09-21 02:15:06 PDT
Created attachment 438796 [details]
Patch
Comment 9 Phillip Mates 2021-09-21 02:21:41 PDT
Comment on attachment 438796 [details]
Patch

sorry for the noise on this patch. I feel like a bull in a china shop when it comes to navigating this interface
Comment 10 EWS 2021-09-21 07:54:45 PDT
Committed r282815 (241946@main): <https://commits.webkit.org/241946@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438796 [details].