[Swarming] Isolate valgrind runner.

[Swarming] Isolate valgrind runner.

This creates an isolate with the v8-side valgrind wrapper and
the valgrind binaries from third_party. It's dynamically
checked if the directories are present.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n

Committed: https://crrev.com/b065c216e751a4bb3514a21e8831e04635ef9d92
Cr-Commit-Position: refs/heads/master@{#33992}

[Michael Achenbach, 2016-02-15 08:30:50]

Description was changed from

==========
[Swarming] Isolate valgrind runner.

BUG=
==========

to

==========
[Swarming] Isolate valgrind runner.

BUG=chromium:535160
LOG=n
NOTRY=true
==========

[Michael Achenbach, 2016-02-15 08:30:50]

machenbach@chromium.org changed reviewers:
+ kjellander@chromium.org, tandrii@chromium.org

[Michael Achenbach, 2016-02-15 08:32:38]

Description was changed from

==========
[Swarming] Isolate valgrind runner.

BUG=chromium:535160
LOG=n
NOTRY=true
==========

to

==========
[Swarming] Isolate valgrind runner.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n
NOTRY=true
==========

[Michael Achenbach, 2016-02-15 08:32:48]

Description was changed from

==========
[Swarming] Isolate valgrind runner.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n
NOTRY=true
==========

to

==========
[Swarming] Isolate valgrind runner.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n
==========

[Michael Achenbach, 2016-02-15 08:34:13]

Description was changed from

==========
[Swarming] Isolate valgrind runner.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n
==========

to

==========
[Swarming] Isolate valgrind runner.

This creates an isolate with the v8-side valgrind wrapper and
the valgrind binaries from third_party. It's dynamically
checked if the directories are present.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n
==========

[Michael Achenbach, 2016-02-15 08:35:30]

PTAL

[kjellander_chromium, 2016-02-15 08:48:28]

A questions and a few nits. You may want to sync up with zhaoqin@ and dpranke@
in http://crbug.com/572694 since they're about to start swarmify those bots
after moving them over to recipes (finally).

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py
File build/has_valgrind.py (right):

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py#newco...
build/has_valgrind.py:12: 
two blank lines before top-level function

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py#newco...
build/has_valgrind.py:13: def DoMain(_):
name this main(_): 
https://google.github.io/styleguide/pyguide.html?showone=Main#Main

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py#newco...
build/has_valgrind.py:17: 
+1 blank line

https://codereview.chromium.org/1700673002/diff/1/tools/run-valgrind.isolate
File tools/run-valgrind.isolate (right):

https://codereview.chromium.org/1700673002/diff/1/tools/run-valgrind.isolate#...
tools/run-valgrind.isolate:17: # This assumes vagrind binaries have been fetched
as a custom deps into
nit: wrap at 80 chars.

https://codereview.chromium.org/1700673002/diff/1/tools/run-valgrind.isolate#...
tools/run-valgrind.isolate:20: '../third_party/valgrind/linux_x86/',
You could use the target_arch variable for checks in the .isolate files. I
thought it was known which target machine architecture you'll run on - don't you
need to know that in order to avoid the risk of running a 64-bit binary on a
32-bit system?

[Michael Achenbach, 2016-02-15 09:55:46]

PTAL

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py
File build/has_valgrind.py (right):

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py#newco...
build/has_valgrind.py:12: 
On 2016/02/15 08:48:28, kjellander (chromium) wrote:
> two blank lines before top-level function

Done.

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py#newco...
build/has_valgrind.py:13: def DoMain(_):
On 2016/02/15 08:48:28, kjellander (chromium) wrote:
> name this main(_): 
> https://google.github.io/styleguide/pyguide.html?showone=Main#Main

Won't work. This is not the python main function but a gyp hook. Gyp expects it
to be called like that and calls the function directly with the pymod_do_main
action in standalone.gypi. It is widely used
https://code.google.com/p/chromium/codesearch#search/&q=pymod_do_main&sq=pack...
and well documented as usual ;)

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py#newco...
build/has_valgrind.py:17: 
On 2016/02/15 08:48:28, kjellander (chromium) wrote:
> +1 blank line

Done.

https://codereview.chromium.org/1700673002/diff/1/tools/run-valgrind.isolate
File tools/run-valgrind.isolate (right):

https://codereview.chromium.org/1700673002/diff/1/tools/run-valgrind.isolate#...
tools/run-valgrind.isolate:17: # This assumes vagrind binaries have been fetched
as a custom deps into
On 2016/02/15 08:48:28, kjellander (chromium) wrote:
> nit: wrap at 80 chars.

Done.

