Attempt to fix the parser caching in the IDL compiler.

PLY has a feature where it'll load a generated lexer
table by importing it as a python module. It turns out
that we were being inconsistent in how we were generating
these modules and that could lead to incomplete builds depending
on when the various IDL compilation targets ran.

This CL attempts to be more consistent about the generation
of the cached files to eliminate that problem, by requiring
the scripts to consistently pass in the location of the cached
files. We can't really fix this without doing more serious
surgery on PLY (or working around it) to be more deterministic
in how the cached file is imported.

R=dcheng@chromium.org, haraken@chromium.org
BUG=1022647

Change-Id: I141059341634633f0df33848638eecc432aa764c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903938Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713872}

third_party/blink/renderer/bindings/BUILD.gn

@@ -66,11 +66,16 @@ template("collect_idl_files") {
       invoker.component,
       "--output",
       rebase_path(invoker.output, root_build_dir),
+      "--cached-parser-tables-dir",
+      rebase_path(bindings_scripts_output_dir, root_build_dir),
     ]
 
     if (defined(invoker.deps)) {
       deps = invoker.deps
+    } else {
+      deps = []
     }
+    deps += [ "scripts:cached_lex_yacc_tables" ]
   }
 }
 

third_party/blink/renderer/bindings/scripts/blink_idl_lexer.py

@@ -79,10 +79,22 @@ class BlinkIDLLexer(IDLLexer):
             # Turn off optimization and caching to help debugging
             optimize = False
             outputdir = None
+
+        # Ensure that if we are writing tables, we got a real outputdir.
+        # Because of the way PLY loads the lexer (via import), it can
+        # be loaded from anywhere in sys.path; requiring an outputdir
+        # can help minimize the likelihood of inconsistencies between
+        # scripts.
+        assert debug or outputdir
+
         if outputdir:
             # Need outputdir in path because lex imports the cached lex table
-            # as a Python module
-            sys.path.append(outputdir)
+            # as a Python module. Putting this at the front of sys.path
+            # helps ensures that we get the one in outputdir, and not some
+            # module by the same name that might exist elsewhere in path,
+            # though it won't guarantee it.
+            if sys.path[0] != outputdir:
+                sys.path.insert(0, outputdir)
 
             if rewrite_tables:
                 tablefile_root = os.path.join(outputdir, LEXTAB)

third_party/blink/renderer/bindings/scripts/blink_idl_parser.py

@@ -93,6 +93,14 @@ class BlinkIDLParser(IDLParser):
             outputdir = None
             picklefile = None
             write_tables = True
+
+        # Ensure that if we are writing tables, we got a real outputdir.
+        # Because of the way PLY loads the lexer (via import), it can
+        # be loaded from anywhere in sys.path; requiring an outputdir
+        # can help minimize the likelihood of inconsistencies between
+        # scripts.
+        assert debug or not write_tables or outputdir
+
         if outputdir:
             picklefile = picklefile or os.path.join(outputdir, 'parsetab.pickle')
             if rewrite_tables:

third_party/blink/renderer/bindings/scripts/blink_idl_parser_test.py

@@ -16,6 +16,7 @@ class BlinkIDLParserTest(unittest.TestCase):
     def test_missing_semicolon_between_definitions(self):
         # No semicolon after enum definition.
         text = '''enum TestEnum { "value" } dictionary TestDictionary {};'''
-        parser = BlinkIDLParser()
+        # We use debug=True to not have to worry about generated parser tables.
+        parser = BlinkIDLParser(debug=True)
         parser.ParseText(filename='', data=text)
         self.assertGreater(parser.GetErrors(), 0)

third_party/blink/renderer/bindings/scripts/collect_idl_files.py

@@ -28,6 +28,8 @@ def parse_options():
                       help="specify a component name")
     parser.add_option('--output', type='string',
                       help="the output file path")
+    parser.add_option('--cached-parser-tables-dir',
+                      help='location of the generated LEX and YACC files')
     options, args = parser.parse_args()
 
     if options.idl_list_file is None:
@@ -36,6 +38,9 @@ def parse_options():
         parser.error("Specify the output file path with --output.")
     if options.component is None:
         parser.error("Specify a component with --component.")
+    if options.cached_parser_tables_dir is None:
+        parser.error("Specify the location of the generated "
+                     "LEX and YACC files with --cached-parser-tables-dir.")
 
     if args:
         parser.error("Unknown arguments {}".format(args))
@@ -47,7 +52,7 @@ def main():
     options, _ = parse_options()
 
     filepaths = utilities.read_idl_files_list_from_file(options.idl_list_file)
