Add recursion=n syntax to DEPS files.

Add recursion=n syntax to DEPS files.

This will let DEPS files specify a higher local bound for recursion.
Note there is an edge case where a dependency can be initially pulled
in with a lower recursion level and then gets ignored subsequently
when a lower level DEPS overrides it's inclusion level and then
includes the same dependency.  I do not solve this case, as
I am intending to use this syntax for top level deps files.

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

[M-A Ruel, 2012-10-16 14:15:00]

I'll require a proper unit test.

http://codereview.chromium.org/11146032/diff/1/gclient.py
File gclient.py (right):

http://codereview.chromium.org/11146032/diff/1/gclient.py#newcode174
gclient.py:174: self.recursion_override = None
Make it protected.

http://codereview.chromium.org/11146032/diff/1/gclient.py#newcode238
gclient.py:238: return self.recursion_override or
max(self.parent.recursion_limit - 1, 0)
It could be 0, which would be valid, so check if the value is None.

http://codereview.chromium.org/11146032/diff/1/gclient.py#newcode453
gclient.py:453: self.recursion_override = local_scope['recursion']
local_scope.get('recursion')

[Isaac (away), 2012-10-17 08:19:04]

Added unit test, ptal.

https://chromiumcodereview.appspot.com/11146032/diff/1/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/11146032/diff/1/gclient.py#newcode174
gclient.py:174: self.recursion_override = None
On 2012/10/16 14:15:00, Marc-Antoine Ruel wrote:
> Make it protected.

I set this in a different class.

https://chromiumcodereview.appspot.com/11146032/diff/1/gclient.py#newcode238
gclient.py:238: return self.recursion_override or
max(self.parent.recursion_limit - 1, 0)
On 2012/10/16 14:15:00, Marc-Antoine Ruel wrote:
> It could be 0, which would be valid, so check if the value is None.

Done.

https://chromiumcodereview.appspot.com/11146032/diff/1/gclient.py#newcode453
gclient.py:453: self.recursion_override = local_scope['recursion']
On 2012/10/16 14:15:00, Marc-Antoine Ruel wrote:
> local_scope.get('recursion')

Done.

[M-A Ruel, 2012-10-17 11:08:24]

Mostly looks good, just a small nit and a question.

lgtm

https://chromiumcodereview.appspot.com/11146032/diff/9002/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/11146032/diff/9002/gclient.py#newcode174
gclient.py:174: # This is currently only supported for top-level deps because
gclient does
Interesting note. I would have said it's just fine; when the same exact
dependency is fetched twice, it's creating two independent Dependency instances.

https://chromiumcodereview.appspot.com/11146032/diff/9002/gclient.py#newcode458
gclient.py:458: if 'recursion' in local_scope:
No need to "if" that. local_scope.get('recursion') returns None if the value
doesn't exist, and None means that the value is not overriden.

[Isaac (away), 2012-10-17 16:24:02]

https://chromiumcodereview.appspot.com/11146032/diff/9002/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/11146032/diff/9002/gclient.py#newcode174
gclient.py:174: # This is currently only supported for top-level deps because
gclient does
On 2012/10/17 11:08:25, Marc-Antoine Ruel wrote:
> Interesting note. I would have said it's just fine; when the same exact
> dependency is fetched twice, it's creating two independent Dependency
instances.

Here's an example of what I'm concerned about.  Maybe it will already work, not
sure!

.gclient{
dep1,
dep2,
}
dep1{deps={dep3}}
dep2{deps={dep3}, recursion=2}
dep3{deps={dep4}}

When we process .gclient -> dep1 -> dep3, we are at recursion limit, so do not
process dep4.  When we process .gclient -> dep2, we do not add dep3 a second
time.

dep4 should have be added but will not be.

https://chromiumcodereview.appspot.com/11146032/diff/9002/gclient.py#newcode458
gclient.py:458: if 'recursion' in local_scope:
On 2012/10/17 11:08:25, Marc-Antoine Ruel wrote:
> No need to "if" that. local_scope.get('recursion') returns None if the value
> doesn't exist, and None means that the value is not overriden.

I want the if for the logging line, though.

[M-A Ruel, 2012-10-17 16:57:34]

https://chromiumcodereview.appspot.com/11146032/diff/9002/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/11146032/diff/9002/gclient.py#newcode174
gclient.py:174: # This is currently only supported for top-level deps because
gclient does
On 2012/10/17 16:24:02, Isaac wrote:
> On 2012/10/17 11:08:25, Marc-Antoine Ruel wrote:
> > Interesting note. I would have said it's just fine; when the same exact
> > dependency is fetched twice, it's creating two independent Dependency
> instances.
> 
> Here's an example of what I'm concerned about.  Maybe it will already work,
not
> sure!
> 
> .gclient{
> dep1,
> dep2,
> }
> dep1{deps={dep3}}
> dep2{deps={dep3}, recursion=2}
> dep3{deps={dep4}}
> 
> When we process .gclient -> dep1 -> dep3, we are at recursion limit, so do not
> process dep4.  When we process .gclient -> dep2, we do not add dep3 a second
> time.
> 
> dep4 should have be added but will not be.

I'm pretty sure it's handled correctly. "deps3" is loaded two times in the graph
as separate instances. Technically I'd like to see this tested but I won't
require it.

still lgtm.

[Isaac (away), 2012-10-17 17:22:41]

Excellent!  I amended my unit test to cover that case and removed my todo.

[commit-bot: I haz the power, 2012-10-17 18:08:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ilevy@chromium.org/11146032/11003

[commit-bot: I haz the power, 2012-10-17 18:11:04]

Change committed as 162464