Fix rebase-update on windows on git-svn repos.

Fix rebase-update on windows on git-svn repos.

Through a comedy of wrapper scripts, the ^ is being swallowed, which
causes the `git config` invocation to match all lines which HAVE
"remote" in them, instead of matching lines which BEGIN with remote.

R=maruel@chromium.org, pkasting@chromium.org
BUG=

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

[Peter Kasting, 2016-01-08 02:11:44]

I'm not a good reviewer on this.

But... wow.  LOL.

[iannucci, 2016-01-08 02:13:36]

On 2016/01/08 at 02:10:14, iannucci wrote:
> 

Note that other things in depot_tools have this bug (probably), but their
patterns look like `^svn-remote...`, which means that even when the ^ gets
silently dropped, the regex still works in practice, since there aren't any
known config keys which have 'svn-remote' in them that's not in the 0th column.

[iannucci, 2016-01-08 02:14:56]

https://codereview.chromium.org/1566343002/diff/1/git_common.py
File git_common.py (right):

https://codereview.chromium.org/1566343002/diff/1/git_common.py#newcode573
git_common.py:573: kwargs.setdefault('shell', False)
GIT_EXE is always an absolute path, so we don't need this layer of cheese. If
this is True, then we need 16 ^'s :(

[iannucci, 2016-01-08 20:54:01]

iannucci@chromium.org changed reviewers:
+ agable@chromium.org

[iannucci, 2016-01-08 20:54:01]

+agable?

[M-A Ruel, 2016-01-08 21:11:21]

lgtm, sorry for the delay, totally missed it.

https://codereview.chromium.org/1566343002/diff/1/git_common.py
File git_common.py (right):

https://codereview.chromium.org/1566343002/diff/1/git_common.py#newcode288
git_common.py:288: if sys.platform.startswith('win'):
if sys.platform == 'win32':

the format you used was because in 2008 python thought it was a good idea to put
win64 in their x64 build but they quickly backtracked.

[iannucci, 2016-01-08 21:28:12]

On 2016/01/08 at 21:11:21, maruel wrote:
> lgtm, sorry for the delay, totally missed it.
> 
> https://codereview.chromium.org/1566343002/diff/1/git_common.py
> File git_common.py (right):
> 
> https://codereview.chromium.org/1566343002/diff/1/git_common.py#newcode288
> git_common.py:288: if sys.platform.startswith('win'):
> if sys.platform == 'win32':
> 
> the format you used was because in 2008 python thought it was a good idea to
put win64 in their x64 build but they quickly backtracked.

I did the startswith to be self-consistent in this file. Worth changing both?

[M-A Ruel, 2016-01-08 21:30:09]

bof, as you want.

M-A
Le 8 janv. 2016 4:28 PM, <iannucci@chromium.org> a écrit :

> On 2016/01/08 at 21:11:21, maruel wrote:
>
>> lgtm, sorry for the delay, totally missed it.
>>
>
> https://codereview.chromium.org/1566343002/diff/1/git_common.py
>> File git_common.py (right):
>>
>
> https://codereview.chromium.org/1566343002/diff/1/git_common.py#newcode288
>> git_common.py:288: if sys.platform.startswith('win'):
>> if sys.platform == 'win32':
>>
>
> the format you used was because in 2008 python thought it was a good idea
>> to
>>
> put win64 in their x64 build but they quickly backtracked.
>
> I did the startswith to be self-consistent in this file. Worth changing
> both?
>
> https://chromiumcodereview.appspot.com/1566343002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[iannucci, 2016-01-08 21:33:37]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2016-01-08 21:33:37]

The patchset sent to the CQ was uploaded after l-g-t-m from maruel@chromium.org
Link to the patchset: https://codereview.chromium.org/1566343002/#ps20001
(title: "use ==")

[iannucci, 2016-01-08 21:33:42]

fixed both

[commit-bot: I haz the power, 2016-01-08 21:33:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1566343002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1566343002/20001

[commit-bot: I haz the power, 2016-01-08 23:41:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-08 23:41:27]

Try jobs failed on following builders:
  depot_tools_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/depot_tools_pre...)

[iannucci, 2016-01-09 08:06:50]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2016-01-09 08:06:50]

The patchset sent to the CQ was uploaded after l-g-t-m from maruel@chromium.org
Link to the patchset: https://codereview.chromium.org/1566343002/#ps40001
(title: "nocover")

[commit-bot: I haz the power, 2016-01-09 08:06:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1566343002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1566343002/40001

[commit-bot: I haz the power, 2016-01-09 08:08:41]

Description was changed from

==========
Fix rebase-update on windows on git-svn repos.

Through a comedy of wrapper scripts, the ^ is being swallowed, which
causes the `git config` invocation to match all lines which HAVE
"remote" in them, instead of matching lines which BEGIN with remote.

R=maruel@chromium.org, pkasting@chromium.org
BUG=
==========

to

==========
Fix rebase-update on windows on git-svn repos.

Through a comedy of wrapper scripts, the ^ is being swallowed, which
causes the `git config` invocation to match all lines which HAVE
"remote" in them, instead of matching lines which BEGIN with remote.

R=maruel@chromium.org, pkasting@chromium.org
BUG=

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

[commit-bot: I haz the power, 2016-01-09 08:08:42]

Committed patchset #3 (id:40001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=298185