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

[Данило Шеган]

Use scalable git-over-http access
=================================

This branch introduces manifest rewriting to change any references to
git://git.linaro.org/ and git://android.git.linaro.org/ to their
simple, scalable http URLs (http://(android.)git.linaro.org/git-ro).

At the same time, we need to append '.git' to repository paths.

Along the way, I also kill the non-working 'mirror' functionality for
non-seeded builds: we should be able to get similar performance with seeded
builds if we get a http proxy going on amazon ec2 cloud.

[Данило Шеган (danilo)]

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/

[lp:~danilo/linaro-android-build-tools/no-seed]

545. By Данило Шеган on 2012-11-22

Minor string reformatting.

[James Tunnicliffe (dooferlad)]

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 "".

review: Approve

[Данило Шеган (danilo)]

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?

[James Tunnicliffe (dooferlad)]

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!

[Данило Шеган (danilo)]

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.

[lp:~danilo/linaro-android-build-tools/no-seed]

546. By Данило Шеган on 2012-11-22

Split out URLs being rewritten into a dict as per review.

[James Tunnicliffe (dooferlad)]

OK, you convinced me.

review: Approve

=== modified file 'build-scripts/helpers'
--- build-scripts/helpers	2012-11-19 11:35:11 +0000
+++ build-scripts/helpers	2012-11-22 14:42:20 +0000
@@ -28,19 +28,15 @@
     mkdir -p out
     cp .repo/manifest.xml out/source-manifest.xml
 
-    # Allow either specify hostname or pass $HUDSON_URL from Jenkins
-    mirror_host=`python -c 'import urlparse, sys; print urlparse.urlparse(sys.argv[1]).netloc if "/jenkins" in sys.argv[1] else sys.argv[1]' "$1"`
+    echo Replace Linaro git URLs with lightweight http git URLs.
+    ${BUILD_SCRIPT_ROOT}/rewrite-manifest.py .repo/manifest.xml > processed-manifest.xml
+    cp processed-manifest.xml .repo/manifest.xml
 
-    time python -c 'import xmlrpclib, sys; print xmlrpclib.ServerProxy(sys.argv[1]).mirror(open(".repo/manifest.xml").read())' "http://$mirror_host:8080" > .repo/mirror-manifest.xml
-    echo "Received modified manifest:"
-    cat .repo/mirror-manifest.xml
     echo ----------------------------
-    cp .repo/mirror-manifest.xml .repo/manifest.xml
     time repo sync $REPO_QUIET -j$SYNC_JOBS
     # Restore source manifest temporarily to create pinned manifest
     cp out/source-manifest.xml .repo/manifest.xml
     repo manifest -r -o out/pinned-manifest.xml
-    cp .repo/mirror-manifest.xml .repo/manifest.xml
 }
 
 repo-sync-from-seed () {
@@ -65,10 +61,16 @@
     mkdir -p out
     cp .repo/manifest.xml out/source-manifest.xml
 
+    echo Replace Linaro git URLs with lightweight http git URLs.
+    ${BUILD_SCRIPT_ROOT}/rewrite-manifest.py .repo/manifest.xml > processed-manifest.xml
+    cp processed-manifest.xml .repo/manifest.xml
+
     export TIMEFORMAT="TIME: Repo sync (using seed as reference): %lR"
     time repo sync $REPO_QUIET -j$SYNC_JOBS
     unset TIMEFORMAT
 
+    # Restore source manifest temporarily to create pinned manifest
+    cp out/source-manifest.xml .repo/manifest.xml
     repo manifest -r -o out/pinned-manifest.xml
 }
 

=== added file 'build-scripts/rewrite-manifest.py'
--- build-scripts/rewrite-manifest.py	1970-01-01 00:00:00 +0000
+++ build-scripts/rewrite-manifest.py	2012-11-22 14:42:20 +0000
@@ -0,0 +1,56 @@
+#!/usr/bin/env python
+
+from xml.etree.cElementTree import (
+    ElementTree,
+    tostring,
+)
+import sys
+
+rewrite_urls = {
+    'git://git.linaro.org': 'http://git.linaro.org/git-ro',
+    'git://git.linaro.org/': 'http://git.linaro.org/git-ro/',
+    'git://android.git.linaro.org': 'http://android.git.linaro.org/git-ro',
+    'git://android.git.linaro.org/': 'http://android.git.linaro.org/git-ro/',
+}
+
+def rewrite_git_urls(root):
+    """Rewrites all android.git.linaro.org and git.linaro.org URLs.
+
+    Makes them use /git-ro and appends '.git' to repo names if they
+    do not already have it.
+    """
+    remotes_to_handle = set([])
+    default_remote = root.find('default').get('remote', None)
+
+    for remote in root.findall("remote"):
+        if default_remote is None:
+            default_remote = remote.get('name')
+
+        remote_url = remote.get('fetch')
+        if remote_url in rewrite_urls:
+            remote.set('fetch', rewrite_urls[remote_url])
+            remotes_to_handle.add(remote.get('name'))
+
+    for project in root.findall("project"):
+        if (project.get('remote') in remotes_to_handle or
+            project.get('remote') is None and
+            default_remote in remotes_to_handle):
+            if not project.get('name').endswith('.git'):
+                project.set('name', project.get('name') + '.git')
+
+if __name__ == '__main__':
+    import argparse
+    parser = argparse.ArgumentParser(
+        description=(
+            'Switch Linaro git URLs to scalable git URLs in repo manifest '
+            'files and print the result manifest on stdout.'))
+    parser.add_argument('manifest', type=str,
+                        help='manifest file to process')
+    args = parser.parse_args()
+
+    # Parse the manifest file.
+    et = ElementTree()
+    doc = open(args.manifest)
+    root = et.parse(doc)
+    rewrite_git_urls(root)
+    print tostring(root, 'UTF-8').decode('utf-8')

=== modified file 'control/setup-control-node'
--- control/setup-control-node	2012-08-02 10:26:24 +0000
+++ control/setup-control-node	2012-11-22 14:42:20 +0000
@@ -13,7 +13,7 @@
 MYPATH=$PWD/`dirname $0`
 HOSTNAME=$1
 [ -z "$HOSTNAME" ] && HOSTNAME=`hostname`
-REPO_MIRROR="--repo-url=git://android.git.linaro.org/tools/repo.git"
+REPO_MIRROR="--repo-url=http://android.git.linaro.org/git-ro/tools/repo.git"
 
 #
 # +++ BASE +++