gclient: implement a less hacky relative URL implementation

gclient: implement a less hacky relative URL implementation

Doing something like basename when concatenating base_url with
relative url.

Added use_relative_urls2 to enable switching between old
implementation and new implementation. Once the old implementation
is deprecated we can remove the use_relative_urls2.

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

[djmm, 2009-12-15 05:59:21]

lgtm

On Mon, Dec 14, 2009 at 7:28 PM, <msb@chromium.org> wrote:

> Reviewers: Marc-Antoine Ruel, djmm,
>
> Description:
> gclient: implement a less hacky relative URL implementation
>
> Doing something like basename when concatenating base_url with
> relative url.
>
> Added use_relative_urls2 to enable switching between old
> implementation and new implementation. Once the old implementation
> is deprecated we can remove the use_relative_urls2.
>
> Please review this at http://codereview.chromium.org/500015
>
> Affected files:
>  M gclient.py
>  M gclient_scm.py
>  M gclient_utils.py
>  M tests/gclient_scm_test.py
>  M tests/gclient_test.py
>  M tests/gclient_utils_test.py
>
>
>

[M-A Ruel, 2009-12-15 18:49:42]

Sorry I'm a bit late, I'd like to have your opinion on my comment.

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

http://codereview.chromium.org/500015/diff/1/2#newcode521
gclient.py:521: local_scope = {}
I would have preferred to have this code moved into a function so it doesn't
duplicate line 441.

http://codereview.chromium.org/500015/diff/1/4
File gclient_utils.py (right):

http://codereview.chromium.org/500015/diff/1/4#newcode46
gclient_utils.py:46: def FullUrlFromRelative2(base_url, url):
Frankly, I would have prefered it to be SCM-dependent. The main issue is that
git cannot have sub-directory checkout so using this is *always* appropriate
while the reverse is not true for FullUrlFromRelative(). Hence I think this
should be moved back to gclient_scm.py

http://codereview.chromium.org/500015/diff/1/6
File tests/gclient_test.py (right):

http://codereview.chromium.org/500015/diff/1/6#newcode802
tests/gclient_test.py:802: '/trunk/deps/third_party/cygwin@3248'
80 cols

[Mandeep Singh Baines, 2009-12-15 20:54:30]

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

http://codereview.chromium.org/500015/diff/1/2#newcode521
gclient.py:521: local_scope = {}
On 2009/12/15 18:49:42, Marc-Antoine Ruel wrote:
> I would have preferred to have this code moved into a function so it doesn't
> duplicate line 441.

Good idea. I'll send out a CL to do this.

http://codereview.chromium.org/500015/diff/1/4
File gclient_utils.py (right):

http://codereview.chromium.org/500015/diff/1/4#newcode46
gclient_utils.py:46: def FullUrlFromRelative2(base_url, url):
On 2009/12/15 18:49:42, Marc-Antoine Ruel wrote:
> Frankly, I would have prefered it to be SCM-dependent. The main issue is that
> git cannot have sub-directory checkout so using this is *always* appropriate
> while the reverse is not true for FullUrlFromRelative(). Hence I think this
> should be moved back to gclient_scm.py

k, I'll send out a CL to do that. I don't want to revert this change as it will
break the ChromiumOS build.