action_derivedsourcesallinone.py: Fix Python style and conditional bug

Source/build/scripts/action_derivedsourcesallinone.py

Index: Source/build/scripts/action_derivedsourcesallinone.py
diff --git a/Source/build/scripts/action_derivedsourcesallinone.py b/Source/build/scripts/action_derivedsourcesallinone.py
index fec5088917a9e24775662d03a2aa2a5b268422d1..fadc61ef685977ea853853f352d2a55c05cb2c0b 100644
--- a/Source/build/scripts/action_derivedsourcesallinone.py
+++ b/Source/build/scripts/action_derivedsourcesallinone.py
@@ -43,15 +43,14 @@
 
 import errno
 import os
-import os.path
 import re
 import subprocess
 import sys
 
 # A regexp for finding Conditional attributes in interface definitions.
-conditionalPattern = re.compile('interface[\s]*\[[^\]]*Conditional=([\_0-9a-zA-Z&|]*)')
+CONDITIONAL_PATTERN = re.compile('\[[^\]]*Conditional=([\_0-9a-zA-Z&|]*)[^\]]\]\s*interface', re.MULTILINE)
 
-copyrightTemplate = """/*
+COPYRIGHT_TEMPLATE = """/*
  * THIS FILE WAS AUTOMATICALLY GENERATED, DO NOT EDIT.
  *
  * This file was generated by the action_derivedsourcesallinone.py script.
@@ -82,141 +81,131 @@ copyrightTemplate = """/*
 """
 
 
-# Wraps conditional with ENABLE() and replace '&','|' with '&&','||' if more than one conditional is specified.
-def formatConditional(conditional):
-    def wrapWithEnable(s):
-        if re.match('[|&]$', s):
+def format_conditional(conditional):
+    """Wraps conditional with ENABLE() and replace '&','|' with '&&','||' if
+    more than one conditional is specified."""
+    def wrap_with_enable(s):
+        if s in ['|', '&']:
             return s * 2
         return 'ENABLE(' + s + ')'
-    return ' '.join(map(wrapWithEnable, conditional))
+    return ' '.join(map(wrap_with_enable, conditional))
 
 
-# Find the conditional interface attribute.
-def extractConditional(idlFilePath):
-    conditional = None
+def extract_conditional(idl_file_path):
+    """Find the conditional interface attribute."""
 
-    # Read file and look for "interface [ Conditional=XXX ]".
-    idlFile = open(idlFilePath)
-    idlContents = idlFile.read().replace('\n', '')
-    idlFile.close()
+    # Read file and look for [Conditional=XXX] interface.
+    with open(idl_file_path) as idl_file:
+        idl_contents = idl_file.read()
 
-    match = conditionalPattern.search(idlContents)
-    if match:
-        conditional = match.group(1)
-        conditional = re.split('([|&])', conditional)
+    match = CONDITIONAL_PATTERN.search(idl_contents)
+    if not match:
+        return None
+    conditional = match.group(1)
+    return re.split('([|&])', conditional)
 
-    return conditional
 
-# Extracts conditional and interface name from each IDL file.
-def extractMetaData(filePaths):
-    metaDataList = []
+def extract_meta_data(file_paths):
+    """Extracts conditional and interface name from each IDL file."""
+    meta_data_list = []
 
-    for f in filePaths:
-        metaData = {}
-        if len(f) == 0:
+    for file_path in file_paths:
+        if not file_path.endswith('.idl'):
+            print 'WARNING: non-IDL file passed: "%s"' % file_path
             continue
-        if not os.path.exists(f):
-            print 'WARNING: file not found: "%s"' % f
+        if not os.path.exists(file_path):
+            print 'WARNING: file not found: "%s"' % file_path
             continue
 
-        # Extract type name from file name
-        (parentPath, fileName) = os.path.split(f)
-        (interfaceName, ext) = os.path.splitext(fileName)
+        # Extract interface name from file name
+        parent_path, file_name = os.path.split(file_path)
+        interface_name, _ = os.path.splitext(file_name)
 
-        if not ext == '.idl':
-            continue
-
-        metaData = {
-            'conditional': extractConditional(f),
-            'name': interfaceName,
+        meta_data = {
+            'conditional': extract_conditional(file_path),
+            'name': interface_name,
         }
+        meta_data_list.append(meta_data)
 
-        metaDataList.append(metaData)
-
-    return metaDataList
-
-
-def generateContent(filesMetaData, partition, totalPartitions):
-    # Sort files by conditionals.
-    filesMetaData.sort()
+    return meta_data_list
 
-    output = []
 
+def generate_content(files_meta_data_this_partition):
     # Add fixed content.
-    output.append(copyrightTemplate)
-    output.append('#define NO_IMPLICIT_ATOMICSTRING\n\n')
+    output = [COPYRIGHT_TEMPLATE,
+              '#define NO_IMPLICIT_ATOMICSTRING\n\n']
 
     # List all includes segmented by if and endif.
-    prevConditional = None
-    for metaData in filesMetaData:
-        name = metaData['name']
-        if (hash(name) % totalPartitions) != partition:
-            continue
-        conditional = metaData['conditional']
-
-        if prevConditional and prevConditional != conditional:
-            output.append('#endif\n')
-        if conditional and prevConditional != conditional:
-            output.append('\n#if %s\n' % formatConditional(conditional))
-
-        output.append('#include "bindings/V8%s.cpp"\n' % name)
-
-        prevConditional = conditional
-
-    if prevConditional:
+    prev_conditional = None
+    files_meta_data_this_partition.sort(key=lambda e: e['conditional'])
+    for meta_data in files_meta_data_this_partition:
+        conditional = meta_data['conditional']
+        if prev_conditional != conditional:
+            if prev_conditional:
+                output.append('#endif\n')
+            if conditional:
+                output.append('\n#if %s\n' % format_conditional(conditional))
+        prev_conditional = conditional
+
+        output.append('#include "bindings/V8%s.cpp"\n' % meta_data['name'])
+
+    if prev_conditional:
         output.append('#endif\n')
 
     return ''.join(output)
 
 
-def writeContent(content, outputFileName):
-    (parentPath, fileName) = os.path.split(outputFileName)
-    if not os.path.exists(parentPath):
-        print parentPath
-        os.mkdir(parentPath)
-    f = open(outputFileName, 'w')
-    f.write(content)
-    f.close()
+def write_content(content, output_file_name):
+    parent_path, file_name = os.path.split(output_file_name)
+    if not os.path.exists(parent_path):
+        print parent_path
+        os.mkdir(parent_path)

[eseidel, 2013/10/10 14:44:47]

This probably won't do what the author intends. Mktree would though 

[Nils Barth (inactive), 2013/10/11 01:58:37]

Good point (rather, os.makedirs).
This making directories is probably unnecessary
(on Linux it's the dir is always gen/blink/bindings),
so we could just open the file and remove this function.
However, it might be needed on Windows, so leaving for now.

+    with open(output_file_name, 'w') as f:
+        f.write(content)
 
 
-def resolveCygpath(cygdriveNames):
+def resolve_cygpath(cygdrive_names):
     cmd = ['cygpath', '-f', '-', '-wa']
     process = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    idlFileNames = []
-    for fileName in cygdriveNames:
-        process.stdin.write("%s\n" % fileName)
+    idl_file_names = []
+    for file_name in cygdrive_names:
+        process.stdin.write('%s\n' % file_name)
         process.stdin.flush()
-        idlFileNames.append(process.stdout.readline().rstrip())
+        idl_file_names.append(process.stdout.readline().rstrip())
     process.stdin.close()
     process.wait()
-    return idlFileNames
+    return idl_file_names
 
 
 def main(args):
-    assert(len(args) > 3)
-    inOutBreakIndex = args.index('--')
-    inputFileName = args[1]
-    outputFileNames = args[inOutBreakIndex+1:]
-
-    inputFile = open(inputFileName, 'r')
-    idlFileNames = []
-    cygdriveNames = []
-    for line in inputFile:
-        idlFileName = line.rstrip().split(' ')[0]
-        if idlFileName.startswith("/cygdrive"):
-            cygdriveNames.append(idlFileName)
-        else:
-            idlFileNames.append(idlFileName)
-
-    if cygdriveNames:
-        idlFileNames.extend(resolveCygpath(cygdriveNames))
-    inputFile.close()
-
-    filesMetaData = extractMetaData(idlFileNames)
-    for fileName in outputFileNames:
-        partition = outputFileNames.index(fileName)
-        fileContents = generateContent(filesMetaData, partition, len(outputFileNames))
-        writeContent(fileContents, fileName)
+    if len(args) <= 3:
+        raise 'Expected at least 4 arguments.'
+    in_out_break_index = args.index('--')
+    input_file_name = args[1]
+    output_file_names = args[in_out_break_index + 1:]
+
+    with open(input_file_name) as input_file:
+        idl_file_names = []
+        cygdrive_names = []
+        for line in input_file:
+            idl_file_name = line.rstrip().split(' ')[0]
+            if idl_file_name.startswith('/cygdrive'):
+                cygdrive_names.append(idl_file_name)
+            else:
+                idl_file_names.append(idl_file_name)
+
+        if cygdrive_names:
+            idl_file_names.extend(resolve_cygpath(cygdrive_names))
+
+    files_meta_data = extract_meta_data(idl_file_names)
+    total_partitions = len(output_file_names)
+    for partition, file_name in enumerate(output_file_names):
+        files_meta_data_this_partition = [
+                meta_data
+                for meta_data in files_meta_data
+                if hash(meta_data['name']) % total_partitions == partition]
+        file_contents = generate_content(files_meta_data_this_partition)
+        write_content(file_contents, file_name)
 
     return 0