New script to build the mojom parser.

New script to build the mojom parser.

R=rudominer@chromium.org, rudominer
BUG= #582

Committed: https://chromium.googlesource.com/external/mojo/+/e2d97ce64b7ea12ffcfb70d5135e846c1e2b126a

[azani, 2015-12-16 16:48:18]

azani@chromium.org changed reviewers:
- rudominer@chromium.org

[azani, 2015-12-16 16:48:18]

Sorry, I actually want to work on this a little more before you review it.
Please ignore.

[azani, 2015-12-16 22:17:28]

Description was changed from

==========
New script to build the mojom parser.

R=rudominer
BUG=
==========

to

==========
New script to build the mojom parser.

R=rudominer
BUG= #582
==========

[azani, 2015-12-16 22:17:28]

azani@chromium.org changed reviewers:
+ rudominer@chromium.org

[azani, 2015-12-16 22:17:34]

ptal

[rudominer, 2015-12-16 22:47:41]

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
File mojom/mojom_parser/tools/build_mojom_parser.py (right):

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:18: src_root: Path to the root of
the source tree.
if None then...

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:19: go_tool: Path to the go tool.
if None then...

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:20: go_root: Path to the root of
the go installation.
if None then...

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:31: src_root =
os.path.dirname(src_root)
It seems more fragile to me to have the hidden dependency that this script can
never be moved into a folder named 'src' than it does to have the explicit
dependency that this script can never be moved at all. I would just assume this
script is in a specific location.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:58: output = 'mojom_parser_%s_%s'
% (target_os, target_arch)
How about: the output directory is an argument to this script. It defaults to
cwd. When invoked from ninja we will use some directory in out/Debug or
out/Release etc.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:69: result =
subprocess.call([go_tool, 'build', '-o', output] , env=environ)
I was under the impression that we had an older version of the go tool currently
in our repo that did not support -o. If so we will have to upgrade.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:81: help='Path to the source
tree.')
add to 'optional' help string.
(Do you envision anyone ever using this?)

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:86: args = parser.parse_args()
out_dir seems like a natural optional argument.

[azani, 2015-12-16 23:11:04]

ptal

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
File mojom/mojom_parser/tools/build_mojom_parser.py (right):

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:18: src_root: Path to the root of
the source tree.
On 2015/12/16 22:47:40, rudominer wrote:
> if None then...

Done.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:19: go_tool: Path to the go tool.
On 2015/12/16 22:47:40, rudominer wrote:
> if None then...

Done.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:20: go_root: Path to the root of
the go installation.
On 2015/12/16 22:47:41, rudominer wrote:
> if None then...

Done.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:31: src_root =
os.path.dirname(src_root)
On 2015/12/16 22:47:41, rudominer wrote:
> It seems more fragile to me to have the hidden dependency that this script can
> never be moved into a folder named 'src' than it does to have the explicit
> dependency that this script can never be moved at all. I would just assume
this
> script is in a specific location.

Done.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:58: output = 'mojom_parser_%s_%s'
% (target_os, target_arch)
On 2015/12/16 22:47:40, rudominer wrote:
> How about: the output directory is an argument to this script. It defaults to
> cwd. When invoked from ninja we will use some directory in out/Debug or
> out/Release etc.

Done.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:69: result =
subprocess.call([go_tool, 'build', '-o', output] , env=environ)
On 2015/12/16 22:47:41, rudominer wrote:
> I was under the impression that we had an older version of the go tool
currently
> in our repo that did not support -o. If so we will have to upgrade.

It works and if you call help build on it, it documents -o.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:81: help='Path to the source
tree.')
On 2015/12/16 22:47:41, rudominer wrote:
> add to 'optional' help string.
> (Do you envision anyone ever using this?)

I mostly put it there for completeness' sake.

https://codereview.chromium.org/1532603002/diff/40001/mojom/mojom_parser/tool...
mojom/mojom_parser/tools/build_mojom_parser.py:86: args = parser.parse_args()
On 2015/12/16 22:47:41, rudominer wrote:
> out_dir seems like a natural optional argument.

Done.

[rudominer, 2015-12-16 23:20:48]

lgtm

[azani, 2015-12-16 23:45:46]

Description was changed from

==========
New script to build the mojom parser.

R=rudominer
BUG= #582
==========

to

==========
New script to build the mojom parser.

R=rudominer@chromium.org, rudominer
BUG= #582

Committed:
https://chromium.googlesource.com/external/mojo/+/e2d97ce64b7ea12ffcfb70d5135...
==========

[azani, 2015-12-16 23:45:48]

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