Makes mojob create a different directory when --asan is specified

Makes mojob create a different directory when --asan is specified

This makes it easier to have both asan and non-asan builds on the same
machine.

BUG=438734
TEST=none
R=eseidel@chromium.org, viettrungluu@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/665d845bf07466b8a83937897684bcc9280aa378

[viettrungluu, 2014-12-04 23:30:54]

https://codereview.chromium.org/775343003/diff/20001/mojo/tools/get_test_list.py
File mojo/tools/get_test_list.py (right):

https://codereview.chromium.org/775343003/diff/20001/mojo/tools/get_test_list...
mojo/tools/get_test_list.py:119: "-t", os.path.basename(build_dir),
Does test_sky really just take a directory (base name) here?

(Its help is vague, basically implying that it's either "Debug" or "Release".
Maybe the help should be updated.)

https://codereview.chromium.org/775343003/diff/20001/mojo/tools/mojob.py
File mojo/tools/mojob.py (right):

https://codereview.chromium.org/775343003/diff/20001/mojo/tools/mojob.py#newc...
mojo/tools/mojob.py:236: build_parser.add_argument('--asan', help='Uses Address
Sanitizer',
Just add "--asan" to parent_parser, once.

[sky, 2014-12-05 00:01:38]

sky@chromium.org changed reviewers:
+ eseidel@chromium.org

[sky, 2014-12-05 00:01:39]

+eseidel

Eric, we're making mojob output to a directory with asan in the name when doing
asan builds, eg Debug_asan. This means test_sky needs the ability to take an
arbitrary directory. The only way I could get this to work is with -t
<dir-name>. This seems like a mild abuse of -t though. Should I add a --asan
option that adds _asan to the directory?

https://codereview.chromium.org/775343003/diff/20001/mojo/tools/get_test_list.py
File mojo/tools/get_test_list.py (right):

https://codereview.chromium.org/775343003/diff/20001/mojo/tools/get_test_list...
mojo/tools/get_test_list.py:119: "-t", os.path.basename(build_dir),
On 2014/12/04 23:30:54, viettrungluu wrote:
> Does test_sky really just take a directory (base name) here?
> 
> (Its help is vague, basically implying that it's either "Debug" or "Release".
> Maybe the help should be updated.)

The script supports a --build-directory argument as well, but that's relative to
out/Type. Specifying Release_asan worked fine. I don't know if that is abusing
the type though.

[eseidel, 2014-12-05 00:20:44]

I'm not familiar with mojob.  James is probably a better reviewer here.

[sky, 2014-12-05 00:22:59]

On 2014/12/05 00:20:44, eseidel wrote:
> I'm not familiar with mojob.  James is probably a better reviewer here.

Sorry for not being clear.

I have the mojob changes done. The only question is how Release_asan (or
whatever) gets passed to test_sky. See
https://codereview.chromium.org/775343003/diff/40001/mojo/tools/get_test_list.py
. I'm using -t foo, where foo will end up with something like Release_asan for
asan builds. WDYT?

[eseidel, 2014-12-05 00:23:00]

This seems fine.  -t is exactly for specifying the build dir. :)

[viettrungluu, 2014-12-05 01:33:22]

On 2014/12/05 00:23:00, eseidel wrote:
> This seems fine.  -t is exactly for specifying the build dir. :)

I guess we should update test_sky's help text then. :)

[viettrungluu, 2014-12-05 01:35:05]

LGTM w/request

https://codereview.chromium.org/775343003/diff/40001/mojo/tools/mojob.py
File mojo/tools/mojob.py (right):

https://codereview.chromium.org/775343003/diff/40001/mojo/tools/mojob.py#newc...
mojo/tools/mojob.py:194: parent_parser.add_argument('--asan', help='Uses Address
Sanitizer',
Could you, as a drive-by, update "uses" to "Use"? (I can't unsee this.)

[eseidel, 2014-12-05 02:23:14]

I guess I was confused.  It looks like it's:
    --build-directory=BUILD_DIRECTORY
                        Path to the directory under which build files are kept
                        (should not include configuration)

[eseidel, 2014-12-05 02:24:05]

eseidel@chromium.org changed reviewers:
+ ojan@chromium.org

[sky, 2014-12-05 15:51:03]

On 2014/12/05 02:24:06, eseidel wrote:

I had originally tried --build-directory (see earlier comments). It appears as
though the --build-directory arg becomes the prefix. So, if I do -t Release
--build-directory _asan I end with a command line of _asan/Release/mojo_shell
(note the last of leading path). When I do -t Release_asan I get the behavior I
want.

You ok with what I have here for now, or should --build-directory become a
suffix to release-type and this is a bug in one of the sky scripts that I need
to fix first?

[eseidel, 2014-12-05 16:24:33]

Right now test_sky is just run-webkit-tests from Chromium under a differnet name
and hugely more complicated than we need.  If -t works for you, I'd just do
that.  I'm sure Chromium must have solved this problem somehow, so we could also
just crib their solution.

Regardless, lgtm.

[sky, 2014-12-05 16:50:24]

https://codereview.chromium.org/775343003/diff/40001/mojo/tools/mojob.py
File mojo/tools/mojob.py (right):

https://codereview.chromium.org/775343003/diff/40001/mojo/tools/mojob.py#newc...
mojo/tools/mojob.py:194: parent_parser.add_argument('--asan', help='Uses Address
Sanitizer',
On 2014/12/05 01:35:05, viettrungluu wrote:
> Could you, as a drive-by, update "uses" to "Use"? (I can't unsee this.)

Done.

[sky, 2014-12-05 16:57:16]

Committed patchset #4 (id:60001) manually as
665d845bf07466b8a83937897684bcc9280aa378 (presubmit successful).