Bug 63651 – Update kill-old-processes logic

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Lucas Forschler]

We currently have a kill-old-processes script running on windows bots.  We should run this on mac bots as well.
A platform agnostic (windows/mac) kill-old-processes script has been checked in already.  Start using it.

[Lucas Forschler]

Created attachment 99144 [details]
Patch to move to new kill-old-processes script

[Lucas Forschler]

Created attachment 99160 [details]
Patch to move to new kill-old-processes script

fixed patch.

[Adam Roben (:aroben)]

Comment on attachment 99160 [details]
Patch to move to new kill-old-processes script

Will KillOldProcesses do the right thing on GTK/Qt/EFL/etc.?

[Lucas Forschler]

It will exit out on any non Cygwin/Darwin platform.  Currently, this script is only run on platforms "win", "chromium-win".  This patch removes that check, so it will now run on darwin as well.

It will raise an exception on non Cygwin/Darwin platforms.  Rather than raising an exception, should we just exit quietly and do nothing?  If someone had specifics for the GTK/Qt/EFL targets, it would be good to update them.

[Adam Roben (:aroben)]

(In reply to comment #4)
> It will raise an exception on non Cygwin/Darwin platforms.  Rather than raising an exception, should we just exit quietly and do nothing?

Yes.

[Lucas Forschler]

Created attachment 99535 [details]
Updated patch

Updated patch to exit clean on unsupported platforms rather than raise an exception.

[Adam Roben (:aroben)]

Comment on attachment 99535 [details]
Updated patch

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

> Tools/BuildSlaveSupport/kill-old-processes:80
> +        sys.exit

Don't you need some parentheses here? sys.exit()

[Lucas Forschler]

Created attachment 99710 [details]
fixed patch.

fixed.

[WebKit Review Bot]

Comment on attachment 99710 [details]
fixed patch.

Rejecting attachment 99710 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 1

Last 500 characters of output:
75f1b5384c06c3d0e73f6948134a89e1359e122e
r90396 = 00d69d39ebebfb88c0ed81355a9508de38c8de30
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/8985403

[Lucas Forschler]

Created attachment 99722 [details]
retry.

previous patch failed to apply, trying again.

[WebKit Review Bot]

Comment on attachment 99722 [details]
retry.

Rejecting attachment 99722 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 1

Last 500 characters of output:
d057bd753106a3b2d9055448e662c77866336023
r90399 = 740b27ceddf9032c6928be43100fd7b2314e7228
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/8983684

[Adam Barth]

Comment on attachment 99722 [details]
retry.

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

> Tools/ChangeLog:3
> +        Need a short description and bug URL (OOPS!)

You need to remove this line in order for the patch to land.  Also, you need to not remove the Reviewed by NOBODY (OOPS!) line.

[Lucas Forschler]

Created attachment 100356 [details]
Updated patch with ChangeLog corrections

update collision with eric's change 90412.  Updated to use sys.exit()

As for the comment about why there is no 't' in WebKitPluginAgen..  when I examined the processes list, it didn't show a 't' in the process name.  I don't think it was truncated in my view, but I could be mistaken.

[Eric Seidel (no email)]

Comment on attachment 99710 [details]
fixed patch.

Cleared Adam Roben's review+ from obsolete attachment 99710 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Eric Seidel (no email)]

Comment on attachment 99722 [details]
retry.

Cleared Adam Roben's review+ from obsolete attachment 99722 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Ryosuke Niwa]

Comment on attachment 100356 [details]
Updated patch with ChangeLog corrections

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

Looks sane to me.

> Tools/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=63651
> +        Update kill-old-processes logic.

The order in which bug title and url appear is wrong. Also you need a blank line after these two lines. Please fix that before you land.

[Ryosuke Niwa]

Comment on attachment 100356 [details]
Updated patch with ChangeLog corrections

Oops, I meant cq-.

[Ryosuke Niwa]

Committed r97341: <http://trac.webkit.org/changeset/97341>