Bug 172103 - [JSC] Build testapi in non Apple ports
Summary: [JSC] Build testapi in non Apple ports
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-14 23:38 PDT by Yusuke Suzuki
Modified: 2017-05-23 09:52 PDT (History)
10 users (show)

See Also:


Attachments
Patch (26.58 KB, patch)
2017-05-15 01:53 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (27.25 KB, patch)
2017-05-15 01:55 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (27.35 KB, patch)
2017-05-15 01:58 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (27.67 KB, patch)
2017-05-15 02:08 PDT, Yusuke Suzuki
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2017-05-14 23:38:52 PDT
It is super useful if we can test JSC with testapi.
Especially, I'm focusing on JSCOnly port.
Comment 1 Yusuke Suzuki 2017-05-15 00:13:31 PDT
Executing testapi in different platforms is actually useful.
I've run testapi on Linux, and maybe I've found a bug.

In testMarkingConstraintAndHeapFinalizers adds a constraint.
And it looks into weak refs.
But in L1205, we start releasing weak refs.
So, after that, if the markingConstraint runs, it find released weak refs.
Comment 2 Yusuke Suzuki 2017-05-15 01:53:33 PDT
Created attachment 310119 [details]
Patch
Comment 3 Yusuke Suzuki 2017-05-15 01:55:55 PDT
Created attachment 310120 [details]
Patch
Comment 4 Yusuke Suzuki 2017-05-15 01:58:30 PDT
Created attachment 310121 [details]
Patch
Comment 5 Build Bot 2017-05-15 01:59:32 PDT
Attachment 310121 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/API/tests/testapi.c:154:  Declaration has space between * and variable name in char* buffer  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/API/tests/testapi.c:1256:  Declaration has space between * and variable name in UniChar* buffer  [whitespace/declaration] [3]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Yusuke Suzuki 2017-05-15 02:08:50 PDT
Created attachment 310123 [details]
Patch
Comment 7 Build Bot 2017-05-15 02:11:36 PDT
Attachment 310123 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/API/tests/testapi.c:154:  Declaration has space between * and variable name in char* buffer  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/API/tests/testapi.c:1256:  Declaration has space between * and variable name in UniChar* buffer  [whitespace/declaration] [3]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Alex Christensen 2017-05-15 12:50:00 PDT
Comment on attachment 310123 [details]
Patch

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

This seems like a great idea.  Someone else will have to look at the testapi.c changes.

> Tools/Scripts/run-javascriptcore-tests:59
> -my $runTestAPI = isAppleCocoaWebKit() || isAppleWinWebKit() || isWinCairo();
> +my $runTestAPI = isAppleCocoaWebKit() || isAppleWinWebKit() || isWinCairo() || isJSCOnly() || isGTK() || isWPE();

Let's just remove this.
Comment 9 Yusuke Suzuki 2017-05-16 00:24:23 PDT
Comment on attachment 310123 [details]
Patch

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

Thanks!

>> Tools/Scripts/run-javascriptcore-tests:59
>> +my $runTestAPI = isAppleCocoaWebKit() || isAppleWinWebKit() || isWinCairo() || isJSCOnly() || isGTK() || isWPE();
> 
> Let's just remove this.

