Fix gclient sync 'dontswitchurl' url stickiness

Fix gclient sync 'dontswitchurl' url stickiness

._Capture throws exception on non-zero status, switch to
subprocess2 capture method. Switch magic value to dontswitchurl.

BUG=241907
R=dpranke@chromium.org, maruel@chromium.org

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

[Dirk Pranke, 2013-05-20 17:55:19]

https://chromiumcodereview.appspot.com/15325003/diff/1/gclient_scm.py
File gclient_scm.py (right):

https://chromiumcodereview.appspot.com/15325003/diff/1/gclient_scm.py#newcode323
gclient_scm.py:323: cwd=self.checkout_path).strip() != 'dontswitchurl'):
I would prefer that we have a comment here about what we're doing and what
'dontswitchurl' is for.

[M-A Ruel, 2013-05-20 20:37:43]

I'd prefer Robbie to decide.

But as I said in the previous CL, I'd prefer configuration line with more
explicit names, like
git config remote.origin.gclient-auto-fix-url False

(I just realized we should not use . or _ but - works just fine)

https://chromiumcodereview.appspot.com/15325003/diff/1/gclient_scm.py
File gclient_scm.py (right):

https://chromiumcodereview.appspot.com/15325003/diff/1/gclient_scm.py#newcode321
gclient_scm.py:321: if (current_url != url and url != 'git://foo' and
subprocess2.capture(
one condition per line, so that the line ends with and
(as per style guide)

[Dirk Pranke, 2013-05-20 20:41:52]

On 2013/05/20 20:37:43, Marc-Antoine Ruel wrote:
> I'd prefer Robbie to decide.
> 
> But as I said in the previous CL, I'd prefer configuration line with more
> explicit names, like
> git config remote.origin.gclient-auto-fix-url False
> 
> (I just realized we should not use . or _ but - works just fine)
> 
> https://chromiumcodereview.appspot.com/15325003/diff/1/gclient_scm.py
> File gclient_scm.py (right):
> 
>
https://chromiumcodereview.appspot.com/15325003/diff/1/gclient_scm.py#newcode321
> gclient_scm.py:321: if (current_url != url and url != 'git://foo' and
> subprocess2.capture(
> one condition per line, so that the line ends with and
> (as per style guide)

Hm. I agree that it would be better to be using a different setting as well.

[iannucci1, 2013-05-20 20:57:38]

On 2013/05/20 20:37:43, Marc-Antoine Ruel wrote:
> I'd prefer Robbie to decide.
> 

What am I deciding here?

+1 on using a boolean var with a descriptive name though.

[Isaac (away), 2013-05-21 07:41:17]

I believe made the requested changes (not 100% sure about style guide comment),
please take another look.

I would have preferred locking down the key/value before sending out a PSA
though...

[Dirk Pranke, 2013-05-22 00:05:15]

lgtm. thanks for making the changes.

https://chromiumcodereview.appspot.com/15325003/diff/8002/gclient_scm.py
File gclient_scm.py (right):

https://chromiumcodereview.appspot.com/15325003/diff/8002/gclient_scm.py#newc...
gclient_scm.py:868: stderr=subprocess2.VOID,
why change this from PIPE to VOID ?

[Isaac (away), 2013-05-22 00:12:25]

https://chromiumcodereview.appspot.com/15325003/diff/8002/gclient_scm.py
File gclient_scm.py (right):

https://chromiumcodereview.appspot.com/15325003/diff/8002/gclient_scm.py#newc...
gclient_scm.py:868: stderr=subprocess2.VOID,
On 2013/05/22 00:05:15, Dirk Pranke wrote:
> why change this from PIPE to VOID ?

This is an unrelated minor bug I spotted.
subprocess2.check_call_out returns (stdout, stderr)
subprocess2.check_output returns check_call_out[0]

So it is discarding stderr.

[commit-bot: I haz the power, 2013-05-22 00:12:42]

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

[commit-bot: I haz the power, 2013-05-22 00:14:33]

Change committed as 201397