Share logic for generating compile DB with run_tool.py

Share logic for generating compile DB with run_tool.py

BUG=676588
R=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2798233002 .
Cr-Commit-Position: refs/heads/master@{#462582}
Committed: https://chromium.googlesource.com/chromium/src/+/886eadcb2ee08f462586e3281c4b682774884e36

[dcheng, 2017-04-06 05:48:55]

dcheng@chromium.org changed reviewers:
+ thakis@chromium.org

[dcheng, 2017-04-06 05:48:56]

Mostly just shuffling some code around. I'm not sure we need generate_compdb.py
anymore, but it's a helpful debugging utility for now.

https://codereview.chromium.org/2798233002/diff/20001/tools/clang/pylib/clang...
File tools/clang/pylib/clang/compile_db.py (right):

https://codereview.chromium.org/2798233002/diff/20001/tools/clang/pylib/clang...
tools/clang/pylib/clang/compile_db.py:24: if
split_command[0].endswith('gomacc.exe'):
This wasn't in the original generate_win_compdb.py, but I added it: otherwise,
the original logic to insert --driver-mode=cl confused the clang tooling because
the command ends up looking like
gomacc.exe --driver-mode=cl clang-cl.exe

https://codereview.chromium.org/2798233002/diff/20001/tools/clang/scripts/gen...
File tools/clang/scripts/generate_win_compdb.py (left):

https://codereview.chromium.org/2798233002/diff/20001/tools/clang/scripts/gen...
tools/clang/scripts/generate_win_compdb.py:23: _NINJA_MSVC_WRAPPER =
re.compile('ninja -t msvc -e .+? -- ')
It looks like ninja doesn't emit this wrapper anymore for is_clang=true builds
on Windows, so I removed it.

[Nico, 2017-04-06 13:30:52]

lgtm since it's just moving things around, but I found a few minor things in
that existing code upon re-reading.

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
File tools/clang/pylib/clang/compile_db.py (right):

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
tools/clang/pylib/clang/compile_db.py:81: # gn args --list=is_clang --short
I don't think that does the right thing when clang is enabled by default. On my
Mac:

$  gn args --list=is_clang --short out/gn
is_clang = false

Yet only building with clang is supported on mac.

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
tools/clang/pylib/clang/compile_db.py:95: f.write(json.dumps(compile_db,
indent=2))
  with open(os.path.join(path, 'compile_commands.json'), 'w') as f:
    f.write...

1. use open, not file
(http://stackoverflow.com/questions/112970/python-when-to-use-file-vs-open)
2. use a scoper so the file gets closed again immediately after it's no longer
needed

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
File tools/clang/scripts/generate_compdb.py (right):

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
tools/clang/scripts/generate_compdb.py:9: addition, ninja generates compile
commands prefixed with the ninja msvc helper,
Only if you don't use is_clang these days. In clang builds, the msvc helper
isn't' used for compiles anymore.

[Ramin Halavati, 2017-04-06 13:59:43]

On 2017/04/06 13:30:52, Nico wrote:
> lgtm since it's just moving things around, but I found a few minor things in
> that existing code upon re-reading.
> 
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> File tools/clang/pylib/clang/compile_db.py (right):
> 
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> tools/clang/pylib/clang/compile_db.py:81: # gn args --list=is_clang --short
> I don't think that does the right thing when clang is enabled by default. On
my
> Mac:
> 
> $  gn args --list=is_clang --short out/gn
> is_clang = false
> 
> Yet only building with clang is supported on mac.
> 
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> tools/clang/pylib/clang/compile_db.py:95: f.write(json.dumps(compile_db,
> indent=2))
>   with open(os.path.join(path, 'compile_commands.json'), 'w') as f:
>     f.write...
> 
> 1. use open, not file
> (http://stackoverflow.com/questions/112970/python-when-to-use-file-vs-open)
> 2. use a scoper so the file gets closed again immediately after it's no longer
> needed
> 
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
> File tools/clang/scripts/generate_compdb.py (right):
> 
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
> tools/clang/scripts/generate_compdb.py:9: addition, ninja generates compile
> commands prefixed with the ninja msvc helper,
> Only if you don't use is_clang these days. In clang builds, the msvc helper
> isn't' used for compiles anymore.

I think generate_win_compdb.py is still required. I tested a clang tool
(https://codereview.chromium.org/2448133006/) as follow on Windows:

1- gn args out/Debug --> added is_clang=true.
2- ninja -C out/Debug chrome -d keeprsp
3- python tools/clang/scripts/run_tool.py traffic_annotation_extractor out/Debug
components/spellcheck/browser --tool-args=c:\workspace

and it failed.

Then I ran 'python tools/clang/scripts/generate_win_compdb.py' and repeated step
3, and it worked this time.

Also tried 'python tools/clang/scripts/run_tool.py traffic_annotation_extractor
--generate-compdb out/Debug components/spellcheck/browser
--tool-args=c:\workspace' and it failed.

[dcheng, 2017-04-06 15:21:45]

On 2017/04/06 13:59:43, Ramin Halavati wrote:
> On 2017/04/06 13:30:52, Nico wrote:
> > lgtm since it's just moving things around, but I found a few minor things in
> > that existing code upon re-reading.
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> > File tools/clang/pylib/clang/compile_db.py (right):
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> > tools/clang/pylib/clang/compile_db.py:81: # gn args --list=is_clang --short
> > I don't think that does the right thing when clang is enabled by default. On
> my
> > Mac:
> > 
> > $  gn args --list=is_clang --short out/gn
> > is_clang = false
> > 
> > Yet only building with clang is supported on mac.
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> > tools/clang/pylib/clang/compile_db.py:95: f.write(json.dumps(compile_db,
> > indent=2))
> >   with open(os.path.join(path, 'compile_commands.json'), 'w') as f:
> >     f.write...
> > 
> > 1. use open, not file
> > (http://stackoverflow.com/questions/112970/python-when-to-use-file-vs-open)
> > 2. use a scoper so the file gets closed again immediately after it's no
longer
> > needed
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
> > File tools/clang/scripts/generate_compdb.py (right):
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
> > tools/clang/scripts/generate_compdb.py:9: addition, ninja generates compile
> > commands prefixed with the ninja msvc helper,
> > Only if you don't use is_clang these days. In clang builds, the msvc helper
> > isn't' used for compiles anymore.
> 
> I think generate_win_compdb.py is still required. I tested a clang tool
> (https://codereview.chromium.org/2448133006/) as follow on Windows:
> 
> 1- gn args out/Debug --> added is_clang=true.
> 2- ninja -C out/Debug chrome -d keeprsp
> 3- python tools/clang/scripts/run_tool.py traffic_annotation_extractor
out/Debug
> components/spellcheck/browser --tool-args=c:\workspace
> 
> and it failed.
> 
> Then I ran 'python tools/clang/scripts/generate_win_compdb.py' and repeated
step
> 3, and it worked this time.
> 
> Also tried 'python tools/clang/scripts/run_tool.py
traffic_annotation_extractor
> --generate-compdb out/Debug components/spellcheck/browser
> --tool-args=c:\workspace' and it failed.

What errors did you see? It worked for me when I tried it. Can you include a
sample entry from the compile database to help debug?

[dcheng, 2017-04-06 15:34:16]

On 2017/04/06 15:21:45, dcheng wrote:
> On 2017/04/06 13:59:43, Ramin Halavati wrote:
> > On 2017/04/06 13:30:52, Nico wrote:
> > > lgtm since it's just moving things around, but I found a few minor things
in
> > > that existing code upon re-reading.
> > > 
> > >
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> > > File tools/clang/pylib/clang/compile_db.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> > > tools/clang/pylib/clang/compile_db.py:81: # gn args --list=is_clang
--short
> > > I don't think that does the right thing when clang is enabled by default.
On
> > my
> > > Mac:
> > > 
> > > $  gn args --list=is_clang --short out/gn
> > > is_clang = false
> > > 
> > > Yet only building with clang is supported on mac.
> > > 
> > >
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> > > tools/clang/pylib/clang/compile_db.py:95: f.write(json.dumps(compile_db,
> > > indent=2))
> > >   with open(os.path.join(path, 'compile_commands.json'), 'w') as f:
> > >     f.write...
> > > 
> > > 1. use open, not file
> > >
(http://stackoverflow.com/questions/112970/python-when-to-use-file-vs-open)
> > > 2. use a scoper so the file gets closed again immediately after it's no
> longer
> > > needed
> > > 
> > >
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
> > > File tools/clang/scripts/generate_compdb.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
> > > tools/clang/scripts/generate_compdb.py:9: addition, ninja generates
compile
> > > commands prefixed with the ninja msvc helper,
> > > Only if you don't use is_clang these days. In clang builds, the msvc
helper
> > > isn't' used for compiles anymore.
> > 
> > I think generate_win_compdb.py is still required. I tested a clang tool
> > (https://codereview.chromium.org/2448133006/) as follow on Windows:
> > 
> > 1- gn args out/Debug --> added is_clang=true.
> > 2- ninja -C out/Debug chrome -d keeprsp
> > 3- python tools/clang/scripts/run_tool.py traffic_annotation_extractor
> out/Debug
> > components/spellcheck/browser --tool-args=c:\workspace
> > 
> > and it failed.
> > 
> > Then I ran 'python tools/clang/scripts/generate_win_compdb.py' and repeated
> step
> > 3, and it worked this time.
> > 
> > Also tried 'python tools/clang/scripts/run_tool.py
> traffic_annotation_extractor
> > --generate-compdb out/Debug components/spellcheck/browser
> > --tool-args=c:\workspace' and it failed.
> 
> What errors did you see? It worked for me when I tried it. Can you include a
> sample entry from the compile database to help debug?

Also, what are your GN args?

[dcheng, 2017-04-06 18:54:46]

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
File tools/clang/pylib/clang/compile_db.py (right):

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
tools/clang/pylib/clang/compile_db.py:81: # gn args --list=is_clang --short
On 2017/04/06 13:30:52, Nico wrote:
> I don't think that does the right thing when clang is enabled by default. On
my
> Mac:
> 
> $  gn args --list=is_clang --short out/gn
> is_clang = false
> 
> Yet only building with clang is supported on mac.

Doh, I thought it worked on Linux when I tested it... but I guess not. Is there
a better way to do this?

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
tools/clang/pylib/clang/compile_db.py:95: f.write(json.dumps(compile_db,
indent=2))
On 2017/04/06 13:30:52, Nico wrote:
>   with open(os.path.join(path, 'compile_commands.json'), 'w') as f:
>     f.write...
> 
> 1. use open, not file
> (http://stackoverflow.com/questions/112970/python-when-to-use-file-vs-open)
> 2. use a scoper so the file gets closed again immediately after it's no longer
> needed

Done.

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
File tools/clang/scripts/generate_compdb.py (right):

https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
tools/clang/scripts/generate_compdb.py:9: addition, ninja generates compile
commands prefixed with the ninja msvc helper,
On 2017/04/06 13:30:52, Nico wrote:
> Only if you don't use is_clang these days. In clang builds, the msvc helper
> isn't' used for compiles anymore.

Oops, forgot to update this comment. Done.

[dcheng, 2017-04-06 19:20:56]

On 2017/04/06 13:59:43, Ramin Halavati wrote:
> On 2017/04/06 13:30:52, Nico wrote:
> > lgtm since it's just moving things around, but I found a few minor things in
> > that existing code upon re-reading.
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> > File tools/clang/pylib/clang/compile_db.py (right):
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> > tools/clang/pylib/clang/compile_db.py:81: # gn args --list=is_clang --short
> > I don't think that does the right thing when clang is enabled by default. On
> my
> > Mac:
> > 
> > $  gn args --list=is_clang --short out/gn
> > is_clang = false
> > 
> > Yet only building with clang is supported on mac.
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/pylib/clang...
> > tools/clang/pylib/clang/compile_db.py:95: f.write(json.dumps(compile_db,
> > indent=2))
> >   with open(os.path.join(path, 'compile_commands.json'), 'w') as f:
> >     f.write...
> > 
> > 1. use open, not file
> > (http://stackoverflow.com/questions/112970/python-when-to-use-file-vs-open)
> > 2. use a scoper so the file gets closed again immediately after it's no
longer
> > needed
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
> > File tools/clang/scripts/generate_compdb.py (right):
> > 
> >
>
https://codereview.chromium.org/2798233002/diff/60001/tools/clang/scripts/gen...
> > tools/clang/scripts/generate_compdb.py:9: addition, ninja generates compile
> > commands prefixed with the ninja msvc helper,
> > Only if you don't use is_clang these days. In clang builds, the msvc helper
> > isn't' used for compiles anymore.
> 
> I think generate_win_compdb.py is still required. I tested a clang tool
> (https://codereview.chromium.org/2448133006/) as follow on Windows:
> 
> 1- gn args out/Debug --> added is_clang=true.
> 2- ninja -C out/Debug chrome -d keeprsp
> 3- python tools/clang/scripts/run_tool.py traffic_annotation_extractor
out/Debug
> components/spellcheck/browser --tool-args=c:\workspace
> 
> and it failed.
> 
> Then I ran 'python tools/clang/scripts/generate_win_compdb.py' and repeated
step
> 3, and it worked this time.
> 
> Also tried 'python tools/clang/scripts/run_tool.py
traffic_annotation_extractor
> --generate-compdb out/Debug components/spellcheck/browser
> --tool-args=c:\workspace' and it failed.

I talked with Ramin offline, and the error was due to a miscommunication. This
CL should be fine, and simplify things for the Windows workflow, so going to go
ahead and land it.

[dcheng, 2017-04-06 19:26:49]

Description was changed from

==========
Share logic for generating compile DB with run_tool.py

BUG=676588
==========

to

==========
Share logic for generating compile DB with run_tool.py

BUG=676588
R=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2798233002 .
Cr-Commit-Position: refs/heads/master@{#462582}
Committed:
https://chromium.googlesource.com/chromium/src/+/886eadcb2ee08f462586e3281c4b...
==========

[dcheng, 2017-04-06 19:26:50]

Committed patchset #5 (id:80001) manually as
886eadcb2ee08f462586e3281c4b682774884e36 (presubmit successful).