GTTF: Print much better error message for dependency cycles.

GTTF: Print much better error message for dependency cycles.

Example:

gyp: Cycles in .gyp file dependency graph detected:
Cycle: build/linux/system.gyp -> third_party/zlib/zlib.gyp -> base/base.gyp -> tools/xdisplaycheck/xdisplaycheck.gyp -> build/linux/system.gyp
Cycle: build/linux/system.gyp -> third_party/zlib/zlib.gyp -> base/base.gyp -> build/linux/system.gyp
Cycle: base/base.gyp -> tools/xdisplaycheck/xdisplaycheck.gyp -> build/linux/system.gyp -> base/base.gyp
Cycle: build/linux/system.gyp -> base/base.gyp -> tools/xdisplaycheck/xdisplaycheck.gyp -> build/linux/system.gyp
Cycle: base/base.gyp -> build/linux/system.gyp -> base/base.gyp
Cycle: base/base.gyp -> tools/xdisplaycheck/xdisplaycheck.gyp -> build/linux/system.gyp -> third_party/zlib/zlib.gyp -> base/base.gyp
Cycle: base/base.gyp -> build/linux/system.gyp -> third_party/zlib/zlib.gyp -> base/base.gyp
Cycle: build/linux/system.gyp -> base/base.gyp -> build/linux/system.gyp

BUG=chromium:35878
Patch by Paweł Hajdan Jr. <phajdan.jr@chromium.org>;
Take 2, originally landed in r1695 and backed out in r1696.
Review URL: https://codereview.chromium.org/23018008/

[scottmg, 2013-08-16 03:07:40]

Nice! The old output is very unhelpful.

[Mark Mentovai, 2013-08-16 04:25:48]

Is there an easy way to test the cycle-finding logic?

[Paweł Hajdan Jr., 2013-08-16 19:26:35]

PTAL

[Mark Mentovai, 2013-08-16 19:32:46]

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input.py
File pylib/gyp/input.py (right):

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input.py#newcode...
pylib/gyp/input.py:1751: cycles.append('Cycle #%d: %s' % (num, ' ->
'.join(simplified_paths)))
I don’t think you really need to print |num|.

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input_test.py
File pylib/gyp/input_test.py (right):

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input_test.py#ne...
pylib/gyp/input_test.py:1: #!/usr/bin/env python
Excellent!

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input_test.py#ne...
pylib/gyp/input_test.py:66: self.assertEquals([(self.nodes['a'],
self.nodes['b'], self.nodes['a']),
The relative ordering of the two cycles is an implementation detail. For this
test, you should call FindCycles and then make sure that it returns a list of
length 2 and that each of the expected cycles is present, but not require a
strict order.

[Paweł Hajdan Jr., 2013-08-16 22:39:41]

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input.py
File pylib/gyp/input.py (right):

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input.py#newcode...
pylib/gyp/input.py:1751: cycles.append('Cycle #%d: %s' % (num, ' ->
'.join(simplified_paths)))
On 2013/08/16 19:32:46, Mark Mentovai wrote:
> I don’t think you really need to print |num|.

I think it can be useful for long cycles, to make it really clear where one
cycle begins and ends.

I can remove it if it's a really strong opinion.

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input_test.py
File pylib/gyp/input_test.py (right):

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input_test.py#ne...
pylib/gyp/input_test.py:66: self.assertEquals([(self.nodes['a'],
self.nodes['b'], self.nodes['a']),
On 2013/08/16 19:32:46, Mark Mentovai wrote:
> The relative ordering of the two cycles is an implementation detail. For this
> test, you should call FindCycles and then make sure that it returns a list of
> length 2 and that each of the expected cycles is present, but not require a
> strict order.

Done.

[Mark Mentovai, 2013-08-17 03:59:02]

LGTM otherwise

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input.py
File pylib/gyp/input.py (right):

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input.py#newcode...
pylib/gyp/input.py:1751: cycles.append('Cycle #%d: %s' % (num, ' ->
'.join(simplified_paths)))
Paweł Hajdan Jr. wrote:
> On 2013/08/16 19:32:46, Mark Mentovai wrote:
> > I don’t think you really need to print |num|.
> 
> I think it can be useful for long cycles, to make it really clear where one
> cycle begins and ends.
> 
> I can remove it if it's a really strong opinion.

I think it kind of implies that there’s some ordering (which is why I asked you
to adjust the test to not expect an ordering).

If you wanted to indent the beginning of each cycle to make them stand out,
that’d be OK, but I think it’s probably already fine if it just says Cycle: at
the beginning of each cycle line.

[Paweł Hajdan Jr., 2013-08-19 18:59:49]

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input.py
File pylib/gyp/input.py (right):

https://codereview.chromium.org/23018008/diff/6001/pylib/gyp/input.py#newcode...
pylib/gyp/input.py:1751: cycles.append('Cycle #%d: %s' % (num, ' ->
'.join(simplified_paths)))
On 2013/08/17 03:59:02, Mark Mentovai wrote:
> Paweł Hajdan Jr. wrote:
> > On 2013/08/16 19:32:46, Mark Mentovai wrote:
> > > I don’t think you really need to print |num|.
> > 
> > I think it can be useful for long cycles, to make it really clear where one
> > cycle begins and ends.
> > 
> > I can remove it if it's a really strong opinion.
> 
> I think it kind of implies that there’s some ordering (which is why I asked
you
> to adjust the test to not expect an ordering).
> 
> If you wanted to indent the beginning of each cycle to make them stand out,
> that’d be OK, but I think it’s probably already fine if it just says Cycle: at
> the beginning of each cycle line.

Done.

[Mark Mentovai, 2013-08-19 19:01:20]

Adjust CL description and LGTM.

[Paweł Hajdan Jr., 2013-08-19 19:36:57]

On 2013/08/19 19:01:20, Mark Mentovai wrote:
> Adjust CL description and LGTM.

Done. Could you also land this? CQ doesn't seem to react to this patch (and also
my previous gyp patch).

[Mark Mentovai, 2013-08-19 19:57:57]

Sure. Landed GYP r1695.

[scottmg, 2013-08-19 21:31:06]

This has made http://build.chromium.org/f/client/gyp/ all red. Could one of you
take a look?

[Mark Mentovai, 2013-08-19 21:40:06]

Backed out at r1696. assertItemsEqual is new in Python 2.7.

http://docs.python.org/2/library/unittest.html#unittest.TestCase.assertItemsE...

[Paweł Hajdan Jr., 2013-08-19 22:22:45]

PTAL - now with trybots and a fix.

Please commit if looks good.

[Mark Mentovai, 2013-08-20 13:28:46]

LGTM. r1700.