RFC: ignore compile_targets returned by analyze

Paweł Hajdan, Jr.

unread,

Oct 20, 2015, 8:32:42 AM10/20/15

to infr...@chromium.org

Please see https://code.google.com/p/chromium/issues/detail?id=544571 . Looks like a CL (https://codereview.chromium.org/1408263002/) passed CQ, but then broke the trybots (e.g. http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/111468).

My understanding is the second tryjob added targets to compile_targets that were never requested by the recipe, and so not tested on the original CL that triggered breakage.

It seems to me https://codereview.chromium.org/1402813002 would fix it, by effectively making compile_targets the same as matching_exes. Is there any reason why it's not landed yet?

I also uploaded https://codereview.chromium.org/1419643002 as a proof of concept - does it make sense to you?

Paweł

Dirk Pranke

unread,

Oct 20, 2015, 4:07:56 PM10/20/15

to Paweł Hajdan, Jr., infr...@chromium.org, Scott Violet, John Abd-El-Malek

On Tue, Oct 20, 2015 at 5:32 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

Please see https://code.google.com/p/chromium/issues/detail?id=544571 . Looks like a CL (https://codereview.chromium.org/1408263002/) passed CQ, but then broke the trybots (e.g. http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/111468).

My understanding is the second tryjob added targets to compile_targets that were never requested by the recipe, and so not tested on the original CL that triggered breakage.

It seems to me https://codereview.chromium.org/1402813002 would fix it, by effectively making compile_targets the same as matching_exes. Is there any reason why it's not landed yet?

We just hadn't gotten to it. I think we should land this today.

I also uploaded https://codereview.chromium.org/1419643002 as a proof of concept - does it make sense to you?

I would think that instead of this change we should just delete 'compile_targets' and only look at matching_exes. I'll follow up on the CL.

-- Dirk

Paweł Hajdan, Jr.

unread,

Oct 29, 2015, 10:29:39 AM10/29/15

to Dirk Pranke, infr...@chromium.org, Scott Violet, John Abd-El-Malek

Okay. Can we also stop both GYP and GN "analyze" from taking two lists of targets as input?

Just wanted to coordinate with you before making changes: should we standardize on "targets" or "build_targets"? Which should be removed, and which should be kept? My suggestion would be to use "targets" - it's shorter, and more generic. "build_targets" might imply little bit too much.

WDYT?

Paweł

Dirk Pranke

unread,

Oct 29, 2015, 5:21:45 PM10/29/15

to Paweł Hajdan, Jr., infr...@chromium.org, Scott Violet, John Abd-El-Malek

On Thu, Oct 29, 2015 at 7:29 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

On Tue, Oct 20, 2015 at 10:07 PM, Dirk Pranke <dpr...@chromium.org> wrote:
On Tue, Oct 20, 2015 at 5:32 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:
I also uploaded https://codereview.chromium.org/1419643002 as a proof of concept - does it make sense to you?

I would think that instead of this change we should just delete 'compile_targets' and only look at matching_exes. I'll follow up on the CL.

Okay. Can we also stop both GYP and GN "analyze" from taking two lists of targets as input?

Sure.

Just wanted to coordinate with you before making changes: should we standardize on "targets" or "build_targets"? Which should be removed, and which should be kept? My suggestion would be to use "targets" - it's shorter, and more generic. "build_targets" might imply little bit too much.