Bug 193138 – [ews-build] use svn-apply script to apply the patches

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Aakash Jain]

Default Buildbot git/svn methods to apply the patch (which uses either patch command or git apply command) can not reliably apply the WebKit patches. Especially in dealing with ChangeLog (e.g.: when there have been further commits on ToT between the time when  patch was generated by an engineer and patch was applied by EWS). 

svn-patch can much effectively handle such cases. Current OpenSource EWS uses svn-patch script to apply the patches. We should also use the same script for new Buildbot based EWS.

[Aakash Jain]

Created attachment 358316 [details]
Proposed patch

Sample run: http://ews-build.webkit-uat.org/#/builders/21/builds/907

[EWS Watchlist]

Attachment 358316 [details] did not pass style-queue:


ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:27:  No name 'worker' in module 'buildbot.steps'  [pylint/E0611] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:128:  [ApplyPatch.start] Instance of 'ApplyPatch' has no 'downloadFileContentToWorker' member  [pylint/E1101] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:129:  [ApplyPatch.start.<lambda>] Instance of 'ApplyPatch' has no 'downloadFileContentToWorker' member  [pylint/E1101] [5]
Total errors found: 3 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 358316 [details]
Proposed patch

Attachment 358316 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10628013

New failing tests:
http/wpt/css/css-animations/start-animation-001.html

[EWS Watchlist]

Created attachment 358319 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Lucas Forschler]

Comment on attachment 358316 [details]
Proposed patch

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

> Tools/BuildSlaveSupport/ews-build/steps.py:95
> +    name = 'clean-and-update-working-directory'

this seems like it is not related to this patch, but if it's just a drive by fix, then go ahead and leave it.

> Tools/BuildSlaveSupport/ews-build/steps.py:120
> +        return sourcestamp.patch[1]

it's unclear to me why we are indexing '1' here, can you put a comment?

[Aakash Jain]

Comment on attachment 358316 [details]
Proposed patch

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

>> Tools/BuildSlaveSupport/ews-build/steps.py:95
>> +    name = 'clean-and-update-working-directory'
> 
> this seems like it is not related to this patch, but if it's just a drive by fix, then go ahead and leave it.

Yeah, it's a drive by fix.

>> Tools/BuildSlaveSupport/ews-build/steps.py:120
>> +        return sourcestamp.patch[1]
> 
> it's unclear to me why we are indexing '1' here, can you put a comment?

It's based on how buildbot internally store the patch (inside sourcestamp). Referred from https://github.com/buildbot/buildbot/blob/master/master/buildbot/steps/source/base.py#L223

Note that Sourcestamp info is visible when you click on 'Debug' tab on any build.

[WebKit Commit Bot]

Comment on attachment 358316 [details]
Proposed patch

Clearing flags on attachment: 358316

Committed r239633: <https://trac.webkit.org/changeset/239633>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/47055515>