git_auto_svn.py: Rewrite http SVN URLs to https

git_auto_svn.py: Rewrite http SVN URLs to https

BUG=435091
TESTED=Ran the following:
git clone https://chromium.googlesource.com/external/webrtc
cd webrtc
git auto-svn
cat .git/config
Verified the URL was using http before this patch, and https after applying it.

R=agable@chromium.org, mmoss@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=293041

[kjellander_chromium, 2014-11-20 13:46:52]

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

[kjellander_chromium, 2014-11-20 13:46:53]

(+mmoss as cc)

I suggest we abandon the dangerous deployment of
https://chromereviews.googleplex.com/118007013/ and do this instead. If feels
silly to risk so much to just get auto-svn working, since it's only needed
temporarily until WebRTC is switched to Git.

[Michael Moss, 2014-11-20 16:51:30]

mmoss@chromium.org changed reviewers:
+ mmoss@chromium.org

[Michael Moss, 2014-11-20 16:51:30]

https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py
File git_auto_svn.py (right):

https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py#newcode88
git_auto_svn.py:88: svn_repo = svn_repo.replace('http://', 'https://')
I'm generally in favor of this change, though I think forcing to https in all
cases might be a bit heavy-handed. I don't know how many http-only upstream
repos we have, or how many anyone would actually be committing to, but I
wouldn't assume it's zero. At the very least, this should probably 'svn ls' or
something to verify that the https variation actually exists.

[agable, 2014-11-20 17:52:27]

On 2014/11/20 at 16:51:30, mmoss wrote:
> https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py
> File git_auto_svn.py (right):
> 
> https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py#newcode88
> git_auto_svn.py:88: svn_repo = svn_repo.replace('http://', 'https://')
> I'm generally in favor of this change, though I think forcing to https in all
cases might be a bit heavy-handed. I don't know how many http-only upstream
repos we have, or how many anyone would actually be committing to, but I
wouldn't assume it's zero. At the very least, this should probably 'svn ls' or
something to verify that the https variation actually exists.

It's a good point, but honestly I'd be happy with this anyway. If we have
http-only upstream repos that are in such heavy use that we get complains about
this, we should fix those repos :D Or add in the additional logic later. I'm
fine with this for now. LGTM.

[Michael Moss, 2014-11-20 18:00:05]

On 2014/11/20 17:52:27, agable wrote:
> On 2014/11/20 at 16:51:30, mmoss wrote:
> > https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py
> > File git_auto_svn.py (right):
> > 
> > https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py#newcode88
> > git_auto_svn.py:88: svn_repo = svn_repo.replace('http://', 'https://')
> > I'm generally in favor of this change, though I think forcing to https in
all
> cases might be a bit heavy-handed. I don't know how many http-only upstream
> repos we have, or how many anyone would actually be committing to, but I
> wouldn't assume it's zero. At the very least, this should probably 'svn ls' or
> something to verify that the https variation actually exists.
> 
> It's a good point, but honestly I'd be happy with this anyway. If we have
> http-only upstream repos that are in such heavy use that we get complains
about
> this, we should fix those repos :D Or add in the additional logic later. I'm
> fine with this for now. LGTM.

OK. LGTM.

[kjellander_chromium, 2014-11-20 18:00:37]

PTAL.

Should I remove my TODO comment if we plan to keep this code in there?

https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py
File git_auto_svn.py (right):

https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py#newcode88
git_auto_svn.py:88: svn_repo = svn_repo.replace('http://', 'https://')
On 2014/11/20 16:51:30, Michael Moss wrote:
> I'm generally in favor of this change, though I think forcing to https in all
> cases might be a bit heavy-handed. I don't know how many http-only upstream
> repos we have, or how many anyone would actually be committing to, but I
> wouldn't assume it's zero. At the very least, this should probably 'svn ls' or
> something to verify that the https variation actually exists.

You're right, what about PS#2? 
That should effectively limit us to Google code projects.

[Michael Moss, 2014-11-20 18:02:52]

On 2014/11/20 18:00:37, kjellander wrote:
> PTAL.
> 
> Should I remove my TODO comment if we plan to keep this code in there?
> 
> https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py
> File git_auto_svn.py (right):
> 
> https://codereview.chromium.org/745473003/diff/1/git_auto_svn.py#newcode88
> git_auto_svn.py:88: svn_repo = svn_repo.replace('http://', 'https://')
> On 2014/11/20 16:51:30, Michael Moss wrote:
> > I'm generally in favor of this change, though I think forcing to https in
all
> > cases might be a bit heavy-handed. I don't know how many http-only upstream
> > repos we have, or how many anyone would actually be committing to, but I
> > wouldn't assume it's zero. At the very least, this should probably 'svn ls'
or
> > something to verify that the https variation actually exists.
> 
> You're right, what about PS#2? 
> That should effectively limit us to Google code projects.

Even better. And seems reasonable to always do that, so no need for the TODO.

[agable, 2014-11-20 18:04:33]

Yeah, this is better. re-LGTM modulo the fact that I already import urlparse so
you should use that rather than 'startswith' and 'in'.

[kjellander_chromium, 2014-11-20 18:12:14]

On 2014/11/20 18:04:33, agable wrote:
> Yeah, this is better. re-LGTM modulo the fact that I already import urlparse
so
> you should use that rather than 'startswith' and 'in'.

Cool. PS#3 uses urlparse

[kjellander_chromium, 2014-11-20 18:13:57]

The CQ bit was checked by kjellander@chromium.org

[commit-bot: I haz the power, 2014-11-20 18:14:43]

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

[commit-bot: I haz the power, 2014-11-20 18:16:53]

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

[commit-bot: I haz the power, 2014-11-20 18:16:54]

Presubmit check for 745473003-40001 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 (106 files) (73.58s) failed
/b/depot_tools/third_party/logilab/astroid/modutils.py:520: UserWarning: Module
six was already imported from /b/depot_tools/third_party/six/__init__.py, but
/usr/local/lib/python2.7/dist-packages is being added to sys.path
  import pkg_resources
Problem importing module .svn: No module named .svn
Problem importing module .svn: No module named .svn
************* Module fix_encoding
E:232, 0: Slice index is not an int, None, or instance with __index__
(invalid-slice-index)
************* Module gclient_scm
E:301,18: No value for argument 'print_func' in constructor call
(no-value-for-parameter)
E:1191,18: No value for argument 'print_func' in constructor call
(no-value-for-parameter)


Presubmit checks took 121.5s to calculate.

[kjellander_chromium, 2014-11-20 19:45:53]

Committed patchset #3 (id:40001) manually as 293041.

[kjellander_chromium, 2014-11-29 16:56:46]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/767913002/ by kjellander@chromium.org.

The reason for reverting is: This actually makes the Git mirrors unusable for
git svn, since there's a mismatch between the svn remote URL and the URLs of the
git-svn-id footers in each Git commit (which confuses scripts like git_cl.py and
makes uploading and committing CLs impossible)..