-    parser = blink_idl_parser.BlinkIDLParser()
+    parser = blink_idl_parser.BlinkIDLParser(outputdir=options.cached_parser_tables_dir)
     ast_group = web_idl.AstGroup(web_idl.Component(options.component))
     for filepath in filepaths:
         ast_group.add_ast_node(blink_idl_parser.parse_file(parser, filepath))

third_party/blink/renderer/bindings/scripts/idl_reader.py

@@ -81,7 +81,7 @@ def validate_blink_idl_definitions(idl_filename, idl_file_basename,
 
 
 class IdlReader(object):
-    def __init__(self, interfaces_info=None, outputdir=''):
+    def __init__(self, interfaces_info=None, outputdir='', debug=False):
         self.extended_attribute_validator = IDLExtendedAttributeValidator()
         self.interfaces_info = interfaces_info
 
@@ -90,7 +90,7 @@ class IdlReader(object):
         else:
             self.interface_dependency_resolver = None
 
-        self.parser = BlinkIDLParser(outputdir=outputdir)
+        self.parser = BlinkIDLParser(outputdir=outputdir, debug=debug)
 
     def read_idl_definitions(self, idl_filename):
         """Returns a dictionary whose key is component and value is an IdlDefinitions object for an IDL file, including all dependencies."""

third_party/blink/tools/blinkpy/bindings/bindings_tests.py

@@ -105,7 +105,7 @@ def TemporaryDirectory():
         shutil.rmtree(name)
 
 
-def generate_interface_dependencies(runtime_enabled_features):
+def generate_interface_dependencies(runtime_enabled_features, cache_directory):
     def idl_paths_recursive(directory):
         # This is slow, especially on Windows, due to os.walk making
         # excess stat() calls. Faster versions may appear in Python 3.5 or
@@ -127,7 +127,7 @@ def generate_interface_dependencies(runtime_enabled_features):
         return idl_paths
 
     def collect_interfaces_info(idl_path_list):
-        info_collector = InterfaceInfoCollector()
+        info_collector = InterfaceInfoCollector(cache_directory)
         for idl_path in idl_path_list:
             info_collector.collect_info(idl_path)
         info = info_collector.get_info_as_dict()
@@ -208,6 +208,9 @@ def bindings_tests(output_directory, verbose, suppress_diff):
                 continue
             directory_with_component = os.path.join(directory, component)
             for filename in os.listdir(directory_with_component):
+                if filename in ('lextab.py', 'parsetab.pickle'):
+                    # Ignore tempfiles from PLY.
+                    continue
                 files.append(os.path.join(directory_with_component, filename))
         return files
 
@@ -288,7 +291,10 @@ def bindings_tests(output_directory, verbose, suppress_diff):
         return features_map
 
     try:
-        generate_interface_dependencies(make_runtime_features_dict())
+        output_dir = os.path.join(output_directory, 'third_party/blink/renderer/bindings/tests/results')
+        if not os.path.exists(output_dir):
+            os.makedirs(output_dir)
+        generate_interface_dependencies(make_runtime_features_dict(), output_directory)
         for component in COMPONENT_DIRECTORY:
             output_dir = os.path.join(output_directory, component)
             if not os.path.exists(output_dir):
@@ -297,7 +303,7 @@ def bindings_tests(output_directory, verbose, suppress_diff):
             options = IdlCompilerOptions(
                 output_directory=output_dir,
                 impl_output_directory=output_dir,
-                cache_directory=None,
+                cache_directory=output_dir,
                 target_component=component)
 
             if component == 'core':
@@ -308,7 +314,7 @@ def bindings_tests(output_directory, verbose, suppress_diff):
                 partial_interface_options = IdlCompilerOptions(
                     output_directory=partial_interface_output_dir,
                     impl_output_directory=None,
-                    cache_directory=None,
+                    cache_directory=output_dir,
                     target_component='modules')
 
             idl_filenames = []

third_party/blink/tools/blinkpy/bindings/collect_idls_into_json.py

@@ -65,7 +65,8 @@ def get_definitions(paths):
     Returns:
       a generator which yields IDL node objects
     """
-    parser = BlinkIDLParser()
+    # We use debug=True to not have to worry about generated parser tables.
+    parser = BlinkIDLParser(debug=True)
     for path in paths:
         definitions = parse_file(parser, path)
         for definition in definitions.GetChildren():

third_party/blink/tools/blinkpy/bindings/collect_idls_into_json_test.py

@@ -35,7 +35,8 @@ _PARTIAL = {
 
 class TestFunctions(unittest.TestCase):
     def setUp(self):
-        parser = BlinkIDLParser()
+        # We use debug=True to not have to worry about generated parser tables.
+        parser = BlinkIDLParser(debug=True)
         path = os.path.join(
             testdata_path, utilities.read_file_to_list(_FILE)[0])
         definitions = parse_file(parser, path)