Add Dependency.requirements which will solve the issues found in pagespeed and parallel checkout.

Add Dependency.requirements which will solve the issues found in pagespeed and parallel checkout.

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

[M-A Ruel, 2010-08-10 19:11:59]

Here's a sample of gclient revinfo -v -v -v on pagespeed. That is exactly what
was needed for  parallel checkout.
(...)
    name: src/testing
    url: http://src.chromium.org/svn/trunk/src/testing@55218
    parsed_url: http://src.chromium.org/svn/trunk/src/testing@55218
    processed: True
    requirements: ['src']

    name: src/testing/gtest
    url: From('src/third_party/chromium_deps', None)
    parsed_url: http://googletest.googlecode.com/svn/trunk@408
    processed: True
    requirements: ['src', 'src/third_party/chromium_deps', 'src/testing']
(...)

I have a full CL implementing the whole fix for pagespeed but it's fairly large
so I'm splitting it up a bit.

[piman, 2010-08-10 19:59:00]

http://codereview.chromium.org/3159002/diff/2001/3001
File gclient.py (right):

http://codereview.chromium.org/3159002/diff/2001/3001#newcode378
gclient.py:378: if s.name.startswith(s2.name):
startswith will yield false positives, e.g. src/foobar will depend on src/foo
when it shouldn't be. Though you can probably work around it by adding
os.path.sep. Are names normalized ? you'll want to do that if not.

[M-A Ruel, 2010-08-10 20:03:59]

updated

http://codereview.chromium.org/3159002/diff/2001/3001
File gclient.py (right):

http://codereview.chromium.org/3159002/diff/2001/3001#newcode378
gclient.py:378: if s.name.startswith(s2.name):
On 2010/08/10 19:59:00, piman wrote:
> startswith will yield false positives, e.g. src/foobar will depend on src/foo
> when it shouldn't be. Though you can probably work around it by adding
> os.path.sep. Are names normalized ? you'll want to do that if not.

Nice catch. Since the name are posix style, I'm joining it with '' so a trailing
slash is always present. It will remove the false positive.

[piman, 2010-08-10 20:09:20]

LGTM

[bradn, 2010-08-10 20:21:02]

http://codereview.chromium.org/3159002/diff/7001/8001
File gclient.py (right):

http://codereview.chromium.org/3159002/diff/7001/8001#newcode374
gclient.py:374: # src/foo. Yes, it's O(n^2)...
Wait so, do I understand correctly that a sort won't work here because its
ordering is too strict (there recursive deps targets that have to come first
because you don't know about the repos they pull in?) Wanna make sure I
understand.

[M-A Ruel, 2010-08-10 20:28:45]

http://codereview.chromium.org/3159002/diff/7001/8001
File gclient.py (right):

http://codereview.chromium.org/3159002/diff/7001/8001#newcode374
gclient.py:374: # src/foo. Yes, it's O(n^2)...
On 2010/08/10 20:21:02, bradn wrote:
> Wait so, do I understand correctly that a sort won't work here because its
> ordering is too strict (there recursive deps targets that have to come first
> because you don't know about the repos they pull in?) Wanna make sure I
> understand.

Yes. In pagespeed, they have something like that:

deps = {
 'src/third_party/chromium_deps: File('...chrome/trunk/src/DEPS'),
 'src/googleurl': From('src/chromium_deps')
}

That just doesn't work when the list is sorted. But just not sorting doesn't
work either, webkit.org's DEPS has other similar issues, hence the exact list of
requirements.

More details at:
http://code.google.com/p/page-speed/source/browse/lib/trunk/src/DEPS

[bradn, 2010-08-10 22:31:10]

LGTM