add PRESUBMIT for native_client_sdk.

Currently just does pylint.

Fix many obvious lint errors (but not all by any means).

BUG=


Review URL: https://chromiumcodereview.appspot.com/10868089

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@154856 0039d316-1c4b-4281-b951-d872f2087c98

17 files changed, 168 insertions, 115 deletions

diff --git a/native_client_sdk/PRESUBMIT.py b/native_client_sdk/PRESUBMIT.py
new file mode 100644
index 0000000..1b711b9
--- /dev/null
+++ b/native_client_sdk/PRESUBMIT.py
@@ -0,0 +1,46 @@
+# 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.
+
+"""Top-level presubmit script for isolate.
+
+See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts for
+details on the presubmit API built into gcl.
+"""
+
+
+def CommonChecks(input_api, output_api):
+  output = []
+  disabled_warnings = [
+    'R0401',  # Cyclic import
+    'W0613',  # Unused argument
+    'E1103',  # subprocess.communicate() generates these :(
+  ]
+  black_list = [r'src/site_scons/.*',
+                r'.*/decode_dump.py',
+                r'src/build_tools/build_sdk.py',
+                r'src/build_tools/buildbot_common.py',
+                r'src/build_tools/buildbot_run.py',
+                r'src/build_tools/build_utils.py',
+                r'src/build_tools/build_updater.py',
+                r'src/build_tools/manifest_util.py',
+                r'src/build_tools/nacl*',
+                r'src/build_tools/generate_make.py',
+                r'src/build_tools/sdk_tools/sdk_update_main.py',
+                r'src/build_tools/sdk_tools/sdk_update.py',
+                r'src/project_templates',
+                r'.*/update_manifest.py',
+                r'.*/update_nacl_manifest.py',
+                r'src/build_tools/tests/.*']
+  canned = input_api.canned_checks
+  output.extend(canned.RunPylint(input_api, output_api, black_list=black_list,
+                disabled_warnings=disabled_warnings))
+  return output
+
+
+def CheckChangeOnUpload(input_api, output_api):
+  return CommonChecks(input_api, output_api)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+  return CommonChecks(input_api, output_api)
diff --git a/native_client_sdk/src/build_tools/build_sdk.py b/native_client_sdk/src/build_tools/build_sdk.py
index f9836f4..4dff2cd 100755
--- a/native_client_sdk/src/build_tools/build_sdk.py
+++ b/native_client_sdk/src/build_tools/build_sdk.py
@@ -129,9 +129,9 @@ def GetBuildArgs(tcname, tcpath, outdir, arch, xarch=None):
   mode = '--mode=opt-host,nacl'
   arch_name = GetArchName(arch, xarch)
   plat = 'platform=' + arch_name
-  bin = 'bindir=' + os.path.join(outdir, 'tools')
+  binarg = 'bindir=' + os.path.join(outdir, 'tools')
   lib = 'libdir=' + GetToolchainNaClLib(tcname, tcpath, arch, xarch)
