Use a regex instead of shlex.split() to get remote url

Use a regex instead of shlex.split() to get remote url

shlex.spit(), in addition to doing str.split(' '), also strips out '\\'
from windows paths, which causes the cache_dir check to fail and Gclient to think
that we're not in cache_dir mode even if we are.

Instead of using shlex to strip data from a stdout log, I think regex is more
suited for this job.

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

[tandrii(chromium), 2014-08-22 12:46:15]

lgtm

[kjellander_chromium, 2014-08-22 13:24:19]

I guess it would be a good idea to add a test for GetActualRemoteURL?
I assume you've tested a couple of cases locally, especially on Windows?

[iannucci, 2014-08-22 16:59:00]

Also agree that this should have a simple unittest to repro the current problem
and be sure that this actually fixes it.

https://codereview.chromium.org/497053002/diff/20001/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/497053002/diff/20001/gclient_scm.py#newcode167
gclient_scm.py:167: return re.search(r'^remote\..*\.url (.*)$', log).group(1)
I think this could just be

log.splitlines()[0].split(' ', 1)[1]

(split lines, split first line on space once, take the right half)

[Ryan Tseng, 2014-08-22 17:06:42]

About waistdeep in gclient_scm_tests.py right now, tests coming soon :)

[Ryan Tseng, 2014-08-22 17:17:17]

So for the record I tested this on the failing windows bot and it fixes it
(gclient sync runs correctly) so I know it works.

