Adding a script to generate au-geneate.zip file

generate_au_zip.py

+#!/usr/bin/python
+# -*- coding: utf-8 -*-

[sosa, 2010/11/08 23:57:56]

Still don't need this

+
+# Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""
+  This script is to generate a zip file of delta-generator and it's dependencies

[scottz-goog, 2010/11/09 03:52:50]

No spaces needed here and might as well end with a period.

+"""
+
+import logging.handlers
+import optparse
+import os
+import re
+import shutil
+import subprocess
+import sys
+import tempfile
+
+# GLOBALS
+
+logging_format = '%(asctime)s - %(filename)s - %(levelname)-8s: %(message)s'
+date_format = '%Y/%m/%d %H:%M:%S'
+logging.basicConfig(level=logging.INFO, format=logging_format,
+                    datefmt=date_format)
+
+def CreateTempDir():
+  """Creates a tempdir and returns the name of the tempdir.
+  """

[sosa, 2010/11/08 23:57:56]

Preference is to save whitespace ... so if """docstring.""" fits it one line
than you should have it that way.

+  temp_dir = tempfile.mkdtemp(suffix='au', prefix='tmp')
+  logging.info('Using tempdir = %s', temp_dir)
+  return temp_dir
+
+
+def CopyRequiredFiles(dest_files_root):
+  """
+    Generates a list of files that are required for au-generator zip file
+    Args:
+      dest_files_root: location of the directory where we should copy the files
+  """
+  if not dest_files_root:
+    logging.error('Invalid option passed for dest_files_root')
+    sys.exit(1)
+  # Files that need to go through ldd
+  ldd_files = ['/usr/bin/delta_generator', '/usr/bin/bsdiff',
+               '/usr/bin/bspatch', '/usr/bin/cgpt']
+  # statically linked files and scripts etc.,

[scottz-goog, 2010/11/09 03:52:50]

Is there any other general name we can give these? support_programs it just
seems like a misnomer to call the list static_files. I know this is nit picking
so i'll leave it up to you but two of the 3 files are scripts not statically
linked files.

[Raja Aluri, 2010/11/09 22:59:45]

Lack of a better name, I can call it non_ldd_files.

+  static_files = ['~/trunk/src/scripts/common.sh',
+                  '~/trunk/src/scripts/cros_generate_update_payload',
+                  '~/trunk/src/scripts/chromeos-common.sh']
+  # We need directories to be copied recursively to a destination within tempdir
+  recurse_dirs = {'~/trunk/src/scripts/lib/shflags': 'lib/shflags'}

[scottz-goog, 2010/11/09 03:52:50]

Is this ever going to be more than this one path? Right now we do a for loop
over it and it has one item.

[Raja Aluri, 2010/11/09 22:59:45]

For now it's only one directory, but in the future if you want to add one more,
you don't need to change the code for copying

+
+  black_list = [
+    'linux-vdso.so',
+    'libgcc_s.so',
+    'librt.so',
+    'libstdc',
+    'libgcc_s.so',
+    'libc.so',
+    'ld-linux-x86-64',
+    'libm.so',
+    'libdl.so',
+    'libresolv.so',
+    ]
+
+  all_files = ldd_files + static_files
+  all_files = map(os.path.expanduser, all_files)
+
+  for file_name in all_files:
+    if not os.path.isfile(file_name):
+      logging.error('file = %s does not exist', file_name)
+      sys.exit(1)
+
+  logging.debug('Given files that need to be copied = %s', all_files)

[scottz-goog, 2010/11/09 03:52:50]

' '.join(all_files) ? This avoids putting a list output in the log message.

+  all_files += DepsToCopy(ldd_files=ldd_files,black_list=black_list)
+  for file_name in all_files:
+    logging.info('Copying file  %s to %s', file_name, dest_files_root)
+    shutil.copy2(file_name, dest_files_root)
+
+  for (source_dir, target_dir) in recurse_dirs.iteritems():

[scottz-goog, 2010/11/09 03:52:50]

no parans necessary here others can disagree but in my experience it is
generally written as for source_dir, target_dir in ....

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+    logging.info('Processing directory %s', source_dir)
+    full_path = os.path.expanduser(source_dir)
+    if not os.path.isdir(full_path):
+      logging.error("Directory given for %s expanded to %s doens't exist.",
+                    source_dir, full_path)
+      sys.exit(1)
+  dest = os.path.join(dest_files_root, target_dir)
+  logging.info('Copying directory %s to %s.', full_path, target_dir)
+  shutil.copytree(full_path, dest, symlinks=None, ignore=None)

[scottz-goog, 2010/11/09 03:52:50]

shutil.copytree(src, dst, symlinks=False, ignore=None)

I assume you want symlinks=False, it looks like we are using the defaults lets
just drop the added options.

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+
+
+def DepsToCopy(ldd_files,black_list):

[scottz-goog, 2010/11/09 03:52:50]

Move this above any function that calls it please. It is a general convention
that keeps the flow of the file consistent. 

[scottz-goog, 2010/11/09 03:52:50]

(ldd_files, black_list)

[Raja Aluri, 2010/11/09 22:59:45]

Done.

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+  """
+    Returns a list of deps for a given dynamic executables list.
+    Args:
+      ldd_files: List of dynamic files that needs to have the deps evaluated
+      black_list: List of files that we should ignore
+   Returns:
+     library_list: List of files that are dependencies
+  """
+  for file_name in ldd_files:
+    logging.info('Running ldd on %s', file_name)
+    cmd = ['/usr/bin/ldd', file_name]
+    stdout_data = ''
+    stderr_data = ''
+
+    try:
+      proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE)
+      (stdout_data, stderr_data) = proc.communicate(input=None)
+    except subprocess.CalledProcessError, e:
+      logging.error('Command %s failed', cmd)
+      logging.error('error code %s', e.returncode)
+      logging.error('ouput %s', e.output)
+      raise
+
+    library_list = []
+
+    if stdout_data:

[scottz-goog, 2010/11/09 03:52:50]

if not stdout_data:
   Error/Do whatever is proper.

Then remove all the other if checks as they should pass assuming we have a
stdout_data. 

+      logging.debug('ldd for %s = stdout = %s stderr =%s', file_name,
+                    stdout_data, stderr_data)
+      library_list = _SplitAndStrip(stdout_data)
+
+    if library_list:
+      library_list = _ExcludeBlacklist(library_list, black_list)
+
+    if library_list:
+      logging.info('Required libraries for %s=%s=', file_name, library_list)
+
+    return library_list
+
+
+def CleanUp(temp_dir):
+  """
+    Cleans up the tempdir
+    Args:
+      temp_dir = name of the directory to cleanup
+  """
+  if not temp_dir:
+    return
+  shutil.rmtree(temp_dir, ignore_errors=True)
+  logging.info('Removed tempdir = %s', temp_dir)
+
+
+def GenerateZipFile(base_name, root_dir):
+  """Returns true if able to generate zip file
+    Args:
+      base_name: name of the zip file
+      root_dir: location of the directory that we should zip
+    Returns:
+      True

