Enforces using cwd in all svn calls.

Enforces using cwd in all svn calls.

That helps weed out some issues faces with svn plus helped me figure out some
misuses.

Most of the commands have been implicitly depending on os.getcwd(). This change
makes it always consistent and clear when dependence on the current directory is
needed.

Remove default arguments to scm.SVN.GenerateDiff and a few other calls to be
sure the refactoring was done right.

R=dpranke@chromium.org
BUG=
TEST=make sure most commands aren't broke


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

[M-A Ruel, 2011-12-02 19:07:26]

The patch is slightly heavier that I desired but it degenerated quite quickly.
Basically, it helps disambiguating between urls and directories by enforcing
both parameters.

[M-A Ruel, 2011-12-02 19:08:13]

On 2011/12/02 19:07:26, Marc-Antoine Ruel wrote:
> The patch is slightly heavier that I desired but it degenerated quite quickly.
> Basically, it helps disambiguating between urls and directories by enforcing
> both parameters.

And I'm pretty it'll break something but I caught many refactoring errors while
fixing the tests failures, yay for tests.

[Dirk Pranke, 2011-12-03 03:20:16]

http://codereview.chromium.org/8771042/diff/1/gcl.py
File gcl.py (right):

http://codereview.chromium.org/8771042/diff/1/gcl.py#newcode739
gcl.py:739: return SVN.GenerateDiff(files, GetRepositoryRoot(), False, None)
Nit: can you pass the last two parameters here by name to make the call more
readable?

http://codereview.chromium.org/8771042/diff/1/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/8771042/diff/1/gclient_scm.py#newcode716
gclient_scm.py:716: os.path.join(self.checkout_path, '.'))
isn't os.path.join(self.checkout_path, '.') just self.checkout_path? (or at
least normpath(self.checkout_path) ?

http://codereview.chromium.org/8771042/diff/1/gclient_scm.py#newcode839
gclient_scm.py:839: from_info_live = scm.SVN.CaptureInfo(from_info['URL'], None)
Seems like CaptureInfo() is usually either from a URL w/ None or from a local
dir with th e URL being None. I'd probably split this into something like
CaptureLocalInfo() and CaptureRemoteInfo() to make the intent clearer (or pass
named parameters); otherwise it seems easy to screw up and pass None for the
wrong arg.

[M-A Ruel, 2011-12-05 15:23:49]

http://codereview.chromium.org/8771042/diff/1/gcl.py
File gcl.py (right):

http://codereview.chromium.org/8771042/diff/1/gcl.py#newcode739
gcl.py:739: return SVN.GenerateDiff(files, GetRepositoryRoot(), False, None)
On 2011/12/03 03:20:16, Dirk Pranke wrote:
> Nit: can you pass the last two parameters here by name to make the call more
> readable?

done

http://codereview.chromium.org/8771042/diff/1/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/8771042/diff/1/gclient_scm.py#newcode716
gclient_scm.py:716: os.path.join(self.checkout_path, '.'))
On 2011/12/03 03:20:16, Dirk Pranke wrote:
> isn't os.path.join(self.checkout_path, '.') just self.checkout_path? (or at
> least normpath(self.checkout_path) ?

I recall I did it on purpose but I forgot why exactly, and forgot to put a
comment there. :/

It had to do with the way it processes directories and if it looks at the
containing directory first. svn's behavior is a tad annoying with that. In
theory using cwd should alleviate that but I don't want to change the behavior
in this CL since it's already risky enough.

http://codereview.chromium.org/8771042/diff/1/gclient_scm.py#newcode839
gclient_scm.py:839: from_info_live = scm.SVN.CaptureInfo(from_info['URL'], None)
On 2011/12/03 03:20:16, Dirk Pranke wrote:
> Seems like CaptureInfo() is usually either from a URL w/ None or from a local
> dir with th e URL being None. I'd probably split this into something like
> CaptureLocalInfo() and CaptureRemoteInfo() to make the intent clearer (or pass
> named parameters); otherwise it seems easy to screw up and pass None for the
> wrong arg.

done, good idea.

[M-A Ruel, 2011-12-12 20:39:54]

ping?

[Dirk Pranke, 2011-12-13 02:56:55]

lgtm. sorry, this fell through the cracks for me.

