Merge lp://qastaging/~danilo/linaro-android-build-tools/no-seed into lp://qastaging/linaro-android-build-tools

Testing this with four builds now:
  https://android-build.linaro.org/jenkins/job/danilo_panda-httpgit-seed/
  https://android-build.linaro.org/jenkins/job/danilo_panda-httpgit-noseed/
  https://android-build.linaro.org/jenkins/job/danilo_galaxynexus-jb-gcc47-aosp-blob/
  https://android-build.linaro.org/jenkins/job/danilo_panda-jb-gcc47-tilt-tracking-blob-tiny/

Looks OK. It feels like this bit should be tidier though:

71 + # Replace git://git.linaro.org/ and http://git.linaro.org/git/
72 + # base URLs with the scalable http://git.linaro.org/git-ro/.
73 + if remote.get('fetch') in (
74 + 'git://git.linaro.org', 'git://git.linaro.org/',
75 + 'http://git.linaro.org/git', 'http://git.linaro.org/git/'):
76 + remote.set('fetch', 'http://git.linaro.org/git-ro/')
77 + remotes_to_handle.add(remote.get('name'))
78 + # Replace git://android.git.linaro.org/ and
79 + # http://android.git.linaro.org/git/ base URLs with the scalable
80 + # http://git.linaro.org/git-ro/.
81 + elif remote.get('fetch') in (
82 + 'git://android.git.linaro.org', 'git://android.git.linaro.org/',
83 + 'http://android.git.linaro.org/git',
84 + 'http://android.git.linaro.org/git/'):
85 + remote.set('fetch', 'http://android.git.linaro.org/git-ro/')
86 + remotes_to_handle.add(remote.get('name'))

Isn't this basically:

url_match = re.search("git://([\w.]*git.linaro.org)(.*?)$", remote.get('fetch'))
if url_match:
    remote.set("fetch", urllib.urljoin("http://" + url_match.group(1)), "git-ro"))

If you wanted to be more careful you could add a check a check that url_match.group(2) is "git/*" or "".

Thanks for the review James: I want to keep static paths so it's more obvious what's going on (regular expressions are harder to update and always require you to stop and think to ensure they are doing the right thing). I even thought of defining a dict along the lines of

 rewrite_urls = {
   'git://git.linaro.org/': 'http://git.linaro.org/git-ro/',
   'git://git.linaro.org': 'http://git.linaro.org/git-ro',
   ...
 }

and using that instead. What do you think?

The dictionary approach is definitely better, but the trailing slash
causing duplications irritates me. You could rstrip('/') if we don't
need it (I assume we don't).

Agree about regexps - often the cause of write only code!

That would still lead to code that has to check for several conditions: eg. it'd become something like

  remote_url = remote.get('fetch')
  # Try to see if there's a remote URL
  target_url = rewrite_urls.get(remote_url,
                                rewrite_urls.get(remote_url.rstrip('/'), None))
  if target_url is not None:
     remote.set('fetch', target_url)

(Or, I could repeat the if a few times, but I'd hate that even more).

I don't think the above would be clear enough, and would be easy to break (one would have to ensure URLs in rewrite_urls are without the '/': either with more code, or by trying not to forget that :), and all to save a few measly almost-duplicated lines which would lead to:

  remote_url = remote.get('fetch')
  if remote_url in rewrite_urls:
     remote.set('fetch', rewrite_urls[remote_url])

On top of all this, I don't want to worry about whether repo cares or doesn't care about '/', and I want to copy whatever is being done in the manifest (if there's a trailing '/', I want rewritten URL to have it as well). Sure, we can add more code for that, but it'd make it even less clear what's going on, and it would be longer.

At this time, I want to live with this duplication, but if you really insist, I'll add the code to deal with the slash.

OK, you convinced me.