[sosa, 2010/11/08 23:57:56]

True if?

+  """
+  logging.info('Generating zip file %s.zip with contents from %s', base_name,
+               root_dir)
+  current_dir = os.getcwd()
+  os.chdir(root_dir)
+  try:
+    output = subprocess.Popen(['zip', '-r', '-9', base_name, '.'],
+                              stdout=subprocess.PIPE).communicate()[0]
+  except OSError, e:
+   logging.error('Execution failed:%s', e.strerror)

[sosa, 2010/11/08 23:57:56]

If you don't use the output variable (output = above) then you shouldn't set it.

+   return False
+  finally:
+    os.chdir(current_dir)
+
+  return True
+
+
+def _ExcludeBlacklist(library_list, black_list):

[sosa, 2010/11/08 23:57:56]

blacklist=[] as default up here?

+  """
+    Deletes the set of files from black_list from the library_list
+    Args:
+      library_list: List of the library names to filter through black_list
+      black_list: List of the black listed names to filter
+    Returns:
+      Filtered library_list
+  """
+  return_list = []
+  pattern = re.compile(r'%s' % '|'.join(black_list))

[scottz-goog, 2010/11/09 03:52:50]

r'|'.join(black_list)?

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+
+  logging.debug('PATTERN: %s=', pattern)
+
+  for library in library_list:
+    if pattern.search(library):
+      logging.debug('BLACK-LISTED = %s=', library)
+      continue
+    return_list.append(library)
+
+  logging.debug('Returning return_list=%s=', return_list)
+
+  return return_list
+
+
+def _SplitAndStrip(data=None):

[scottz-goog, 2010/11/09 03:52:50]

Should be above any functions that call it.

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+  """
+    Prunes the ldd output, so that we can get a meaningful list of library names
+    Example of data:
+        linux-vdso.so.1 =>  (0x00007ffffc96a000)
+        libbz2.so.1 => /lib/libbz2.so.1 (0x00007f3ff8782000)
+        libc.so.6 => /lib/libc.so.6 (0x00007f3ff83ff000)
+        /lib64/ld-linux-x86-64.so.2 (0x00007f3ff89b3000)
+    Args:
+      data: list of libraries from ldd output
+    Returns:
+      list of libararies that we should copy
+
+  """
+  if not data:

[scottz-goog, 2010/11/09 03:52:50]

Why should this ever be called with a None value?

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+    return None
+  return_list = []
+  for line in data.split('\n'):
+    line = re.sub('.*not a dynamic executable.*', '', line)
+    line = re.sub('.* =>\s+', '', line)
+    line = re.sub('\(0x.*\)\s?', '', line)
+    line = line.strip()
+    if not len(line):
+      continue
+    logging.debug('MATCHED line = %s', line)
+    return_list.append(line)
+
+  return return_list
+
+
+def CopyZipToFinalDestination(output_dir, zip_file_name):
+  """
+  Copies the generated zip file to a final destination
+  Args:
+    output_dir: Directory where the file should be copied to
+    zip_file_name: name of the zip file that should be copied
+  Returns:
+    True on Success