https://codereview.chromium.org/1700673002/diff/1/tools/run-valgrind.isolate#...
tools/run-valgrind.isolate:20: '../third_party/valgrind/linux_x86/',
On 2016/02/15 08:48:28, kjellander (chromium) wrote:
> You could use the target_arch variable for checks in the .isolate files. I
> thought it was known which target machine architecture you'll run on - don't
you
> need to know that in order to avoid the risk of running a 64-bit binary on a
> 32-bit system?

That way yes. But I could very well run a 32 bit binary in a 64 bit system. If
my target build is 64 bits, the target machine must be 64 as well, if my target
build is 32 bits, the target machine could be both (in theory - in practice, I
think all bots are 64 bits - but this doesn't cost much).

[Michael Achenbach, 2016-02-15 10:00:46]

On 2016/02/15 08:48:28, kjellander (chromium) wrote:
> A questions and a few nits. You may want to sync up with zhaoqin@ and dpranke@
> in http://crbug.com/572694 since they're about to start swarmify those bots
> after moving them over to recipes (finally).

I basically c/p'ed the current syncing design here: 
https://codereview.chromium.org/1583933006/

In all other regards, chromium is different as they integrate the stuff in their
run-tests.py, which I'm not going to spend time on. Also our infra stuff is
home-grown...

[kjellander_chromium, 2016-02-15 11:49:26]

lgtm

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py
File build/has_valgrind.py (right):

https://codereview.chromium.org/1700673002/diff/1/build/has_valgrind.py#newco...
build/has_valgrind.py:13: def DoMain(_):
On 2016/02/15 09:55:45, Michael Achenbach wrote:
> On 2016/02/15 08:48:28, kjellander (chromium) wrote:
> > name this main(_): 
> > https://google.github.io/styleguide/pyguide.html?showone=Main#Main
> 
> Won't work. This is not the python main function but a gyp hook. Gyp expects
it
> to be called like that and calls the function directly with the pymod_do_main
> action in standalone.gypi. It is widely used
>
https://code.google.com/p/chromium/codesearch#search/&q=pymod_do_main&sq=pack...
> and well documented as usual ;)

Ah, I didn't pay attention to (or knew about) that. Sorry - I should have known
that you know how to name a main function ;)

https://codereview.chromium.org/1700673002/diff/1/tools/run-valgrind.isolate
File tools/run-valgrind.isolate (right):

https://codereview.chromium.org/1700673002/diff/1/tools/run-valgrind.isolate#...
tools/run-valgrind.isolate:20: '../third_party/valgrind/linux_x86/',
On 2016/02/15 09:55:45, Michael Achenbach wrote:
> On 2016/02/15 08:48:28, kjellander (chromium) wrote:
> > You could use the target_arch variable for checks in the .isolate files. I
> > thought it was known which target machine architecture you'll run on - don't
> you
> > need to know that in order to avoid the risk of running a 64-bit binary on a
> > 32-bit system?
> 
> That way yes. But I could very well run a 32 bit binary in a 64 bit system. If
> my target build is 64 bits, the target machine must be 64 as well, if my
target
> build is 32 bits, the target machine could be both (in theory - in practice, I
> think all bots are 64 bits - but this doesn't cost much).

Fair enough, simplicity is better.

[Michael Achenbach, 2016-02-15 12:28:22]

The CQ bit was checked by machenbach@chromium.org

[commit-bot: I haz the power, 2016-02-15 12:28:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1700673002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1700673002/20001

[commit-bot: I haz the power, 2016-02-15 12:47:19]

Description was changed from

==========
[Swarming] Isolate valgrind runner.

This creates an isolate with the v8-side valgrind wrapper and
the valgrind binaries from third_party. It's dynamically
checked if the directories are present.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n
==========

to

==========
[Swarming] Isolate valgrind runner.

This creates an isolate with the v8-side valgrind wrapper and
the valgrind binaries from third_party. It's dynamically
checked if the directories are present.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n
==========

[commit-bot: I haz the power, 2016-02-15 12:47:20]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-15 12:47:59]

Description was changed from

==========
[Swarming] Isolate valgrind runner.

This creates an isolate with the v8-side valgrind wrapper and
the valgrind binaries from third_party. It's dynamically
checked if the directories are present.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n
==========

to

==========
[Swarming] Isolate valgrind runner.

This creates an isolate with the v8-side valgrind wrapper and
the valgrind binaries from third_party. It's dynamically
checked if the directories are present.

Follow up after https://codereview.chromium.org/1585093002/
and https://codereview.chromium.org/1583933006/

BUG=chromium:535160
LOG=n

Committed: https://crrev.com/b065c216e751a4bb3514a21e8831e04635ef9d92
Cr-Commit-Position: refs/heads/master@{#33992}
==========

[commit-bot: I haz the power, 2016-02-15 12:47:59]

Patchset 2 (id:??) landed as
https://crrev.com/b065c216e751a4bb3514a21e8831e04635ef9d92
Cr-Commit-Position: refs/heads/master@{#33992}