Make the DEPS parsing saner.

Move DEPS parsing into a single function. This is a move towards having each DEPS entry being a Dependency instance.

TEST=new revinfo unit tests

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

[M-A Ruel, 2010-06-16 20:10:48]

It's another step towards http://codereview.chromium.org/2627007. Albeit this
review starts to become much more readable with the incremental changes I've
done.

[M-A Ruel, 2010-06-18 13:52:54]

Ping? I really want the whole thing from http://codereview.chromium.org/2627007
to get in ASAP.

[Mandeep Singh Baines, 2010-06-18 14:44:48]

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

http://codereview.chromium.org/2839008/diff/1/2#newcode185
gclient.py:185: def ParseDepsFile(self, direct_reference):
What is the From keyword?

http://codereview.chromium.org/2839008/diff/1/2#newcode193
gclient.py:193: self.direct_reference = direct_reference
All of the code above here can be re-written as:

self.direct_reference = direct_reference
if self.deps_parsed:
  return

http://codereview.chromium.org/2839008/diff/1/2#newcode200
gclient.py:200: if not deps_content:
Not sure this is necessary. Code should work fine without this code. Nice to
remove special cases.

http://codereview.chromium.org/2839008/diff/1/2#newcode241
gclient.py:241: if self.use_relative_paths:
From the way self.use_relative_paths is used, I'm not sure it needs to be a
member variable. Why can't it just be a local.

[M-A Ruel, 2010-06-18 14:58:02]

ptal

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

http://codereview.chromium.org/2839008/diff/1/2#newcode185
gclient.py:185: def ParseDepsFile(self, direct_reference):
On 2010/06/18 14:44:49, Mandeep Singh Baines wrote:
> What is the From keyword?

It permits to fetch a dependency defined in another DEPS file without fetching
all the dependencies from it.

http://codereview.chromium.org/2839008/diff/1/2#newcode193
gclient.py:193: self.direct_reference = direct_reference
On 2010/06/18 14:44:49, Mandeep Singh Baines wrote:
> All of the code above here can be re-written as:
> 
> self.direct_reference = direct_reference
> if self.deps_parsed:
>   return

Not exactly, as if self.direct_reference was True, it must never switch back to
false.

In the current code it doesn't make sense since direct_reference is not used
yet. :(  but that's necessary once every DEPS entry is a Dependency instance, in
that case, we don't want the From() keyword to 'activate' all the dependencies
in it.

I agree the conditions were ugly, I made it cleaner.

http://codereview.chromium.org/2839008/diff/1/2#newcode200
gclient.py:200: if not deps_content:
On 2010/06/18 14:44:49, Mandeep Singh Baines wrote:
> Not sure this is necessary. Code should work fine without this code. Nice to
> remove special cases.

done

http://codereview.chromium.org/2839008/diff/1/2#newcode241
gclient.py:241: if self.use_relative_paths:
On 2010/06/18 14:44:49, Mandeep Singh Baines wrote:
> From the way self.use_relative_paths is used, I'm not sure it needs to be a
> member variable. Why can't it just be a local.

done

[Mandeep Singh Baines, 2010-06-18 15:58:05]

LGTM

[M-A Ruel, 2010-06-21 18:45:57]

I know this is bad hygiene but I'm reusing this review so you can easily see the
differences between the prior review and this new bug fixed change w.r.t. From()
keyword. Enabled From() smoke testing and a bit more of revinfo testing.

[Nasser Grainawi, 2010-06-21 18:49:43]

lgtm

[M-A Ruel, 2010-06-21 21:00:22]

Bad hygiene take two. :/ Please see the diff between patchset 3 and 4

Until full recursive dependency tree is done, DEPS file needs to be parsed every
time the deps are needed.

[M-A Ruel, 2010-06-23 14:02:01]

On 2010/06/21 21:00:22, Marc-Antoine Ruel wrote:
> Bad hygiene take two. :/ Please see the diff between patchset 3 and 4
> 
> Until full recursive dependency tree is done, DEPS file needs to be parsed
every
> time the deps are needed.

Ping?

[Nasser Grainawi, 2010-06-23 19:12:47]

On 2010/06/23 14:02:01, Marc-Antoine Ruel wrote:
> On 2010/06/21 21:00:22, Marc-Antoine Ruel wrote:
> > Bad hygiene take two. :/ Please see the diff between patchset 3 and 4
> > 
> > Until full recursive dependency tree is done, DEPS file needs to be parsed
> every
> > time the deps are needed.
> 
> Ping?

So you changed it to always re-parse the DEPS file if ParseAllDeps is called? I
can't say I fully understand the impacts of this change, but it lgtm.

[M-A Ruel, 2010-06-23 19:14:02]

On 2010/06/23 19:12:47, Nasser Grainawi wrote:
> So you changed it to always re-parse the DEPS file if ParseAllDeps is called?
I
> can't say I fully understand the impacts of this change, but it lgtm.

Yes this is necessary with the From() keyword. It is a stop gap measure though
and will be removed in a later change.