[scottz-goog, 2010/11/09 03:52:50]

False if it fails...

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+  """
+  if not os.path.isfile(zip_file_name):
+    logging.error("Zip file %s doesn't exist. Returning False", zip_file_name)
+    return False
+
+  if not os.path.isdir(output_dir):
+    logging.debug('Creating %s', output_dir)
+    os.makedirs(output_dir)
+  logging.info('Copying %s to %s', zip_file_name, output_dir)
+  shutil.copy2(zip_file_name, output_dir)
+  return True
+
+
+def main():
+  """

[scottz-goog, 2010/11/09 03:52:50]

Docstring all on one line. 

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+  Main function to start the script
+  """
+  parser = optparse.OptionParser()
+
+  parser.add_option( '-d', '--debug', dest='debug', action='store_true',
+    default=False, help='Verbose Default: False',)

[scottz-goog, 2010/11/09 03:52:50]

('-d'
Also properly align default with the rest of the arguments. 

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+  parser.add_option('-o', '--output-dir', dest='output_dir',
+                    default='/tmp/au-generator.zip',
+                    help='Specify the output location for copying the zipfile')
+  parser.add_option('-z', '--zip-name', dest='zip_name',
+                    default='au-generator.zip', help='Name of the zip file')
+  parser.add_option('-k', '--keep-temp', dest='keep_temp', default=False,
+                    action='store_true', help='Keep the temp files...',)
+
+  (options, args) = parser.parse_args()
+  if options.debug:
+    logging.setLevel(logging.DEBUG)

[scottz-goog, 2010/11/09 03:52:50]

Generally for clarity it is good to separate if statements with spaces after
them.

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+  logging.debug('Options are %s ', options)
+
+  temp_dir = CreateTempDir()
+  dest_files_root = os.path.join(temp_dir, 'au-generator')
+  os.makedirs(dest_files_root)
+  CopyRequiredFiles(dest_files_root=dest_files_root)

[scottz-goog, 2010/11/09 03:52:50]

this is no longer a keyword option as defined above.

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+  zip_file_name = os.path.join(temp_dir, options.zip_name)
+  GenerateZipFile(base_name=zip_file_name, root_dir=dest_files_root)

[scottz-goog, 2010/11/09 03:52:50]

also no longer keyword options.

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+  CopyZipToFinalDestination(output_dir=options.output_dir,
+                            zip_file_name=zip_file_name)

[scottz-goog, 2010/11/09 03:52:50]

same..

+  if not options.keep_temp:
+    CleanUp(temp_dir=temp_dir)

[scottz-goog, 2010/11/09 03:52:50]

same..

[Raja Aluri, 2010/11/09 22:59:45]

Done.

+
+
+if __name__ == '__main__':
+  main()
+  sys.exit(0)

[scottz-goog, 2010/11/09 03:52:50]

sys.exit(0) is not necessary the exit code will be zero if you get here.

[Raja Aluri, 2010/11/09 22:59:45]

Done.