Update mb.py to support gn directories with no args.gn

Update mb.py to support gn directories with no args.gn

A newly generated gn directory doesn't have an args.gn file so mb.py
needs to account for that. This is done by using toolchain.ninja to
identify a gn directory, and by teaching GNValsFromDir to handle a
missing args.gn file. The unittest was also updated to provide the
toolchain.ninja file.

BUG=616631
Committed: https://crrev.com/ecc0c1cd87f50efda32b82494c0c12b27798264b
Cr-Commit-Position: refs/heads/master@{#397476}

[brucedawson, 2016-05-31 20:28:42]

brucedawson@chromium.org changed reviewers:
+ dpranke@chromium.org

[brucedawson, 2016-05-31 20:28:43]

While doing some isolate testing I was stumped by this error message for a
while. I think it should contain something like the information in this CL. The
exact details are not crucial. PTAL.

[Dirk Pranke, 2016-05-31 20:39:03]

https://codereview.chromium.org/2027823002/diff/1/tools/mb/mb.py
File tools/mb/mb.py (right):

https://codereview.chromium.org/2027823002/diff/1/tools/mb/mb.py#newcode535
tools/mb/mb.py:535: (gn_args_path, build_dir, build_dir))
Ah, good catch. 

However, the MB code is actually wrong since, as you've discovered, you can have
a perfectly legit gn-built build directory w/o any args.gn file.

MB needs to be smarter about this and check for the existence of a
toolchain.ninja file in that directory as well, as we do in 

https://code.google.com/p/chromium/codesearch?q=factory.py#chromium/src/third...

MB's commands also work w/ GYP builds, so this message isn't quite right as-is
and we might want to use a slightly different message like the one on line 87 in
the above file.

Do you want to make that change, or shall I?

[brucedawson, 2016-05-31 20:44:03]

https://codereview.chromium.org/2027823002/diff/1/tools/mb/mb.py
File tools/mb/mb.py (right):

https://codereview.chromium.org/2027823002/diff/1/tools/mb/mb.py#newcode535
tools/mb/mb.py:535: (gn_args_path, build_dir, build_dir))
Making "it just work" would be better. Is checking for toolchain.ninja the
appropriate thing to do?

The message could also be improved, perhaps, but if the check is accurate then
that probably isn't necessary.

If the change is just to check for toolchain.ninja instead of args.gn then I can
do that.

[Dirk Pranke, 2016-05-31 20:48:23]

https://codereview.chromium.org/2027823002/diff/1/tools/mb/mb.py
File tools/mb/mb.py (right):

https://codereview.chromium.org/2027823002/diff/1/tools/mb/mb.py#newcode535
tools/mb/mb.py:535: (gn_args_path, build_dir, build_dir))
On 2016/05/31 20:44:03, brucedawson wrote:
> Making "it just work" would be better. Is checking for toolchain.ninja the
> appropriate thing to do?
> 
> The message could also be improved, perhaps, but if the check is accurate then
> that probably isn't necessary.
> 
> If the change is just to check for toolchain.ninja instead of args.gn then I
can
> do that.

It's a little bit more complicated. If we think this is a GN build directory,
then we try to read
from args.gn to figure out the list of args (see line 543). So you'd need to
modify GNValsFromDir() to handle a missing
args.gn file.

But that should still be straightforward. 

And, yes, checking for toolchain.ninja is sufficient as long as you're not
trying to defend against the case where you have GYP-generated and GN-generated
files in the same directory (which I think we assert for and error out on
somewhere anyway).

[brucedawson, 2016-06-01 23:44:16]

Description was changed from

==========
Improve error message for mb.py isolate on new directory

If you create a new gn output directory with "gn gen" then it does not
contain an args.gn file. This causes mb.py isolate to fail with a
cryptic error message. In order to save users from having to look at the
script source code in order to troubleshoot the error message should:
- mention that args.gn was not found
- mention that args.gn can be generated using "gn args"

This change adds both of those bits of information to the error message.
==========

to

==========
Update mb.py to support gn directories with no args.gn

A newly generated gn directory doesn't have an args.gn file so mb.py
needs to account for that. This is done by using toolchain.ninja to
identify a gn directory, and by teaching GNValsFromDir to handle a
missing args.gn file. The unittest was also updated to provide the
toolchain.ninja file.
==========

[brucedawson, 2016-06-01 23:45:38]

mb.py isolate now handles freshly generated gn directories, and the tests pass.

I thought about using try/except to handle the possibly missing args.gn but
ended up slightly preferring Exists().

PTAL.

[brucedawson, 2016-06-01 23:45:54]

Description was changed from

==========
Update mb.py to support gn directories with no args.gn

A newly generated gn directory doesn't have an args.gn file so mb.py
needs to account for that. This is done by using toolchain.ninja to
identify a gn directory, and by teaching GNValsFromDir to handle a
missing args.gn file. The unittest was also updated to provide the
toolchain.ninja file.
==========

to

==========
Update mb.py to support gn directories with no args.gn

A newly generated gn directory doesn't have an args.gn file so mb.py
needs to account for that. This is done by using toolchain.ninja to
identify a gn directory, and by teaching GNValsFromDir to handle a
missing args.gn file. The unittest was also updated to provide the
toolchain.ninja file.

BUG=616631
==========

[Dirk Pranke, 2016-06-02 00:30:55]

lgtm, thanks!

[brucedawson, 2016-06-02 00:51:39]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-06-02 00:52:51]

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

[commit-bot: I haz the power, 2016-06-02 01:59:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-02 01:59:49]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brucedawson, 2016-06-02 16:57:47]

On 2016/06/02 01:59:49, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux
(JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

"""<H1>Over Quota</H1>
This application is temporarily over its serving quota.  Please try again
later."""

Okay - trying again later.

[brucedawson, 2016-06-02 16:57:51]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-06-02 16:58:32]

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

[commit-bot: I haz the power, 2016-06-02 18:25:12]

Description was changed from

==========
Update mb.py to support gn directories with no args.gn

A newly generated gn directory doesn't have an args.gn file so mb.py
needs to account for that. This is done by using toolchain.ninja to
identify a gn directory, and by teaching GNValsFromDir to handle a
missing args.gn file. The unittest was also updated to provide the
toolchain.ninja file.

BUG=616631
==========

to

==========
Update mb.py to support gn directories with no args.gn

A newly generated gn directory doesn't have an args.gn file so mb.py
needs to account for that. This is done by using toolchain.ninja to
identify a gn directory, and by teaching GNValsFromDir to handle a
missing args.gn file. The unittest was also updated to provide the
toolchain.ninja file.

BUG=616631
==========

[commit-bot: I haz the power, 2016-06-02 18:25:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-02 18:26:23]

Description was changed from

==========
Update mb.py to support gn directories with no args.gn

A newly generated gn directory doesn't have an args.gn file so mb.py
needs to account for that. This is done by using toolchain.ninja to
identify a gn directory, and by teaching GNValsFromDir to handle a
missing args.gn file. The unittest was also updated to provide the
toolchain.ninja file.

BUG=616631
==========

to

==========
Update mb.py to support gn directories with no args.gn

A newly generated gn directory doesn't have an args.gn file so mb.py
needs to account for that. This is done by using toolchain.ninja to
identify a gn directory, and by teaching GNValsFromDir to handle a
missing args.gn file. The unittest was also updated to provide the
toolchain.ninja file.

BUG=616631

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

[commit-bot: I haz the power, 2016-06-02 18:26:24]

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