Bugfixing URL construction for extract build step

scripts/slave/extract_build.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """A tool to extract a build, executed by a buildbot slave.
 """
 
 import optparse
 import os
@@ skipping 59 matching lines @@
 
   abs_webkit_dir = None
   if options.webkit_dir:
     abs_webkit_dir = os.path.join(abs_build_dir, '..', options.webkit_dir)
   base_filename, version_suffix = slave_utils.GetZipFileNames(
       options.build_properties, abs_build_dir, abs_webkit_dir, extract=True)
 
   replace_dict = dict(options.build_properties)
   # If builddir isn't specified, assume buildbot used the builder name
   # as the root folder for the build.
-  if not replace_dict.get('parent_builddir'):
+  if not replace_dict.get('parent_builddir') and replace_dict.get('parentname'):

[kjellander_chromium, 2012/12/28 16:47:31]

This avoids crashing if an empty build_properties dict is passed, which I think
should be possible.

[Isaac (away), 2013/01/08 08:54:02]

Still might crash because replace_dict wouldn't have parent_builddir and could
fail the string format operation on line 91.  How about instead we change line
81 to have replace_dict.get('parentname', '')

[kjellander_chromium, 2013/01/08 09:32:52]

Good idea. Updated. 
On 2013/01/08 08:54:02, Isaac wrote:

     replace_dict['parent_builddir'] = replace_dict['parentname']
   replace_dict['base_filename'] = base_filename
   url = options.build_url or options.factory_properties.get('build_url')
   if not url:
     url = ('http://%(parentslavename)s/b/build/slave/%(parent_builddir)s/'
            'chrome_staging')
   if url[-4:] != '.zip': # assume filename not specified
-    url = os.path.join(url, '%(base_filename)s.zip')
+    # Append the filename to the base URL. First strip any trailing slashes.

[kjellander_chromium, 2012/12/28 16:47:31]

Simple way of avoiding double slashes being concatenated into the URL.
urlparse.urljoin cannot be used since it doesn't support gs:// URLs (in addition
to the somewhat complicated way it works).

+    url = url.strip('/')

[Isaac (away), 2013/01/08 08:54:02]

nit, cleaner to use rstrip

[kjellander_chromium, 2013/01/08 09:32:52]

Done.

+    url = '%s/%s' % (url, '%(base_filename)s.zip')
   url = url % replace_dict
 
   versioned_url = url.replace('.zip', version_suffix + '.zip')
   return url, versioned_url
 
 
 def real_main(options):
   """ Download a build, extract it to build\BuildDir\full-build-win32
       and rename it to build\BuildDir\Target
   """
@@ skipping 136 matching lines @@
   if not options.target:
     options.target = options.factory_properties.get('target', 'Release')
   if not options.webkit_dir:
     options.webkit_dir = options.factory_properties.get('webkit_dir')
 
   return real_main(options)
 
 
 if '__main__' == __name__:
   sys.exit(main())