If people are okay with committing this now and correct tests later, I can
submit this as is (after robbie's comment).

[iannucci, 2014-08-22 17:21:46]

On 2014/08/22 17:17:17, Ryan T. (EMEA time) wrote:
> So for the record I tested this on the failing windows bot and it fixes it
> (gclient sync runs correctly) so I know it works.
> 

^ this is not testing, it's verification :)

> If people are okay with committing this now and correct tests later, I can
> submit this as is (after robbie's comment).

I would really rather there be tests. Can't we just mock scm.GIT.Capture and
then write direct unittests for this function?

[hinoka, 2014-08-22 17:41:08]

Added super duper simple tests, see PS3 for the emulated failure, PS4 passes.

[iannucci, 2014-08-22 17:43:46]

lgtm

https://codereview.chromium.org/497053002/diff/60001/tests/gclient_scm_test.py
File tests/gclient_scm_test.py (right):

https://codereview.chromium.org/497053002/diff/60001/tests/gclient_scm_test.p...
tests/gclient_scm_test.py:96: REMOTE_STRINGS = [('remote.origin.url
E:\\foo\\bar', 'E:\\foo\\bar'),
oh, can you do one with quotes? I think that's what the shlex was for.

[iannucci, 2014-08-22 17:45:04]

On 2014/08/22 17:43:46, iannucci wrote:
> lgtm
> 
> https://codereview.chromium.org/497053002/diff/60001/tests/gclient_scm_test.py
> File tests/gclient_scm_test.py (right):
> 
>
https://codereview.chromium.org/497053002/diff/60001/tests/gclient_scm_test.p...
> tests/gclient_scm_test.py:96: REMOTE_STRINGS = [('remote.origin.url
> E:\\foo\\bar', 'E:\\foo\\bar'),
> oh, can you do one with quotes? I think that's what the shlex was for.

oh, yeah that's definitely it... meh

git config wat.nerds '"HEYOOOOOO'

writes:
[wat]
  nerds \"HEYOOOOOO

[Ryan Tseng, 2014-08-22 17:48:11]

On 2014/08/22 17:45:04, iannucci wrote:
> On 2014/08/22 17:43:46, iannucci wrote:
> > lgtm
> > 
> >
https://codereview.chromium.org/497053002/diff/60001/tests/gclient_scm_test.py
> > File tests/gclient_scm_test.py (right):
> > 
> >
>
https://codereview.chromium.org/497053002/diff/60001/tests/gclient_scm_test.p...
> > tests/gclient_scm_test.py:96: REMOTE_STRINGS = [('remote.origin.url
> > E:\\foo\\bar', 'E:\\foo\\bar'),
> > oh, can you do one with quotes? I think that's what the shlex was for.
> 
> oh, yeah that's definitely it... meh
> 
> git config wat.nerds '"HEYOOOOOO'
> 
> writes:
> [wat]
>   nerds \"HEYOOOOOO

ಠ_ಠ so thats why shlex was around?

doesn't seem to be necessary tho?

raichu(webrtc_fix) ~/depot_tools$ git config --local --get-regexp remote.*.url
remote.origin.url
https://chromium.googlesource.com/chromium/tools/depot_tools.git
remote.nerds.url "HEYOOOOOO

[iannucci, 2014-08-22 17:50:30]

On 2014/08/22 17:48:11, Ryan T. (EMEA time) wrote:
> On 2014/08/22 17:45:04, iannucci wrote:
> > On 2014/08/22 17:43:46, iannucci wrote:
> > > lgtm
> > > 
> > >
> https://codereview.chromium.org/497053002/diff/60001/tests/gclient_scm_test.py
> > > File tests/gclient_scm_test.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/497053002/diff/60001/tests/gclient_scm_test.p...
> > > tests/gclient_scm_test.py:96: REMOTE_STRINGS = [('remote.origin.url
> > > E:\\foo\\bar', 'E:\\foo\\bar'),
> > > oh, can you do one with quotes? I think that's what the shlex was for.
> > 
> > oh, yeah that's definitely it... meh
> > 
> > git config wat.nerds '"HEYOOOOOO'
> > 
> > writes:
> > [wat]
> >   nerds \"HEYOOOOOO
> 
> ಠ_ಠ so thats why shlex was around?
> 
> doesn't seem to be necessary tho?
> 
> raichu(webrtc_fix) ~/depot_tools$ git config --local --get-regexp remote.*.url
> remote.origin.url
> https://chromium.googlesource.com/chromium/tools/depot_tools.git
> remote.nerds.url "HEYOOOOOO

Oh, right... nm nvm me then. Just add a test and shipit :p Probably add one with
a space and a quote and call it good.

[Ryan Tseng, 2014-08-22 17:56:36]

done

[Ryan Tseng, 2014-08-22 17:56:47]

The CQ bit was checked by hinoka@google.com

[commit-bot: I haz the power, 2014-08-22 17:57:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hinoka@chromium.org/497053002/80001

[commit-bot: I haz the power, 2014-08-22 17:58:59]

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

[commit-bot: I haz the power, 2014-08-22 17:59:00]

Presubmit check for 497053002-80001 failed and returned exit status 1.

Running presubmit commit checks ...
Checking out rietveld...
Running save-description-on-failure.sh
Running push-basic.sh
Running upstream.sh
Running submit-from-new-dir.sh
Running abandon.sh
Running submodule-merge-test.sh
Running upload-local-tracking-branch.sh
Running hooks.sh
Running post-dcommit-hook-test.sh
Running upload-stale.sh
Running patch.sh
Running basic.sh

** Presubmit ERRORS **
Pylint (101 files) (31.45s) failed
************* Module
/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py
E0602:173,26:SCMWrapper.GetActualRemoteURL: Undefined variable
'_get_first_remote_url'
E0602:181,30:SCMWrapper.GetActualRemoteURL: Undefined variable
'_get_first_remote_url'


tests/gclient_smoketest.py (46.13s) failed
................F...F.F.F...................F.
======================================================================
FAIL: testPreDepsHooks (__main__.GClientSmokeGIT)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 1040, in testPreDepsHooks
    self.assertTree(tree)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/testing_support/fake_repos.py",
line 886, in assertTree
    self.assertEquals(diff, [])
AssertionError: {'src/git_pre_deps_hooked': 'git_pre_deps_hooked'} != []

======================================================================
FAIL: testSync (__main__.GClientSmokeGIT)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 874, in testSync
    ['deleting'])
  File "tests/gclient_smoketest.py", line 84, in parseGclient
    self.checkString(expected_stderr, stderr)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/testing_support/fake_repos.py",
line 868, in checkString
    self.assertEquals(expected, result, msg)
AssertionError: '' != 'Traceback (most recent call last):\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
2072, in <module>\n    sys.exit(Main(sys.argv[1:]))\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
2060, in Main\n    return dispatcher.execute(OptionParser(), argv)\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/subcommand.py", line
245, in execute\n    return command(parser, args[1:])\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
1838, in CMDsync\n    ret = client.RunOnDeps(\'update\', args)\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
1331, in RunOnDeps\n    self._CheckConfig()\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
1127, in _CheckConfig\n    actual_url = scm.GetActualRemoteURL(self._options)\n 
File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py",
line 173, in GetActualRemoteURL\n    actual_remote_url =
_get_first_remote_url(self.checkout_path)\nNameError: global name
\'_get_first_remote_url\' is not defined\n'

======================================================================
FAIL: testSyncJobs (__main__.GClientSmokeGIT)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 956, in testSyncJobs
    untangle=True)
  File "tests/gclient_smoketest.py", line 84, in parseGclient
    self.checkString(expected_stderr, stderr)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/testing_support/fake_repos.py",
line 868, in checkString
    self.assertEquals(expected, result, msg)
AssertionError: '' != 'Traceback (most recent call last):\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
2072, in <module>\n    sys.exit(Main(sys.argv[1:]))\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
2060, in Main\n    return dispatcher.execute(OptionParser(), argv)\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/subcommand.py", line
245, in execute\n    return command(parser, args[1:])\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
1838, in CMDsync\n    ret = client.RunOnDeps(\'update\', args)\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
1331, in RunOnDeps\n    self._CheckConfig()\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
1127, in _CheckConfig\n    actual_url = scm.GetActualRemoteURL(self._options)\n 
File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py",
line 173, in GetActualRemoteURL\n    actual_remote_url =
_get_first_remote_url(self.checkout_path)\nNameError: global name
\'_get_first_remote_url\' is not defined\n'

======================================================================
FAIL: testRevertAndStatus (__main__.GClientSmokeGITMutates)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 1148, in testRevertAndStatus
    ['running', 'running'])
  File "tests/gclient_smoketest.py", line 84, in parseGclient
    self.checkString(expected_stderr, stderr)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/testing_support/fake_repos.py",
line 868, in checkString
    self.assertEquals(expected, result, msg)
AssertionError: '' != 'Traceback (most recent call last):\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
2072, in <module>\n    sys.exit(Main(sys.argv[1:]))\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
2060, in Main\n    return dispatcher.execute(OptionParser(), argv)\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/subcommand.py", line
245, in execute\n    return command(parser, args[1:])\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
1896, in CMDrevert\n    return client.RunOnDeps(\'revert\', args)\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
1331, in RunOnDeps\n    self._CheckConfig()\n  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py", line
1127, in _CheckConfig\n    actual_url = scm.GetActualRemoteURL(self._options)\n 
File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py",
line 173, in GetActualRemoteURL\n    actual_remote_url =
_get_first_remote_url(self.checkout_path)\nNameError: global name
\'_get_first_remote_url\' is not defined\n'

======================================================================
FAIL: testSkiaDEPSChangeGit (__main__.SkiaDEPSTransitionSmokeTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 1507, in testSkiaDEPSChangeGit
    self.assertEqual(res[2], 0, 'DEPS change sync failed.')
AssertionError: DEPS change sync failed.

----------------------------------------------------------------------
Ran 46 tests in 44.700s

FAILED (failures=5)
Traceback (most recent call last):
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 2072, in <module>
    sys.exit(Main(sys.argv[1:]))
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 2060, in Main
    return dispatcher.execute(OptionParser(), argv)
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/subcommand.py",
line 245, in execute
    return command(parser, args[1:])
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 1838, in CMDsync
    ret = client.RunOnDeps('update', args)
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 1331, in RunOnDeps
    self._CheckConfig()
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 1127, in _CheckConfig
    actual_url = scm.GetActualRemoteURL(self._options)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
173, in GetActualRemoteURL
    actual_remote_url = _get_first_remote_url(self.checkout_path)
NameError: global name '_get_first_remote_url' is not defined

Traceback (most recent call last):
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 2072, in <module>
    sys.exit(Main(sys.argv[1:]))
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 2060, in Main
    return dispatcher.execute(OptionParser(), argv)
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/subcommand.py",
line 245, in execute
    return command(parser, args[1:])
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 1838, in CMDsync
    ret = client.RunOnDeps('update', args)
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 1331, in RunOnDeps
    self._CheckConfig()
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 1127, in _CheckConfig
    actual_url = scm.GetActualRemoteURL(self._options)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
173, in GetActualRemoteURL
    actual_remote_url = _get_first_remote_url(self.checkout_path)
NameError: global name '_get_first_remote_url' is not defined

Traceback (most recent call last):
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 2072, in <module>
    sys.exit(Main(sys.argv[1:]))
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient.py",
line 2060, in Main
    return dispatcher.execute(OptionParser(), argv)
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/subcommand.py",
line 245, in execute
    return command(parser, args[1:])
  File "/b/infra_internal/com…
(message too large)

[Ryan Tseng, 2014-08-22 18:06:43]

The CQ bit was checked by hinoka@google.com

[commit-bot: I haz the power, 2014-08-22 18:07:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hinoka@chromium.org/497053002/100001

[commit-bot: I haz the power, 2014-08-22 18:08:56]

Committed patchset #6 (100001) as 291446