[Findit] Check existence of given targets before running compile in recipe findit/chromium/compile.

[Findit] Check existence of given targets before running compile in recipe findit/chromium/compile.

A "target" here is actually a node in ninja's build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

Because a "target" could be added or deleted by some commit in the regression range, we have to check whether it still exists before re-running compile at a different commit. Otherwise, compile will always fail.
Use 'ninja -t query TARGET1 [TARGET2 ...]' to test whether "target"s exist or not: ninja exits with 0 if all the targets exist; otherwise with 1.


BUG=592715, 560991
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299206

[stgao, 2016-03-07 20:05:32]

Description was changed from

==========
[Findit] Check existence of given targets before running compile.

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

to

==========
[Findit] Check existence of given targets before running compile.

BUG=592715
==========

[stgao, 2016-03-07 20:28:43]

Description was changed from

==========
[Findit] Check existence of given targets before running compile.

BUG=592715
==========

to

==========
[Findit] Check existence of given targets before running compile.

BUG=592715, 560991
==========

[stgao, 2016-03-07 21:30:26]

Patchset #1 (id:1) has been deleted

[stgao, 2016-03-07 22:54:47]

Patchset #1 (id:20001) has been deleted

[stgao, 2016-03-07 23:04:38]

Description was changed from

==========
[Findit] Check existence of given targets before running compile.

BUG=592715, 560991
==========

to

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

Use 'ninja -t query' to test whether a "target" exists or not.
A "target" here is a node in the ninja build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

BUG=592715, 560991
==========

[stgao, 2016-03-07 23:04:48]

Description was changed from

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

Use 'ninja -t query' to test whether a "target" exists or not.
A "target" here is a node in the ninja build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

BUG=592715, 560991
==========

to

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

Use 'ninja -t query' to test whether a "target" exists or not.

A "target" here is a node in the ninja build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

BUG=592715, 560991
==========

[stgao, 2016-03-07 23:05:13]

Description was changed from

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

Use 'ninja -t query' to test whether a "target" exists or not.

A "target" here is a node in the ninja build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

BUG=592715, 560991
==========

to

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

Use 'ninja -t query' to test whether a "target" exists or not.

A "target" here is actually a node in ninja's build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

BUG=592715, 560991
==========

[stgao, 2016-03-07 23:10:22]

Patchset #1 (id:40001) has been deleted

[stgao, 2016-03-07 23:10:29]

Patchset #1 (id:60001) has been deleted

[stgao, 2016-03-07 23:14:40]

stgao@chromium.org changed reviewers:
+ agable@chromium.org, iannucci@chromium.org, thakis@chromium.org

[stgao, 2016-03-07 23:14:40]

Hi forks, would you mind helping review this CL?
You may check http://crbug.com/592715 for the background.

thakis@: stuff related to ninja, namely scripts/slave/check_target_existence.py
and scripts/slave/recipe_modules/findit/api.py

agable@, iannucci@: stuff related to recipe.

[stgao, 2016-03-08 01:06:15]

Description was changed from

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

Use 'ninja -t query' to test whether a "target" exists or not.

A "target" here is actually a node in ninja's build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

BUG=592715, 560991
==========

to

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

A "target" here is actually a node in ninja's build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

Because a "target" could be added or deleted by some commit in the regression
range, we have to check whether it still exists before re-running compile at a
different commit. Otherwise, compile will always fail.
Use 'ninja -t query TARGET' to test whether a "target" exists or not: ninja
exits with 0 if the target exists; otherwise with 1.


BUG=592715, 560991
==========

[Nico, 2016-03-08 19:41:37]

My two files lgtm

https://codereview.chromium.org/1766863002/diff/80001/scripts/slave/check_tar...
File scripts/slave/check_target_existence.py (right):

