Improve generator logic for cached cores with file inputs

Generators with file_input_parameters stored a file with the hash of
the input files. If there was a mismatch, the cache was invalidated.

This changes the logic so that there is instead a sub directory created
with the hash of the input logic. This allows caching multiple cached results
instead of just keeping the most recent one.

Author: olofk

fusesoc/edalizer.py

@@ -581,17 +581,6 @@ def _sha256_file_input_hexdigest(self):
 
         return hash.hexdigest()
 
-    def _fwrite_hash(self, hashfile, data):
-        with open(hashfile, "w") as f:
-            f.write(data)
-
-    def _fread_hash(self, hashfile):
-        data = ""
-        with open(hashfile) as f:
-            data = f.read()
-
-        return data
-
     def _run(self, generator_cwd):
         logger.info("Generating " + str(self.vlnv))
 
@@ -633,6 +622,11 @@ def is_generator_cacheable(self):
     def is_cacheable(self):
         return self.is_input_cacheable() or self.is_generator_cacheable()
 
+    def acquire_cache_lock(self):
+        have_lock = False
+        # while not have_lock:
+        #    if
+
     def generate(self):
         """Run a parametrized generator
 
@@ -642,14 +636,24 @@ def generate(self):
 
         hexdigest = self._sha256_input_yaml_hexdigest()
 
-        logger.debug("Generator input yaml hash: " + hexdigest)
+        logger.debug("Generator parameters hash: " + hexdigest)
 
         generator_cwd = os.path.join(
             self.gen_root,
             "generator_cache",
             self.vlnv.sanitized_name + "-" + hexdigest,
         )
 
+        if "file_input_parameters" in self.generator:
+            # If file_input_parameters has been configured in the generator
+            # parameters will be iterated to look for files to add to the
+            # input files hash calculation.
+            file_input_hash = self._sha256_file_input_hexdigest()
+            generator_cwd = os.path.join(generator_cwd, file_input_hash)
+            logger.debug("Generator input files hash: " + file_input_hash)
+
+        logger.debug("Generator cwd: " + generator_cwd)
+
         if os.path.lexists(generator_cwd) and not os.path.isdir(generator_cwd):
             raise RuntimeError(
                 "Unable to create generator working directory since it already exists and is not a directory: "
@@ -658,59 +662,18 @@ def generate(self):
                 + "Remove it manually or run 'fusesoc gen clean'"
             )
 
-        if self.is_input_cacheable():
-            # Input cache enabled. Check if cached output already exists in generator_cwd.
-            logger.debug("Input cache enabled.")
-
-            # If file_input_parameters has been configured in the generator
-            # parameters will be iterated to look for files to add to the
-            # input files hash calculation.
-            if "file_input_parameters" in self.generator:
-                file_input_hash = self._sha256_file_input_hexdigest()
-
-                logger.debug("Generator file input hash: " + file_input_hash)
-
-                hashfile = os.path.join(generator_cwd, ".fusesoc_file_input_hash")
-
-                rerun = False
-
-                if os.path.isfile(hashfile):
-                    cached_hash = self._fread_hash(hashfile)
-                    logger.debug("Cached file input hash: " + cached_hash)
-
-                    if not file_input_hash == cached_hash:
-                        logger.debug("File input has changed.")
-                        rerun = True
-                    else:
-                        logger.info("Found cached output for " + str(self.vlnv))
-
-                else:
-                    logger.debug("File input hash file does not exist: " + hashfile)
-                    rerun = True
-
-                if rerun:
-                    shutil.rmtree(generator_cwd, ignore_errors=True)
-                    self._run(generator_cwd)
-                    self._fwrite_hash(hashfile, file_input_hash)
-
-            elif os.path.isdir(generator_cwd):
-                logger.info("Found cached output for " + str(self.vlnv))
-            else:
-                # No directory found. Run generator.
-                self._run(generator_cwd)
-
-        elif self.is_generator_cacheable():
-            # Generator cache enabled. Call the generator and let it
-            # decide if the old output still is valid.
-            logger.debug("Generator cache enabled.")
-            self._run(generator_cwd)
+        if self.is_input_cacheable() and os.path.isdir(generator_cwd):
+            logger.info("Found cached output for " + str(self.vlnv))
         else:
-            # No caching enabled. Try to remove directory if it already exists.
-            # This could happen if a generator that has been configured with
-            # caching is changed to no caching.
-            logger.debug("Generator cache is not enabled.")
-            shutil.rmtree(generator_cwd, ignore_errors=True)
+            # TODO: Acquire a lock here to ensure that we are the only users
+            if self.is_generator_cacheable():
+                logger.warning(
+                    "Support for generator-side cachable cores are deprecated and will be removed"
+                )
+            else:
+                shutil.rmtree(generator_cwd, ignore_errors=True)
             self._run(generator_cwd)
+            # TODO: Release cache lock
 
         cores = []
         logger.debug("Looking for generated or cached cores in " + generator_cwd)

tests/capi2_cores/misc/generate/generate.core

@@ -14,7 +14,12 @@ targets:
   default:
     filesets :
       - default
-    generate : [testgenerate_without_params, testgenerate_with_params, testgenerate_with_override : {the_value : 138}, testgenerate_with_cache]
+    generate :
+      - testgenerate_without_params
+      - testgenerate_with_params
+      - testgenerate_with_override : {the_value : 138}
+      - testgenerate_with_cache
+      - testgenerate_with_file_cache
     toplevel : na
 
   nogenerate: {generate : []}
@@ -39,5 +44,10 @@ generate:
 
   testgenerate_with_cache:
     generator: generator2
+    parameters:
+      some_option: some_value
+
+  testgenerate_with_file_cache:
+    generator: generator3
     parameters:
       file_in_param1: file_cachetest

tests/capi2_cores/misc/generate/generators.core

@@ -14,4 +14,9 @@ generators:
     interpreter: python3
     command: testgen.py
     cache_type: input
+
+  generator3:
+    interpreter: python3
+    command: testgen.py
+    cache_type: input
     file_input_parameters: file_in_param1

tests/test_capi2.py

@@ -324,11 +324,13 @@ def test_capi2_get_generators():
     core = Core(Core2Parser(), os.path.join(cores_dir, "generate", "generators.core"))
 
     generators = core.get_generators()
-    assert len(generators) == 2
+    assert len(generators) == 3
     assert generators["generator1"]["command"] == "testgen.py"
     assert generators["generator2"]["command"] == "testgen.py"
     assert generators["generator2"]["cache_type"] == "input"
-    assert generators["generator2"]["file_input_parameters"] == "file_in_param1"
+    assert generators["generator3"]["command"] == "testgen.py"
+    assert generators["generator3"]["cache_type"] == "input"
+    assert generators["generator3"]["file_input_parameters"] == "file_in_param1"
 
 
 def test_capi2_get_parameters():
@@ -606,6 +608,12 @@ def test_capi2_get_ttptttg():
             "name": "testgenerate_with_cache",
             "generator": "generator2",
             "pos": "append",
+            "config": {"some_option": "some_value"},
+        },
+        {
+            "name": "testgenerate_with_file_cache",
+            "generator": "generator3",
+            "pos": "append",
             "config": {"file_in_param1": "file_cachetest"},
         },
     ]

tests/test_edalizer.py

@@ -148,18 +148,18 @@ def test_tool_or_flow():
 
 
 def test_generators():
-    import os
     import shutil
     import tempfile
+    from pathlib import Path
 
     from fusesoc.config import Config
     from fusesoc.coremanager import CoreManager
     from fusesoc.edalizer import Edalizer
     from fusesoc.librarymanager import Library
     from fusesoc.vlnv import Vlnv
 
-    tests_dir = os.path.dirname(__file__)
-    cores_dir = os.path.join(tests_dir, "capi2_cores", "misc", "generate")
+    tests_dir = Path(__file__).parent
+    cores_dir = tests_dir / "capi2_cores" / "misc" / "generate"
 
     lib = Library("edalizer", cores_dir)
 
@@ -168,14 +168,14 @@ def test_generators():
 
     core = cm.get_core(Vlnv("::generate"))
 
-    build_root = tempfile.mkdtemp(prefix="export_")
-    export_root = os.path.join(build_root, "exported_files")
+    build_root = Path(tempfile.mkdtemp(prefix="export_"))
+    export_root = build_root / "exported_files"
 
     edalizer = Edalizer(
         toplevel=core.name,
         flags={"tool": "icarus"},
         core_manager=cm,
-        work_root=os.path.join(build_root, "work"),
+        work_root=build_root / "work",
         export_root=export_root,
         system_name=None,
     )
@@ -193,11 +193,13 @@ def test_generators():
                 "::generate-testgenerate_with_params:0",
                 "::generate-testgenerate_with_override:0",
                 "::generate-testgenerate_with_cache:0",
+                "::generate-testgenerate_with_file_cache:0",
             ],
             "::generate-testgenerate_without_params:0": [],
             "::generate-testgenerate_with_params:0": [],
             "::generate-testgenerate_with_override:0": [],
             "::generate-testgenerate_with_cache:0": [],
+            "::generate-testgenerate_with_file_cache:0": [],
         },
         "parameters": {"p": {"datatype": "str", "paramtype": "vlogparam"}},
         "tool_options": {"icarus": {}},
@@ -209,25 +211,39 @@ def test_generators():
     }
 
     assert ref_edam == edalizer.edam
-    edalizer.export()
 
     name_to_core = {str(core.name): core for core in edalizer.cores}
     for flavour in ["testgenerate_with_params", "testgenerate_without_params"]:
         core_name = f"::generate-{flavour}:0"
         assert core_name in name_to_core
         core = name_to_core[core_name]
 
-    # Test generator input cache and file_input_params
-    core_name = f"::generate-testgenerate_with_cache:0"
+    # Test generator input without file_input_params
+    core_name = "::generate-testgenerate_with_cache:0"
     assert core_name in name_to_core
     core = name_to_core[core_name]
-    assert os.path.isdir(core.core_root)
 
-    hash = ""
-    with open(os.path.join(core.core_root, ".fusesoc_file_input_hash")) as f:
-        hash = f.read()
+    core_root = Path(core.core_root)
+    assert core_root.is_dir()
+    assert (
+        core_root.name
+        == "generate-testgenerate_with_cache_0-616d6cf151dba72fcd893c08a8e18e6dba2b81ee25dec08c92e0177064dfc18c"
+    )
+    shutil.rmtree(core.core_root, ignore_errors=True)
 
-    # SHA256 hash of test file content
-    assert hash == "da265f9dccc9d9e64d059f677508f9550b403c99e6ce5df07c6fb1d711d0ee99"
+    # Test generator input file_input_params
+    core_name = "::generate-testgenerate_with_file_cache:0"
+    assert core_name in name_to_core
+    core = name_to_core[core_name]
+    core_root = Path(core.core_root)
 
+    assert core_root.is_dir()
+    assert (
+        core_root.name
+        == "da265f9dccc9d9e64d059f677508f9550b403c99e6ce5df07c6fb1d711d0ee99"
+    )
+    assert (
+        core_root.parent.name
+        == "generate-testgenerate_with_file_cache_0-f3d9e1e462ef1f7113fafbacd62d6335dd684e69332f75498fb01bfaaa7c11ee"
+    )
     shutil.rmtree(core.core_root, ignore_errors=True)