[commit-bot: I haz the power, 2011-12-13 20:28:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/8771042/7001

[commit-bot: I haz the power, 2011-12-13 20:36:13]

Change committed as 114262

[Nico, 2011-12-13 22:27:40]

This broke `gcl change` for me. Is sent you an email with details, maruel.

[achuithb, 2011-12-14 11:22:47]

On 2011/12/13 22:27:40, Nico wrote:
> This broke `gcl change` for me. Is sent you an email with details, maruel.

Yup, gcl change is broken.

[achuithb, 2011-12-14 11:32:51]

gcl change no longer recognizes that files are in other changelists. I cannot
add files to a changelist either.

svn up -r 114261 in depot_tools is a temporary fix for others who are stuck.
drover --revert 114262 doesn't work.

[M-A Ruel, 2011-12-14 13:37:46]

On 2011/12/14 11:32:51, achuith.bhandarkar wrote:
> gcl change no longer recognizes that files are in other changelists. I cannot
> add files to a changelist either.
> 
> svn up -r 114261 in depot_tools is a temporary fix for others who are stuck.
> drover --revert 114262 doesn't work.

It was fixed a few _minutes_ later in http://codereview.chromium.org/8937010

[Nico, 2011-12-14 18:08:37]

On Wed, Dec 14, 2011 at 5:37 AM,  <maruel@chromium.org> wrote:
> It was fixed a few _minutes_ later in http://codereview.chromium.org/8937010

It looks like it's still broken for me over here. I ran `gclient`,
which should autoupdate gcl as far as I understand.

[achuithb, 2011-12-15 02:13:33]

On 2011/12/14 18:08:37, Nico wrote:
> On Wed, Dec 14, 2011 at 5:37 AM,  <mailto:maruel@chromium.org> wrote:
> > It was fixed a few _minutes_ later in http://codereview.chromium.org/8937010
> 
> It looks like it's still broken for me over here. I ran `gclient`,
> which should autoupdate gcl as far as I understand.

It is still broken:

achuith@achuithz600:~/code/chromium/src/chrome/browser/ui/webui$ gcl change
mouse
Loaded authentication cookies from /home/achuith/.codereview_upload_cookies
mouse changelist saved.
Got an exception
Command svn info --xml
chrome/browser/ui/webui/chrome/browser/ui/webui/options2/chromeos/system_options_handler.cc
returned non-zero exit status 1 in
/usr/local/google/users/achuith/code/chromium/src/
<?xml version="1.0"?>
<info>

svn: 'chrome/browser/ui/webui/chrome/browser/ui/webui/options2/chromeos' is not
a working copy
svn: 'chrome/browser/ui/webui/chrome/browser/ui/webui/options2/chromeos' does
not exist

achuith@achuithz600:~/code/chromium/src/chrome/browser/ui/webui$ svn info
options2/chromeos/
Path: options2/chromeos
URL: svn://chrome-svn/chrome/trunk/src/chrome/browser/ui/webui/options2/chromeos
Repository Root: svn://chrome-svn/chrome
Repository UUID: 0039d316-1c4b-4281-b951-d872f2087c98
Revision: 114508
Node Kind: directory
Schedule: normal
Last Changed Author: jhawkins@chromium.org
Last Changed Rev: 114493
Last Changed Date: 2011-12-14 13:27:58 -0800 (Wed, 14 Dec 2011)

gcl change works if you're at the root of the tree (ie, src/), but not if you're
in a sub-folder.

[M-A Ruel, 2011-12-15 02:18:14]

cd to the root directory in the meantime.


Le 14 décembre 2011 21:13, <achuith@chromium.org> a écrit :

>
http://codereview.chromium.**org/8771042/<http://codereview.chromium.org/8771...
>

[M-A Ruel, 2011-12-15 02:18:23]

Or stop using gcl, seriously.

Le 14 décembre 2011 21:17, Marc-Antoine Ruel <maruel@chromium.org> a écrit :

> cd to the root directory in the meantime.
>
>
> Le 14 décembre 2011 21:13, <achuith@chromium.org> a écrit :
>
>>
http://codereview.chromium.**org/8771042/<http://codereview.chromium.org/8771...
>>
>
>

[achuithb, 2011-12-15 02:21:36]

On 2011/12/15 02:18:23, Marc-Antoine Ruel wrote:
> Or stop using gcl, seriously.
> 
> Le 14 décembre 2011 21:17, Marc-Antoine Ruel <mailto:maruel@chromium.org> a
écrit :
> 
> > cd to the root directory in the meantime.
> >
> >
> > Le 14 décembre 2011 21:13, <mailto:achuith@chromium.org> a écrit :
> >
> >>
>
http://codereview.chromium.**org/8771042/%3Chttp://codereview.chromium.org/87...>
> >>
> >
> >

What should I be using? git?

[M-A Ruel, 2011-12-15 02:34:49]

Yep. :)

But I'll fix the bug, I just got a P0 to fix first.

Le 14 décembre 2011 21:21, <achuith@chromium.org> a écrit :

> On 2011/12/15 02:18:23, Marc-Antoine Ruel wrote:
>
>> Or stop using gcl, seriously.
>>
>
>  Le 14 décembre 2011 21:17, Marc-Antoine Ruel <mailto:maruel@chromium.org>
>> a
>>
> écrit :
>
>  > cd to the root directory in the meantime.
>> >
>> >
>> > Le 14 décembre 2011 21:13, <mailto:achuith@chromium.org> a écrit :
>> >
>> >>
>>
>
> http://codereview.chromium.****org/8771042/%3Chttp://coderevi**
> ew.chromium.org/8771042/ <http://codereview.chromium.org/8771042/>>
>
>> >>
>> >
>> >
>>
>
> What should I be using? git?
>
>
http://codereview.chromium.**org/8771042/<http://codereview.chromium.org/8771...
>