Bug 172103 – [JSC] Build testapi in non Apple ports

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Yusuke Suzuki]

It is super useful if we can test JSC with testapi.
Especially, I'm focusing on JSCOnly port.

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Created attachment 310119 [details]
Patch

[Yusuke Suzuki]

Created attachment 310120 [details]
Patch

[Yusuke Suzuki]

Created attachment 310121 [details]
Patch

[Build Bot]

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.

[Yusuke Suzuki]

Created attachment 310123 [details]
Patch

[Build Bot]

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.

[Alex Christensen]

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.

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Committed r216914: <http://trac.webkit.org/changeset/216914>

[Csaba Osztrogonác]

(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

[Csaba Osztrogonác]

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.

[Csaba Osztrogonác]

FYI, I disabled this test on the ARM JSCOnly bots.

[Yusuke Suzuki]

(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.

[Yusuke Suzuki]

Committed r216924: <http://trac.webkit.org/changeset/216924>

[Csaba Osztrogonác]

(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

[Mark Lam]

(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?

[Csaba Osztrogonác]

(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.

[Csaba Osztrogonác]

(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.

[Mark Lam]

(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>.