OK, dropped.
Comment 10 Yusuke Suzuki 2017-05-16 00:27:55 PDT
Committed r216914: <http://trac.webkit.org/changeset/216914>
Comment 11 Csaba Osztrogonác 2017-05-16 01:03:48 PDT
(In reply to Yusuke Suzuki from comment #10)
> Committed r216914: <http://trac.webkit.org/changeset/216914>

It broke the Apple Mac cmake build:
https://build.webkit.org/builders/Apple%20El%20Capitan%20CMake%20Debug%20%28Build%29/builds/1662
Comment 12 Csaba Osztrogonác 2017-05-16 01:10:31 PDT
https://trac.webkit.org/changeset/216914/webkit/trunk/Tools/Scripts/run-javascriptcore-tests

Running this test on JSCOnly bots won't work ever on ARM JSCOnly bots,
since they execute run-javascriptcore-tests on X86_64 with --remote-config-file
option, which means that run-jsc-stress-tests executes tests on a remote machine.

See this build:
https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/588

Fortunately I found that we can disable this test with
RUN_JAVASCRIPTCORE_TESTS_TESTAPI=false environment variable.
I'll exactly do this immediately.
Comment 13 Csaba Osztrogonác 2017-05-16 01:16:07 PDT
FYI, I disabled this test on the ARM JSCOnly bots.
Comment 14 Yusuke Suzuki 2017-05-16 02:07:37 PDT
(In reply to Csaba Osztrogonác from comment #11)
> (In reply to Yusuke Suzuki from comment #10)
> > Committed r216914: <http://trac.webkit.org/changeset/216914>
> 
> It broke the Apple Mac cmake build:
> https://build.webkit.org/builders/
> Apple%20El%20Capitan%20CMake%20Debug%20%28Build%29/builds/1662

I'll attempt to fix it by adding files for PlatformMac.
Comment 15 Yusuke Suzuki 2017-05-16 02:41:01 PDT
Committed r216924: <http://trac.webkit.org/changeset/216924>
Comment 16 Csaba Osztrogonác 2017-05-17 01:44:50 PDT
(In reply to Yusuke Suzuki from comment #15)
> Committed r216924: <http://trac.webkit.org/changeset/216924>

still broken ...

Undefined symbols for architecture x86_64:
  "_testObjectiveCAPI", referenced from:
      _main in testapi.c.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [bin/testapi] Error 1
Comment 17 Mark Lam 2017-05-17 09:50:30 PDT
(In reply to Csaba Osztrogonác from comment #16)
> (In reply to Yusuke Suzuki from comment #15)
> > Committed r216924: <http://trac.webkit.org/changeset/216924>
> 
> still broken ...
> 
> Undefined symbols for architecture x86_64:
>   "_testObjectiveCAPI", referenced from:
>       _main in testapi.c.o
> ld: symbol(s) not found for architecture x86_64
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
> make[2]: *** [bin/testapi] Error 1

This is weird because the call to testObjectiveCAPI is guarded by #if JSC_OBJC_API_ENABLED.  Why is JSC_OBJC_API_ENABLED set for linux?
Comment 18 Csaba Osztrogonác 2017-05-17 23:41:06 PDT
(In reply to Mark Lam from comment #17)
> (In reply to Csaba Osztrogonác from comment #16)
> > (In reply to Yusuke Suzuki from comment #15)
> > > Committed r216924: <http://trac.webkit.org/changeset/216924>
> > 
> > still broken ...
> > 
> > Undefined symbols for architecture x86_64:
> >   "_testObjectiveCAPI", referenced from:
> >       _main in testapi.c.o
> > ld: symbol(s) not found for architecture x86_64
> > clang: error: linker command failed with exit code 1 (use -v to see
> > invocation)
> > make[2]: *** [bin/testapi] Error 1
> 
> This is weird because the call to testObjectiveCAPI is guarded by #if
> JSC_OBJC_API_ENABLED.  Why is JSC_OBJC_API_ENABLED set for linux?

It's not Linux, but the official Apple Mac cmake build.
Comment 19 Csaba Osztrogonác 2017-05-22 03:42:24 PDT
(In reply to Yusuke Suzuki from comment #15)
> Committed r216924: <http://trac.webkit.org/changeset/216924>

FYI: The Apple Mac cmake build is still broken after this change.
Comment 20 Mark Lam 2017-05-23 09:52:07 PDT
(In reply to Csaba Osztrogonác from comment #19)
> (In reply to Yusuke Suzuki from comment #15)
> > Committed r216924: <http://trac.webkit.org/changeset/216924>
> 
> FYI: The Apple Mac cmake build is still broken after this change.

According to Yusuke, this should have been fixed in r217182: <http://trac.webkit.org/r217182>.