https://codereview.chromium.org/1766863002/diff/80001/scripts/slave/check_tar...
scripts/slave/check_target_existence.py:28: cmd = ['ninja', '-C',
target_build_dir, '-t', 'query']
(i'm assuming you checked that this does the right thing with multiple targets)

https://codereview.chromium.org/1766863002/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/findit/api.py (right):

https://codereview.chromium.org/1766863002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/findit/api.py:102: name='generate_build_files')
I suppose this assumes we don't need this with gyp? Will this do the right thing
(which I guess is "don't run this function there"?) on bots that still use gyp

https://codereview.chromium.org/1766863002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/findit/api.py:106:
args.extend(['--target-build-dir', self.m.chromium.output_dir])
nit: replace previous two lines with

  args = ['--target-build-dir', self.m.chromium.output_dir]

https://codereview.chromium.org/1766863002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/findit/api.py:115: return step.json.output['found']
If you don't need the not_found output, why write it?

[iannucci, 2016-03-09 19:01:52]

recipe stuff lgtm, though we should move the new script file into the module as
I describe.

https://chromiumcodereview.appspot.com/1766863002/diff/80001/scripts/slave/ch...
File scripts/slave/check_target_existence.py (right):

https://chromiumcodereview.appspot.com/1766863002/diff/80001/scripts/slave/ch...
scripts/slave/check_target_existence.py:1: #!/usr/bin/env python
let's add this under the findit module in the 'resources' subdirectory. Then you
can access it in the findit module by doing
self.resource('check_target_existence.py')

[stgao, 2016-03-09 20:09:12]

Comments are addressed.

https://chromiumcodereview.appspot.com/1766863002/diff/80001/scripts/slave/ch...
File scripts/slave/check_target_existence.py (right):

https://chromiumcodereview.appspot.com/1766863002/diff/80001/scripts/slave/ch...
scripts/slave/check_target_existence.py:1: #!/usr/bin/env python
On 2016/03/09 19:01:52, iannucci wrote:
> let's add this under the findit module in the 'resources' subdirectory. Then
you
> can access it in the findit module by doing
> self.resource('check_target_existence.py')

Nice! I didn't know about self.resource.

https://chromiumcodereview.appspot.com/1766863002/diff/80001/scripts/slave/ch...
scripts/slave/check_target_existence.py:28: cmd = ['ninja', '-C',
target_build_dir, '-t', 'query']
On 2016/03/08 19:41:36, Nico wrote:
> (i'm assuming you checked that this does the right thing with multiple
targets)

Yes, it works as the code expected here. I checked the ninja source code and
also tested with the ninja binary.
If all targets exist, ninja will exit with 0; otherwise ninja will complain upon
the first none-existing target and exit with 1 immediately.

https://chromiumcodereview.appspot.com/1766863002/diff/80001/scripts/slave/re...
File scripts/slave/recipe_modules/findit/api.py (right):

https://chromiumcodereview.appspot.com/1766863002/diff/80001/scripts/slave/re...
scripts/slave/recipe_modules/findit/api.py:102: name='generate_build_files')
On 2016/03/08 19:41:36, Nico wrote:
> I suppose this assumes we don't need this with gyp? Will this do the right
thing
> (which I guess is "don't run this function there"?) on bots that still use gyp

I think so. If the project generator tool is gyp, src/build/gyp_chromium is run
by runhooks as part of the code checkout,
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re...
.

I will double check with Dirk on this.

https://chromiumcodereview.appspot.com/1766863002/diff/80001/scripts/slave/re...
scripts/slave/recipe_modules/findit/api.py:106:
args.extend(['--target-build-dir', self.m.chromium.output_dir])
On 2016/03/08 19:41:36, Nico wrote:
> nit: replace previous two lines with
> 
>   args = ['--target-build-dir', self.m.chromium.output_dir]

Done.

https://chromiumcodereview.appspot.com/1766863002/diff/80001/scripts/slave/re...
scripts/slave/recipe_modules/findit/api.py:115: return step.json.output['found']
On 2016/03/08 19:41:36, Nico wrote:
> If you don't need the not_found output, why write it?

Removed.
Original idea is to keep them for completeness as the script is added to a
shared location.

[stgao, 2016-03-09 20:13:43]

Description was changed from

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

A "target" here is actually a node in ninja's build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

Because a "target" could be added or deleted by some commit in the regression
range, we have to check whether it still exists before re-running compile at a
different commit. Otherwise, compile will always fail.
Use 'ninja -t query TARGET' to test whether a "target" exists or not: ninja
exits with 0 if the target exists; otherwise with 1.


BUG=592715, 560991
==========

to

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

A "target" here is actually a node in ninja's build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

Because a "target" could be added or deleted by some commit in the regression
range, we have to check whether it still exists before re-running compile at a
different commit. Otherwise, compile will always fail.
Use 'ninja -t query TARGET1 [TARGET2 ...]' to test whether "target"s exist or
not: ninja exits with 0 if all the targets exist; otherwise with 1.


BUG=592715, 560991
==========

[iannucci, 2016-03-09 22:09:14]

lgtm

[stgao, 2016-03-10 01:31:43]

The CQ bit was checked by stgao@chromium.org

[stgao, 2016-03-10 01:31:43]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/1766863002/#ps140001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-03-10 01:31:53]

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

[commit-bot: I haz the power, 2016-03-10 01:35:54]

Description was changed from

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

A "target" here is actually a node in ninja's build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

Because a "target" could be added or deleted by some commit in the regression
range, we have to check whether it still exists before re-running compile at a
different commit. Otherwise, compile will always fail.
Use 'ninja -t query TARGET1 [TARGET2 ...]' to test whether "target"s exist or
not: ninja exits with 0 if all the targets exist; otherwise with 1.


BUG=592715, 560991
==========

to

==========
[Findit] Check existence of given targets before running compile in recipe
findit/chromium/compile.

A "target" here is actually a node in ninja's build graph, eg:
1. An executable target like browser_tests
2. An object file like obj/path/to/Source.o
3. An action like build/linux:gio_loader
4. An generated header file like gen/library_loaders/libgio.h
5. and so on

Because a "target" could be added or deleted by some commit in the regression
range, we have to check whether it still exists before re-running compile at a
different commit. Otherwise, compile will always fail.
Use 'ninja -t query TARGET1 [TARGET2 ...]' to test whether "target"s exist or
not: ninja exits with 0 if all the targets exist; otherwise with 1.


BUG=592715, 560991

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299206
==========

[commit-bot: I haz the power, 2016-03-10 01:35:55]

Committed patchset #4 (id:140001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=299206