fix: partial backport of #648 (#670)

This just fixes the issue where substituting_steps would be applied to
all flows, disallowing the use of flows like OpenInKLayout or
OpenInOpenROAD.

The breaking strict class-level substitution AND the re-numbering will
both remain in 3.0.0.

Author: donn

Changelog.md

@@ -14,6 +14,13 @@
 ## Documentation
 -->
 
+# 2.3.8
+
+## Misc. Enhancements/Bugfixes
+
+* Fixed substitutions in `config.json` being applied to all flows. It now only
+  applies to the flow in meta.flow (which falls back to `Classic` if it's null.)
+  
 # 2.3.7
 
 ## Tool Updates

openlane/__main__.py

@@ -21,7 +21,7 @@
 import subprocess
 from textwrap import dedent
 from functools import partial
-from typing import Any, Dict, Sequence, Tuple, Type, Optional, List, Union
+from typing import Any, Dict, Sequence, Tuple, Type, Optional, List
 
 import click
 from cloup import (
@@ -77,21 +77,39 @@ def run(
             err("No config file(s) have been provided.")
             ctx.exit(1)
 
-        flow_description: Optional[Union[str, List[str]]] = None
-        substitutions: Union[None, Dict[str, Union[str, None]]] = None
+        TargetFlow: Optional[Type[Flow]] = Flow.factory.get("Classic")
 
         for config_file in config_files:
             if meta := Config.get_meta(config_file):
-                if meta.flow is not None:
-                    flow_description = meta.flow
-                    if meta.substituting_steps is not None:
-                        substitutions = meta.substituting_steps
+                # to maintain backwards compat, in 3 you will need to explicitly
+                # set the flow you're substituting
+                target_flow_desc = meta.flow or "Classic"
+
+                if isinstance(target_flow_desc, str):
+                    if found := Flow.factory.get(target_flow_desc):
+                        TargetFlow = found
+                    else:
+                        err(
+                            f"Unknown flow '{meta.flow}' specified in configuration file's 'meta' object."
+                        )
+                        ctx.exit(1)
+                elif isinstance(target_flow_desc, list):
+                    TargetFlow = SequentialFlow.make(target_flow_desc)
+                if meta.substituting_steps is not None and issubclass(
+                    TargetFlow, SequentialFlow
+                ):
+                    if meta.flow is None:
+                        warn(
+                            'config_file currently has substituting_steps set with no flow, where it will fall back to Classic. Starting OpenLane 3.0.0, this will be an error. Please update your configuration to explicitly set "flow" to "Classic".'
+                        )
+                    TargetFlow = TargetFlow.Substitute(meta.substituting_steps)  # type: ignore  # Type checker is being rowdy with this one
 
         if flow_name is not None:
-            flow_description = flow_name
-
-        if flow_description is None:
-            flow_description = "Classic"
+            if found := Flow.factory.get(flow_name):
+                TargetFlow = found
+            else:
+                err(f"Unknown flow '{flow_name}' passed to initialization function.")
+                ctx.exit(1)
 
         if len(initial_state_element_override):
             if with_initial_state is None:
@@ -114,18 +132,9 @@ def run(
                 overrides=overrides,
             )
 
-        TargetFlow: Type[Flow]
-
-        if isinstance(flow_description, str):
-            if FlowClass := Flow.factory.get(flow_description):
-                TargetFlow = FlowClass
-            else:
-                err(
-                    f"Unknown flow '{flow_description}' specified in configuration file's 'meta' object."
-                )
-                ctx.exit(1)
-        else:
-            TargetFlow = SequentialFlow.make(flow_description)
+        assert (
+            TargetFlow is not None
+        ), "TargetFlow is unexpectedly None. Please report this as a bug."
 
         kwargs: Dict[str, Any] = {
             "pdk_root": pdk_root,
@@ -134,8 +143,6 @@ def run(
             "config_override_strings": config_override_strings,
             "design_dir": design_dir,
         }
-        if issubclass(TargetFlow, SequentialFlow):
-            kwargs["Substitute"] = substitutions
         flow = TargetFlow(config_files, **kwargs)
     except PassedDirectoryError as e:
         err(e)

openlane/flows/sequential.py

@@ -39,6 +39,8 @@
     DeferredStepError,
 )
 
+Substitution = Union[str, Type[Step], None]
+
 
 class SequentialFlow(Flow):
     """
@@ -120,6 +122,20 @@ class CustomSequentialFlow(SequentialFlow):
 
         return CustomSequentialFlow
 
+    @classmethod
+    def Substitute(
+        Self, Substitutions: Dict[str, Substitution]
+    ) -> Type[SequentialFlow]:
+        """
+        Convenience method to quickly subclass a sequential flow and add
+        Substitutions to it.
+
+        The new flow shall be named ``{previous_flow_name}'``.
+
+        :param Substitutions: The substitutions to use for the new subclass.
+        """
+        return type(Self.__name__ + "'", (Self,), {"Substitutions": Substitutions})
+
     def __init__(
         self,
         *args,

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "openlane"
-version = "2.3.7"
+version = "2.3.8"
 description = "An infrastructure for implementing chip design flows"
 authors = ["Efabless Corporation and Contributors <[email protected]>"]
 readme = "Readme.md"