-  args = [scons, mode, plat, bin, lib, '-j10',
+  args = [scons, mode, plat, binarg, lib, '-j10',
           'install_bin', 'install_lib']
   if tcname == 'glibc':
     args.append('--nacl_glibc')
@@ -310,29 +310,29 @@ def InstallHeaders(tc_dst_inc, pepper_ver, tc_name):
 
   # Copy in the gles2 headers
   buildbot_common.MakeDir(os.path.join(ppapi, 'gles2'))
-  buildbot_common.CopyDir(os.path.join(PPAPI_DIR,'lib','gl','gles2','*.h'),
+  buildbot_common.CopyDir(os.path.join(PPAPI_DIR, 'lib', 'gl', 'gles2', '*.h'),
           os.path.join(ppapi, 'gles2'))
 
   # Copy the EGL headers
   buildbot_common.MakeDir(os.path.join(tc_dst_inc, 'EGL'))
   buildbot_common.CopyDir(
-          os.path.join(PPAPI_DIR,'lib','gl','include','EGL', '*.h'),
+          os.path.join(PPAPI_DIR, 'lib', 'gl', 'include', 'EGL', '*.h'),
           os.path.join(tc_dst_inc, 'EGL'))
 
   # Copy the GLES2 headers
   buildbot_common.MakeDir(os.path.join(tc_dst_inc, 'GLES2'))
   buildbot_common.CopyDir(
-          os.path.join(PPAPI_DIR,'lib','gl','include','GLES2', '*.h'),
+          os.path.join(PPAPI_DIR, 'lib', 'gl', 'include', 'GLES2', '*.h'),
           os.path.join(tc_dst_inc, 'GLES2'))
 
   # Copy the KHR headers
   buildbot_common.MakeDir(os.path.join(tc_dst_inc, 'KHR'))
   buildbot_common.CopyDir(
-          os.path.join(PPAPI_DIR,'lib','gl','include','KHR', '*.h'),
+          os.path.join(PPAPI_DIR, 'lib', 'gl', 'include', 'KHR', '*.h'),
           os.path.join(tc_dst_inc, 'KHR'))
 
   # Copy the lib files
-  buildbot_common.CopyDir(os.path.join(PPAPI_DIR,'lib'), 
+  buildbot_common.CopyDir(os.path.join(PPAPI_DIR, 'lib'), 
           os.path.join(tc_dst_inc, 'ppapi'))
 
 
@@ -398,11 +398,10 @@ def BuildStepCopyBuildHelpers(pepperdir, platform):
                                os.path.join(pepperdir, 'tools' ,'make.exe'))
     rename_list = ['ncval_x86_32', 'ncval_x86_64',
                    'sel_ldr_x86_32', 'sel_ldr_x86_64']
-    tools = os.path.join(pepperdir, 'tools')
     for name in rename_list:
-        src = os.path.join(pepperdir, 'tools', name)
-        dst = os.path.join(pepperdir, 'tools', name + '.exe')
-        buildbot_common.Move(src, dst)
+      src = os.path.join(pepperdir, 'tools', name)
+      dst = os.path.join(pepperdir, 'tools', name + '.exe')
+      buildbot_common.Move(src, dst)
 
 
 EXAMPLE_LIST = [
diff --git a/native_client_sdk/src/build_tools/build_utils.py b/native_client_sdk/src/build_tools/build_utils.py
index 5bad1a4..c0707ed 100644
--- a/native_client_sdk/src/build_tools/build_utils.py
+++ b/native_client_sdk/src/build_tools/build_utils.py
@@ -186,7 +186,7 @@ def SVNRevision():
     p = subprocess.Popen('git svn info', shell=True, stdout=subprocess.PIPE,
                          cwd=run_path)
     if p.wait() != 0:
-      raise AssertionError('Cannot determine SVN revision of this repository');
+      raise AssertionError('Cannot determine SVN revision of this repository')
 
   svn_info = p.communicate()[0]
   m = re.search('Revision: ([0-9]+)', svn_info)
diff --git a/native_client_sdk/src/build_tools/make_rules.py b/native_client_sdk/src/build_tools/make_rules.py
index b5c8823..cbc3577 100755
--- a/native_client_sdk/src/build_tools/make_rules.py
+++ b/native_client_sdk/src/build_tools/make_rules.py
@@ -5,6 +5,8 @@
 
 import os
 
+# pylint: disable=C0301
+# This file contains lines longer than 80
 
 #
 # Default macros for various platforms.
@@ -294,6 +296,10 @@ class MakeRules(object):
 
   def __init__(self, tc, cfg=None, arch=None):
     self.tc = tc
+    self.project = ''
+    self.cfg = ''
+    self.arch = ''
+    self.ptype = ''
     self.defines = []
     self.includes = []
     self.libraries = []
@@ -332,7 +338,7 @@ class MakeRules(object):
     self.vars['<EXT>'] = EXT
     out = '<tc>/<config>/%s_<ARCH>.o : %s $(THIS_MAKE) | <tc>/<config>\n' % (
         os.path.splitext(src)[0], src)
-    out+= BUILD_RULES[self.tc][EXT][self.cfg] + '\n\n'
+    out += BUILD_RULES[self.tc][EXT][self.cfg] + '\n\n'
     return self.Replace(out)
 
   def BuildLinkRule(self):
@@ -396,7 +402,6 @@ class MakeRules(object):
     self.SetLibraries(libs)
 
   def SetSource(self, src):
-    self.source = source
     self.vars['<src>'] = src
 
   def SetToolchain(self, tc):
@@ -421,7 +426,6 @@ class MakeRules(object):
     # Add other passed in replacements
     for key in kwargs:
       self.vars['<%s>' % key] = kwargs[key]
-    self.var_set = kwargs
 
   def Replace(self, text):
     return Replace(text, self.vars)
diff --git a/native_client_sdk/src/build_tools/manifest_util.py b/native_client_sdk/src/build_tools/manifest_util.py
index 6b6acca..57cab4f 100644
--- a/native_client_sdk/src/build_tools/manifest_util.py
+++ b/native_client_sdk/src/build_tools/manifest_util.py
@@ -47,9 +47,9 @@ def GetHostOS():
       }[sys.platform]
 
 
-def DictToJSON(dict):
+def DictToJSON(pydict):
   """Convert a dict to a JSON-formatted string."""
-  pretty_string = json.dumps(dict, sort_keys=False, indent=2)
+  pretty_string = json.dumps(pydict, sort_keys=False, indent=2)
   # json.dumps sometimes returns trailing whitespace and does not put
   # a newline at the end.  This code fixes these problems.
   pretty_lines = pretty_string.split('\n')
@@ -108,13 +108,13 @@ class Archive(dict):
     """ Create a new archive for the given host-os name. """
     self['host_os'] = host_os_name
 
-  def CopyFrom(self, dict):
+  def CopyFrom(self, src):
     """Update the content of the archive by copying values from the given
        dictionary.
 
     Args:
-      dict: The dictionary whose values must be copied to the archive."""
-    for key, value in dict.items():
+      src: The dictionary whose values must be copied to the archive."""
+    for key, value in src.items():
       self[key] = value
 
   def Validate(self):
diff --git a/native_client_sdk/src/build_tools/nacl-mono-buildbot.py b/native_client_sdk/src/build_tools/nacl-mono-buildbot.py
index 2d468a7..ff849e3 100755
--- a/native_client_sdk/src/build_tools/nacl-mono-buildbot.py
+++ b/native_client_sdk/src/build_tools/nacl-mono-buildbot.py
@@ -15,7 +15,8 @@ GS_MANIFEST_PATH = 'gs://nativeclient-mirror/nacl/nacl_sdk/'
 SDK_MANIFEST = 'naclsdk_manifest2.json'
 MONO_MANIFEST = 'naclmono_manifest.json' 
 
-def build_and_upload_mono(sdk_revision, pepper_revision, sdk_url, upload_path, args):
+def build_and_upload_mono(sdk_revision, pepper_revision, sdk_url,
+                          upload_path, args):
   install_dir = 'naclmono'
   buildbot_common.RemoveDir(install_dir)
 
@@ -111,7 +112,6 @@ def update_mono_sdk_json(infos):
   mono_manifest = json.loads(manifest_file.read())
   manifest_file.close()
 
-  newbundles = {}
   for info in infos:
     bundle = {}
     bundle['name'] = info['naclmono_name']
diff --git a/native_client_sdk/src/build_tools/sdk_tools/sdk_update.py b/native_client_sdk/src/build_tools/sdk_tools/sdk_update.py
index 1bfefe3..ac423a5 100755
--- a/native_client_sdk/src/build_tools/sdk_tools/sdk_update.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/sdk_update.py
@@ -3,13 +3,6 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import os
-import re
-import subprocess
-from sdk_update_common import *
-import sys
-import tempfile
-
 """Shim script for the SDK updater, to allow automatic updating.
 
 The purpose of this script is to be a shim which automatically updates
@@ -20,6 +13,12 @@ When the sdk_tools bundle has been updated to the most recent version, this
 script forwards its arguments to sdk_updater_main.py.
 """
 
+import os
+import subprocess
+from sdk_update_common import *
+import sys
+import tempfile
+
 
 SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
 SDK_UPDATE_MAIN = os.path.join(SCRIPT_DIR, 'sdk_update_main.py')
diff --git a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_common.py b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_common.py
index 7cf0a88..99d4c62 100644
--- a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_common.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_common.py
@@ -2,6 +2,8 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+"""Utility functions for sdk_update.py and sdk_update_main.py."""
+
 import errno
 import os
 import shutil
@@ -9,8 +11,6 @@ import subprocess
 import sys
 import time
 
-"""Utility functions for sdk_update.py and sdk_update_main.py."""
-
 
 class Error(Exception):
   """Generic error/exception for sdk_update module"""
@@ -36,7 +36,7 @@ def RemoveDir(outdir):
 
   try:
     shutil.rmtree(outdir)
-  except:
+  except OSError:
     if not os.path.exists(outdir):
       return
     # On Windows this could be an issue with junctions, so try again with rmdir
@@ -49,6 +49,7 @@ def RenameDir(srcdir, destdir):
      rename if it already exists."""
 
   max_tries = 5
+  num_tries = 0
   for num_tries in xrange(max_tries):
     try:
       RemoveDir(destdir)
@@ -61,8 +62,9 @@ def RenameDir(srcdir, destdir):
       # handling a Windows flaky access error.  Sleep one second and try
       # again.
       time.sleep(num_tries + 1)
+
   # end of while loop -- could not RenameDir
-  raise Error('Could not RenameDir %s => %s after %d tries.\n' %
+  raise Error('Could not RenameDir %s => %s after %d tries.\n'
               'Please check that no shells or applications '
               'are accessing files in %s.'
               % (srcdir, destdir, num_tries, destdir))
diff --git a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py
index 14aac81..8b18794 100755
--- a/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/sdk_update_main.py
@@ -48,10 +48,10 @@ Example Usage:
   naclsdk help update
   naclsdk sources --list'''
 
-CONFIG_FILENAME='naclsdk_config.json'
-MANIFEST_FILENAME='naclsdk_manifest2.json'
-SDK_TOOLS='sdk_tools'  # the name for this tools directory
-USER_DATA_DIR='sdk_cache'
+CONFIG_FILENAME = 'naclsdk_config.json'
+MANIFEST_FILENAME = 'naclsdk_manifest2.json'
+SDK_TOOLS = 'sdk_tools'  # the name for this tools directory
+USER_DATA_DIR = 'sdk_cache'
 
 HTTP_CONTENT_LENGTH = 'Content-Length'  # HTTP Header field for content length
 
@@ -134,7 +134,8 @@ def ExtractInstaller(installer, outdir):
     curpath = os.getcwd()
     try:
       tar_file = cygtar.CygTar(installer, 'r', verbose=True)
-      if outdir: os.chdir(outdir)
+      if outdir:
+        os.chdir(outdir)
       tar_file.Extract()
     finally:
       if tar_file:
@@ -202,7 +203,8 @@ def DownloadArchiveToFile(archive, dest_path):
                     'Expected %s bytes but got %s' %
                     (archive.url, content_length, size))
     finally:
-      if from_stream: from_stream.close()
+      if from_stream:
+        from_stream.close()
   return sha1, size
 
 
@@ -235,8 +237,7 @@ def LoadManifestFromURLs(urls):
       raise Error('Unable to open %s. [%s]' % (url, e))
 
     manifest_stream = cStringIO.StringIO()
-    sha1, size = manifest_util.DownloadAndComputeHash(url_stream,
-                                                      manifest_stream)
+    manifest_util.DownloadAndComputeHash(url_stream, manifest_stream)
     temp_manifest = manifest_util.SDKManifest()
     temp_manifest.LoadDataFromString(manifest_stream.getvalue())
 
@@ -491,8 +492,8 @@ def Update(options, argv, config):
     bundle_name: Install/Update only the given bundle
   '''
   DebugPrint("Running Update command with: %s, %s" % (options, argv))
-  ALL='all'  # Update all bundles
-  RECOMMENDED='recommended'  # Only update the bundles with recommended=yes
+  ALL = 'all'  # Update all bundles
+  RECOMMENDED = 'recommended'  # Only update the bundles with recommended=yes
 
   parser = optparse.OptionParser(usage=Update.__doc__)
   parser.add_option(
@@ -536,7 +537,7 @@ def Update(options, argv, config):
     def UpdateBundle():
       '''Helper to install a bundle'''
       archive = bundle.GetHostOSArchive()
-      (scheme, host, path, _, _, _) = urlparse.urlparse(archive['url'])
+      (_, _, path, _, _, _) = urlparse.urlparse(archive['url'])
       dest_filename = os.path.join(options.user_data_dir, path.split('/')[-1])
       sha1, size = DownloadArchiveToFile(archive, dest_filename)
       if sha1 != archive.GetChecksum():
@@ -598,7 +599,7 @@ def Sources(options, argv, config):
       '-l', '--list', dest='do_list',
       default=False, action='store_true',
       help='List additional package sources')
-  (source_options, args) = parser.parse_args(argv)
+  source_options, _ = parser.parse_args(argv)
 
   write_config = False
   if source_options.url_to_add:
diff --git a/native_client_sdk/src/build_tools/sdk_tools/set_nacl_env.py b/native_client_sdk/src/build_tools/sdk_tools/set_nacl_env.py
index 57958f2..c2aaf69 100755
--- a/native_client_sdk/src/build_tools/sdk_tools/set_nacl_env.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/set_nacl_env.py
@@ -9,7 +9,6 @@ __author__ = 'gwink@google.com (Georges Winkenbach)'
 
 import optparse
 import os
-import re
 import subprocess
 import sys
 
diff --git a/native_client_sdk/src/build_tools/sdk_tools/update_manifest.py b/native_client_sdk/src/build_tools/sdk_tools/update_manifest.py
index 9b7e7c6..f0af47d 100755
--- a/native_client_sdk/src/build_tools/sdk_tools/update_manifest.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/update_manifest.py
@@ -25,7 +25,7 @@ sys.path.append(os.path.join(NACL_DIR, 'build'))
 
 import sdk_update
 
-HELP='''"Usage: %prog [-b bundle] [options]"
+HELP = '''"Usage: %prog [-b bundle] [options]"
 
 Actions for particular bundles:
   sdk_tools: Upload the most recently built nacl_sdk.zip and sdk_tools.tgz
@@ -84,7 +84,7 @@ def UpdateBundle(bundle, options):
     option_val = getattr(options, option_key, None)
     if option_val is not None:
       bundle[attribute_key] = option_val
-      delattr(options, option_key);
+      delattr(options, option_key)
   # Validate what we have so far; we may just avoid going through a lengthy
   # download, just to realize that some other trivial stuff is missing.
   bundle.Validate()
@@ -93,7 +93,7 @@ def UpdateBundle(bundle, options):
     platform_url = getattr(options, option_key, None)
     if platform_url is not None:
       bundle.UpdateArchive(host_os, platform_url)
-      delattr(options, option_key);
+      delattr(options, option_key)
 
 
 class UpdateSDKManifest(sdk_update.SDKManifest):
@@ -154,7 +154,7 @@ class UpdateSDKManifest(sdk_update.SDKManifest):
           raise Error('Unused option "%s" for bundle "%s"' % (key, bundle_name))
         else:
           raise Error('No bundle name specified')
-    return True;
+    return True
 
   def UpdateManifest(self, options):
     ''' Update the manifest object with values from the command-line options
diff --git a/native_client_sdk/src/build_tools/update_nacl_manifest.py b/native_client_sdk/src/build_tools/update_nacl_manifest.py
index 1212784..7363834 100755
--- a/native_client_sdk/src/build_tools/update_nacl_manifest.py
+++ b/native_client_sdk/src/build_tools/update_nacl_manifest.py
@@ -32,7 +32,7 @@ GS_SDK_MANIFEST = GS_BUCKET_PATH + MANIFEST_BASENAME
 GS_MANIFEST_BACKUP_DIR = GS_BUCKET_PATH + 'manifest_backups/'
 
 CANARY_BUNDLE_NAME = 'pepper_canary'
-CANARY='canary'
+CANARY = 'canary'
 
 
 def SplitVersion(version_string):
@@ -378,7 +378,7 @@ class VersionFinder(object):
     """
     for platform, channel, version, _ in self.history:
       version = SplitVersion(version)
-      if with_platform == platform and channel==CANARY:
+      if with_platform == platform and channel == CANARY:
         yield channel, version
 
 
@@ -407,7 +407,7 @@ class VersionFinder(object):
       try:
         for i, platform_gen in enumerate(platform_generators):
           if platform_versions[i][1] != shared_version:
-              platform_versions[i] = platform_gen.next()
+            platform_versions[i] = platform_gen.next()
       except StopIteration:
         return
 
@@ -437,8 +437,8 @@ class VersionFinder(object):
 
     assert all(file.startswith('gs://') for file in files)
 
-    archives = [file for file in files if not file.endswith('.json')]
-    manifests = [file for file in files if file.endswith('.json')]
+    archives = [f for f in files if not f.endswith('.json')]
+    manifests = [f for f in files if f.endswith('.json')]
 
     # don't include any archives that don't have an associated manifest.
     return filter(lambda a: a + '.json' in manifests, archives)
@@ -615,7 +615,7 @@ def main(args):
   try:
     delegate = RealDelegate(dryrun=options.dryrun, gsutil=options.gsutil)
     Run(delegate, ('mac', 'win', 'linux'))
-  except Exception, e:
+  except Exception:
     if options.mailfrom and options.mailto:
       traceback.print_exc()
       scriptname = os.path.basename(sys.argv[0])
diff --git a/native_client_sdk/src/examples/httpd.py b/native_client_sdk/src/examples/httpd.py
index e715f5b..a130ed1 100755
--- a/native_client_sdk/src/examples/httpd.py
+++ b/native_client_sdk/src/examples/httpd.py
@@ -19,16 +19,17 @@ import sys
 import urlparse
 
 
-EXAMPLE_PATH=os.path.dirname(os.path.abspath(__file__))
+EXAMPLE_PATH = os.path.dirname(os.path.abspath(__file__))
 NACL_SDK_ROOT = os.getenv('NACL_SDK_ROOT', os.path.dirname(EXAMPLE_PATH))
 
 
 if os.path.exists(NACL_SDK_ROOT):
   sys.path.append(os.path.join(NACL_SDK_ROOT, 'tools'))
+  # pylint: disable=F0401
   import decode_dump
   import getos
 else:
-  NACL_SDK_ROOT=None
+  NACL_SDK_ROOT = None
 
 last_nexe = None
 last_nmf = None
@@ -63,6 +64,8 @@ def SanityCheckDirectory():
 # faster responses.
 class QuittableHTTPServer(SocketServer.ThreadingMixIn,
                           BaseHTTPServer.HTTPServer):
+  is_running = False
+
   def serve_forever(self, timeout=0.5):
     self.is_running = True
     self.timeout = timeout
@@ -77,13 +80,13 @@ class QuittableHTTPServer(SocketServer.ThreadingMixIn,
 
 
 # "Safely" split a string at |sep| into a [key, value] pair.  If |sep| does not
-# exist in |str|, then the entire |str| is the key and the value is set to an
+# exist in |pair|, then the entire |pair| is the key and the value is set to an
 # empty string.
-def KeyValuePair(str, sep='='):
-  if sep in str:
-    return str.split(sep)
+def KeyValuePair(pair, sep='='):
+  if sep in pair:
+    return pair.split(sep)
   else:
-    return [str, '']
+    return [pair, '']
 
 
 # A small handler that looks for '?quit=1' query in the path and shuts itself
@@ -103,28 +106,28 @@ class QuittableHTTPHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
     path = self.translate_path(self.path)
     f = None
     if os.path.isdir(path):
-        if not self.path.endswith('/'):
-            # redirect browser - doing basically what apache does
-            self.send_response(301)
-            self.send_header("Location", self.path + "/")
-            self.end_headers()
-            return None
-        for index in "index.html", "index.htm":
-            index = os.path.join(path, index)
-            if os.path.exists(index):
-                path = index
-                break
-        else:
-            return self.list_directory(path)
+      if not self.path.endswith('/'):
+        # redirect browser - doing basically what apache does
+        self.send_response(301)
+        self.send_header("Location", self.path + "/")
+        self.end_headers()
+        return None
+      for index in "index.html", "index.htm":
+        index = os.path.join(path, index)
+        if os.path.exists(index):
+          path = index
+          break
+      else:
+        return self.list_directory(path)
     ctype = self.guess_type(path)
     try:
-        # Always read in binary mode. Opening files in text mode may cause
-        # newline translations, making the actual size of the content
-        # transmitted *less* than the content-length!
-        f = open(path, 'rb')
+      # Always read in binary mode. Opening files in text mode may cause
+      # newline translations, making the actual size of the content
+      # transmitted *less* than the content-length!
+      f = open(path, 'rb')
     except IOError:
-        self.send_error(404, "File not found")
-        return None
+      self.send_error(404, "File not found")
+      return None
     self.send_response(200)
     self.send_header("Content-type", ctype)
     fs = os.fstat(f.fileno())
@@ -156,7 +159,6 @@ class QuittableHTTPHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
     SimpleHTTPServer.SimpleHTTPRequestHandler.do_GET(self)
 
   def do_POST(self):
-    (_, _,path, query, _) = urlparse.urlsplit(self.path)
     if 'Content-Length' in self.headers:
       if not NACL_SDK_ROOT:
         self.wfile('Could not find NACL_SDK_ROOT to decode trace.')
diff --git a/native_client_sdk/src/tools/create_nmf.py b/native_client_sdk/src/tools/create_nmf.py
index 195c553..ba5f1e4 100755
--- a/native_client_sdk/src/tools/create_nmf.py
+++ b/native_client_sdk/src/tools/create_nmf.py
@@ -118,7 +118,7 @@ class NmfUtils(object):
 
   def __init__(self, main_files=None, objdump='x86_64-nacl-objdump',
                lib_path=None, extra_files=None, lib_prefix=None,
-               toolchain=None, remap={}):
+               toolchain=None, remap=None):
     '''Constructor
 
     Args:
@@ -142,7 +142,7 @@ class NmfUtils(object):
     self.needed = None
     self.lib_prefix = lib_prefix or []
     self.toolchain = toolchain
-    self.remap = remap
+    self.remap = remap or {}
 
 
   def GleanFromObjdump(self, files):
@@ -203,10 +203,10 @@ class NmfUtils(object):
     Returns:
       A list of system paths that match the given name within the lib_path'''
     files = []
-    for dir in self.lib_path:
-      file = os.path.join(dir, name)
-      if os.path.exists(file):
-        files.append(file)
+    for dirname in self.lib_path:
+      filename = os.path.join(dirname, name)
+      if os.path.exists(filename):
+        files.append(filename)
     if not files:
       raise Error('cannot find library %s' % name)
     return files
@@ -227,7 +227,7 @@ class NmfUtils(object):
     if runnable:
       examined = set()
       all_files, unexamined = self.GleanFromObjdump(
-          dict([(file, None) for file in self.main_files]))
+          dict([(f, None) for f in self.main_files]))
       for name, arch_file in all_files.items():
         arch_file.url = name
         if unexamined:
@@ -430,7 +430,7 @@ def Main(argv):
                  toolchain=options.toolchain,
                  remap=remap)
 
-  manifest = nmf.GetManifest()
+  nmf.GetManifest()
   if options.output is None:
     sys.stdout.write(nmf.GetJson())
   else:
diff --git a/native_client_sdk/src/tools/getos.py b/native_client_sdk/src/tools/getos.py
index b16870d..8bfd2fe 100755
--- a/native_client_sdk/src/tools/getos.py
+++ b/native_client_sdk/src/tools/getos.py
@@ -17,7 +17,7 @@ import subprocess
 import sys
 
 
-TOOL_PATH=os.path.dirname(os.path.abspath(__file__))
+TOOL_PATH = os.path.dirname(os.path.abspath(__file__))
 
 
 def ErrOut(text):
@@ -59,10 +59,10 @@ def GetSystemArch(platform):
 
   if platform in ['mac', 'linux']:
     try:
-      pobj = subprocess.Popen(['uname', '-m'],stdout= subprocess.PIPE)
-      arch, serr = pobj.communicate()
+      pobj = subprocess.Popen(['uname', '-m'], stdout= subprocess.PIPE)
+      arch = pobj.communicate()[0]
       arch = arch.split()[0]
-    except:
+    except Exception:
       arch = None
   return arch
 
@@ -82,13 +82,13 @@ def GetChromeArch(platform):
       pobj = subprocess.Popen(['objdump', '-f', chrome_path],
                               stdout=subprocess.PIPE,
                               stderr=subprocess.PIPE)
-      arch, serr = pobj.communicate()
-      format = re.compile(r'(file format) ([a-zA-Z0-9_\-]+)')
-      arch = format.search(arch).group(2)
+      arch = pobj.communicate()[0]
+      file_format = re.compile(r'(file format) ([a-zA-Z0-9_\-]+)')
+      arch = file_format.search(arch).group(2)
       if '64' in arch:
         return 'x86_64'
       return 'x86_32'
-    except:
+    except Exception:
       print "FAILED"
       arch = None
   return arch
diff --git a/native_client_sdk/src/tools/oshelpers.py b/native_client_sdk/src/tools/oshelpers.py
index fb2f3eb..253afa7 100755
--- a/native_client_sdk/src/tools/oshelpers.py
+++ b/native_client_sdk/src/tools/oshelpers.py
@@ -22,8 +22,8 @@ def IncludeFiles(filters, files):
   if not filters: 
     return files
   match = set()
-  for filter in filters:
-    match |= set(fnmatch.filter(files, filter))
+  for file_filter in filters:
+    match |= set(fnmatch.filter(files, file_filter))
   return [name for name in files if name in match]
 
 
@@ -35,8 +35,8 @@ def ExcludeFiles(filters, files):
   if not filters: 
     return files
   match = set()
-  for filter in filters:
-    excludes = set(fnmatch.filter(files, filter))
+  for file_filter in filters:
+    excludes = set(fnmatch.filter(files, file_filter))
     match |= excludes
   return [name for name in files if name not in match]
 
@@ -163,7 +163,7 @@ def Mkdir(args):
       print 'mkdir ' + dst
     try:
       os.makedirs(dst)
-    except OSError as error:
+    except OSError:
       if os.path.isdir(dst):
         if options.parents:
           continue
@@ -192,7 +192,7 @@ def MovePath(options, src, dst):
         raise OSError('mv: FAILED TO REMOVE ' + dst)
     else:
       raise OSError('mv: already exists ' + dst)
-  for i in range(5):
+  for _ in range(5):
     try:
       os.rename(src, dst)
       return
@@ -216,12 +216,13 @@ def Move(args):
   options, files = parser.parse_args(args)
   if len(files) < 2:
     parser.error('ERROR: expecting SOURCE... and DEST.')
-  if options.verbose:
-    print 'mv %s %s' % (src, dst)
 
   srcs = files[:-1]
   dst = files[-1]
 
+  if options.verbose:
+    print 'mv %s %s' % (' '.join(srcs), dst)
+
   for src in srcs:
     MovePath(options, src, dst)
   return 0
@@ -366,10 +367,10 @@ def Zip(args):
       src_files.append(src_file)
       if options.recursive and os.path.isdir(src_file):
         for root, dirs, files in os.walk(src_file):
-          for dir in dirs:
-            src_files.append(os.path.join(root, dir))
-          for file in files:
-            src_files.append(os.path.join(root, file))
+          for dirname in dirs:
+            src_files.append(os.path.join(root, dirname))
+          for filename in files:
+            src_files.append(os.path.join(root, filename))
 
   zip_stream = None
   # zip_data represents a list of the data to be written or appended to the
diff --git a/native_client_sdk/src/tools/zip_test.py b/native_client_sdk/src/tools/zip_test.py
index 76dffba..f3c7dd2 100755
--- a/native_client_sdk/src/tools/zip_test.py
+++ b/native_client_sdk/src/tools/zip_test.py
@@ -95,8 +95,8 @@ class TestZip(unittest.TestCase):
     self.assertEqual(self.zipfile.namelist()[0], file1)
 
   def testAddFilesWithGlob(self):
-    file1 = self.MakeFile('file1', 1024)
-    file2 = self.MakeFile('file2', 3354)
+    self.MakeFile('file1', 1024)
+    self.MakeFile('file2', 3354)
     self.RunZip([self.zipname, 'file*'])
     self.OpenZipFile()
     self.assertEqual(len(self.zipfile.namelist()), 2)
@@ -155,7 +155,7 @@ class TestZip(unittest.TestCase):
     os.mkdir(self.GetTempPath('dir1'))
     file1 = self.MakeFile(os.path.join('dir1', 'file1'), 256)
     os.mkdir(self.GetTempPath(os.path.join('dir1', 'dir2')))
-    file2 = self.MakeFile(os.path.join('dir1', 'dir2', 'file2'), 1234)
+    self.MakeFile(os.path.join('dir1', 'dir2', 'file2'), 1234)
     self.RunZip([self.zipname, '-r', 'dir1'])
     self.OpenZipFile()
     self.assertEqual(len(self.zipfile.namelist()), 4)
@@ -189,5 +189,5 @@ def main():
   return int(not result.wasSuccessful())
 
 
-if __name__=='__main__':
+if __name__ == '__main__':
   sys.exit(main())