Bug 202419 - REGRESSION (r250375): [old EWS] JSC EWS is always marking Patches as success
Summary: REGRESSION (r250375): [old EWS] JSC EWS is always marking Patches as success
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-01 12:04 PDT by Caio Lima
Modified: 2019-10-10 09:41 PDT (History)
8 users (show)

See Also:


Attachments
Patch (2.75 KB, patch)
2019-10-08 14:08 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch for landing (2.75 KB, patch)
2019-10-08 16:20 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Caio Lima 2019-10-01 12:04:42 PDT
The patch #379917 in https://bugs.webkit.org/show_bug.cgi?id=202140 contains a build error, however jsc, jsc-i386 and jsc-mips are being marked as success.
Comment 1 Aakash Jain 2019-10-03 10:53:13 PDT

*** This bug has been marked as a duplicate of bug 199753 ***
Comment 2 Aakash Jain 2019-10-03 12:20:41 PDT
This is actually not a duplicate of bug 199753. Bug 199753 is about the bubble color being wrong (when the test fails). This bug is about the build/test passing when they should have failed.
Comment 3 Aakash Jain 2019-10-03 12:22:27 PDT
(In reply to Caio Lima from comment #0)
> The patch #379917 in https://bugs.webkit.org/show_bug.cgi?id=202140 contains a build error, however jsc, jsc-i386 and jsc-mips are being marked as success.
Just to add more info https://webkit-queues.webkit.org/patch/379917/jsc-armv7-ews says "Built patch", while it should have failed.
Comment 4 Alexey Proskuryakov 2019-10-03 15:51:25 PDT
Does EWS actually build the EWS tool? Not sure if there is a bug here.
Comment 5 Caio Lima 2019-10-03 16:26:09 PDT
(In reply to Alexey Proskuryakov from comment #4)
> Does EWS actually build the EWS tool? Not sure if there is a bug here.

What do you mean by EWS tool?

AFICT, If I uploaded a patch that doesn't build into any JSC port 1 week ago, those bubble would be red and report "Patch does not build". However they are being marked as "Passed" and looking into the log, I can see that they successfully build and run tests. It is also the case when we have test failures. I noticed that this is happening since last week. I'm not sure if this is the expected output from EWS bots.
Comment 6 Alexey Proskuryakov 2019-10-03 16:43:39 PDT
*the jsc tool

Well, it is kind of a mystery indeed. I'm not sure about other EWSes, but the "jsc" one that runs tests seems like it should have failed indeed.

The command that wrongly succeeds is:

webkit-patch --status-host=webkit-queues.webkit.org --bot-id=ews127 build --no-clean --no-update --build-style=release --group=jsc --port=mac
Comment 7 Alexey Proskuryakov 2019-10-04 12:55:01 PDT
I can reproduce locally. "webkit-patch build" exits with status 0, so it's a problem in or below webkit-patch.
Comment 8 Alexey Proskuryakov 2019-10-08 12:53:09 PDT
Well, run_and_throw_if_fail in executive.py has this line:

            exit_code = self._run_command_with_teed_output(args, child_stdout, **kwargs)

And the _run_command_with_teed_output function never returns a value. Looks like a regression from r250375.
Comment 9 Jonathan Bedard 2019-10-08 13:28:19 PDT
(In reply to Alexey Proskuryakov from comment #8)
> Well, run_and_throw_if_fail in executive.py has this line:
> 
>             exit_code = self._run_command_with_teed_output(args,
> child_stdout, **kwargs)
> 
> And the _run_command_with_teed_output function never returns a value. Looks
> like a regression from r250375.

Totally is a regression, I have the fix.
Comment 10 Jonathan Bedard 2019-10-08 14:08:55 PDT
Created attachment 380463 [details]
Patch
Comment 11 Aakash Jain 2019-10-08 15:53:13 PDT
rs=me
Comment 12 Jonathan Bedard 2019-10-08 16:20:53 PDT
Created attachment 380476 [details]
Patch for landing
Comment 13 WebKit Commit Bot 2019-10-08 16:38:21 PDT
Comment on attachment 380476 [details]
Patch for landing

Clearing flags on attachment: 380476

Committed r250877: <https://trac.webkit.org/changeset/250877>
Comment 14 WebKit Commit Bot 2019-10-08 16:38:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Radar WebKit Bug Importer 2019-10-08 16:39:16 PDT
<rdar://problem/56098044>
Comment 16 Jonathan Bedard 2019-10-08 16:40:42 PDT
Alexey has another complaint about the weirdness of the log, looking into that as well.
Comment 17 Jonathan Bedard 2019-10-10 09:41:22 PDT
(In reply to Jonathan Bedard from comment #16)
> Alexey has another complaint about the weirdness of the log, looking into
> that as well.

After the fix in r250877, the log looks fine, the end is:

The following build commands failed:
	CompileC /Volumes/Shared/OpenSource/WebKitBuild/JavaScriptCore.build/Release/jsc.build/Objects-normal/x86_64/jsc.o /Volumes/Shared/OpenSource/Source/JavaScriptCore/jsc.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)
/Volumes/Shared/OpenSource/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal
    export OBJROOT=/Volumes/Shared/OpenSource/WebKitBuild
    export ONLY_ACTIVE_ARCH=YES
    export OS=MACOS
    export OSAC=/usr/bin/osacompile
    export OTHER_CFLAGS=" --system-header-prefix=unicode/"
    export OTHER_CPLUSPLUSFLAGS=" -isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/System.framework/PrivateHeaders --system-header-prefix=unicode/"
    export OTHER_LDFLAGS=" -Wl,-unexported_symbol,__ZTISt9bad_alloc -Wl,-unexported_symbol,__ZTISt9exception -Wl,-unexported_symbol,__ZTSSt9bad_alloc -Wl,-unexported_symbol,__ZTSSt9exception -Wl,-unexported_symbol,__ZdlPvS_ -Wl,-unexported_symbol,__ZnwmPv -Wl,-unexported_symbol,__ZNKSt3__18functionIFvvEEclEv -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEEC1EOS2_ -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEEC2EOS2_ -Wl,-unexported_symbol,__ZNKSt3__18functionIFvRN3JSC17BytecodeGeneratorEPNS1_10RegisterIDEEEclES3_S5_ -Wl,-unexported_symbol,__ZNSt3__18functionIFvRN3JSC17BytecodeGeneratorEPNS1_10RegisterIDEEED1Ev -Wl,-unexported_symbol,__ZNSt3__18functionIFvRN3JSC17BytecodeGeneratorEPNS1_10RegisterIDEEED2Ev -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEED1Ev -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEED2Ev -Wl,-unexported_symbol,__ZTVNSt3__117bad_function_callE -force_load "/Volumes/Shared/OpenSource/WebKitBuild/Release/DerivedSources/JavaScriptCore/libWTF.a" -framework CoreServices"
    export OTHER_LDFLAGS_BASE="-Wl,-unexported_symbol,__ZTISt9bad_alloc -Wl,-unexported_symbol,__ZTISt9exception -Wl,-unexported_symbol,__ZTSSt9bad_alloc -Wl,-unexported_symbol,__ZTSSt9exception -Wl,-unexported_symbol,__ZdlPvS_ -Wl,-unexported_symbol,__ZnwmPv -Wl,-unexported_symbol,__ZNKSt3__18functionIFvvEEclEv -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEEC1EOS2_ -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEEC2EOS2_ -Wl,-unexported_symbol,__ZNKSt3__18functionIFvRN3JSC17BytecodeGeneratorEPNS1_10RegisterIDEEEclES3_S5_ -Wl,-unexported_symbol,__ZNSt3__18functionIFvRN3JSC17BytecodeGeneratorEPNS1_10RegisterIDEEED1Ev -Wl,-unexported_symbol,__ZNSt3__18functionIFvRN3JSC17BytecodeGeneratorEPNS1_10RegisterIDEEED2Ev -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEED1Ev -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEED2Ev -Wl,-unexported_symbol,__ZTVNSt3__117bad_function_callE -force_load "/Volumes/Shared/OpenSource/WebKitBuild/Release/DerivedSources/JavaScriptCore/libWTF.a""
    export OTHER_LDFLAGS_HIDE_SYMBOLS="-Wl,-unexported_symbol,__ZTISt9bad_alloc -Wl,-unexported_symbol,__ZTISt9exception -Wl,-unexported_symbol,__ZTSSt9bad_alloc -Wl,-unexported_symbol,__ZTSSt9exception -Wl,-unexported_symbol,__ZdlPvS_ -Wl,-unexported_symbol,__ZnwmPv -Wl,-unexported_symbol,__ZNKSt3__18functionIFvvEEclEv -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEEC1EOS2_ -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEEC2EOS2_ -Wl,-unexported_symbol,__ZNKSt3__18functionIFvRN3JSC17BytecodeGeneratorEPNS1_10RegisterIDEEEclES3_S5_ -Wl,-unexported_symbol,__ZNSt3__18functionIFvRN3JSC17BytecodeGeneratorEPNS1_10RegisterIDEEED1Ev -Wl,-unexported_symbol,__ZNSt3__18functionIFvRN3JSC17BytecodeGeneratorEPNS1_10RegisterIDEEED2Ev -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEED1Ev -Wl,-unexported_symbol,__ZNSt3__18functionIFvvEED2Ev -Wl,-unexported_symbol,__ZTVNSt3__117bad_function_callE"
    export PACKAGE_TYPE=com.apple.package-type.wrapper.framework

Failed to run "['Tools/Scripts/build-jsc', '--release']" exit_code: 65

Pretty sure the logging weirdness Alexey observed was because we weren't early exiting on non-zero exit codes.