context: allow redefinitions of variables

doc/directives.md

@@ -12,10 +12,9 @@ other parts of the omnifest.
 Variable scope is global, an `otk.define` directive anywhere in the omnifest
 tree will result in the defined names being hoisted to the global scope.
 
-Redefinitions of variables are forbidden and will cause an error when detected.
-A redefinition of a variable is assigning a value different from the value it
-is currently holding. It is thus wise to 'namespace' variables by putting them
-inside a map.
+Redefinitions of variables are allowed. This allows for setting up default
+values. If `-w duplicate-definition` is passed as an argument to `otk` then
+`otk` will warn on all duplicate definitions.
 
 Expects a `map` for its value.
 

src/otk/command.py

@@ -35,10 +35,22 @@
     "--identifier",
     help="An identifier to include in all log records generated by `otk -j`. Can only be used together with `-j`.",
 )
-def root(verbose: int, json: bool, identifier: str) -> None:
+@click.option(
+    "-w",
+    "--warn",
+    type=click.Choice(["duplicate-definition"]),
+    multiple=True,
+    help="Enable warnings, can be passed multiple times.",
+)
+@click.pass_context
+def root(
+    ctx: click.Context, verbose: int, json: bool, identifier: str, warn: list[str]
+) -> None:
     """`otk` is the omnifest toolkit. A program to work with omnifest inputs
     and translate them into the native formats for image build tooling."""
 
+    ctx.ensure_object(dict)
+
     logging.basicConfig(
         level=logging.WARNING - (10 * verbose),
         handlers=[
@@ -54,6 +66,11 @@ def root(verbose: int, json: bool, identifier: str) -> None:
         ],
     )
 
+    # This context is passed along to all other subcommands.
+    ctx.obj["context"] = {
+        "duplicate_definitions_warning": "duplicate-definition" in warn,
+    }
+
     # We do this check *after* setting up the handlers so the error is formatted
     if identifier and not json:
         log.error("cannot use `-i` without also using `-j`")
@@ -63,7 +80,8 @@ def root(verbose: int, json: bool, identifier: str) -> None:
 @root.command()
 @click.argument("input", type=click.Path(exists=True))
 @click.argument("output", type=click.Path(), required=False)
-def compile(input: str, output: str | None) -> None:
+@click.pass_context
+def compile(ctx: click.Context, input: str, output: str | None) -> None:
     """Compile a given omnifest into its targets."""
 
     log.info("Compiling the input file %r to %r", input, output)
@@ -72,7 +90,8 @@ def compile(input: str, output: str | None) -> None:
 @root.command()
 @click.argument("input", type=click.Path(exists=True))
 @click.argument("output", type=click.Path(), required=False)
-def flatten(input: str, output: str | None) -> None:
+@click.pass_context
+def flatten(ctx: click.Context, input: str, output: str | None) -> None:
     """Flatten a given omnifest by performing all includes."""
 
     log.info("flattening %r to %r", input, output or "STDOUT")
@@ -83,7 +102,7 @@ def flatten(input: str, output: str | None) -> None:
     # TODO posixpath serializer for json output
     log.info("include root is %r", str(root))
 
-    context = Context(root)
+    context = Context(root, **ctx.obj["context"])
 
     tree = Omnifest.from_yaml_path(file).to_tree()
 

src/otk/transform/context.py

@@ -20,15 +20,27 @@
 
 
 class Context:
+    duplicate_definitions_allowed: bool
+    duplicate_definitions_warning: bool
+
     _version: int | None
     _path: pathlib.Path
     _variables: dict[str, Any]
 
-    def __init__(self, path: pathlib.Path | None = None) -> None:
+    def __init__(
+        self,
+        path: pathlib.Path | None = None,
+        *,
+        duplicate_definitions_allowed: bool = True,
+        duplicate_definitions_warning: bool = False,
+    ) -> None:
         self._version = None
         self._path = path if path else pathlib.Path(".")
         self._variables = {}
 
+        self.duplicate_definitions_allowed = duplicate_definitions_allowed
+        self.duplicate_definitions_warning = duplicate_definitions_warning
+
     def version(self, v: int) -> None:
         # Set the context version, duplicate definitions with different
         # versions are an error
@@ -40,12 +52,19 @@ def version(self, v: int) -> None:
         self._version = v
 
     def define(self, name: str, value: Any) -> None:
-        # Since we go through the tree multiple times it's easy to ignore
-        # duplicate definitions as long as they define to the *same* value.
-        if name in self._variables and self._variables[name] != value:
-            raise TransformDefineDuplicateError()
-
-        self._variables[name] = value
+        if name in self._variables:
+            if not self.duplicate_definitions_allowed:
+                raise TransformDefineDuplicateError()
+
+            if self.duplicate_definitions_warning:
+                log.warn(
+                    "redefinition of %r, previous values was %r and new value is %r",
+                    name,
+                    self._variables[name],
+                    value,
+                )
+        else:
+            self._variables[name] = value
 
     def variable(self, name: str) -> Any:
         parts = name.split(".")

test/test_transform_context.py

@@ -6,6 +6,7 @@
     TransformVariableTypeError,
     TransformVariableIndexTypeError,
     TransformVariableIndexRangeError,
+    TransformDefineDuplicateError,
 )
 
 
@@ -63,3 +64,19 @@ def test_context_unhappy():
 
     with pytest.raises(TransformVariableIndexRangeError):
         ctx.variable("bar.3")
+
+
+def test_context_duplicate_definition():
+    ctx0 = Context()
+
+    # Redefinition allowed
+    ctx0.define("foo", "bar")
+    ctx0.define("foo", "bar")
+
+    ctx1 = Context(duplicate_definitions_allowed=False)
+
+    # Redefinition NOT allowed
+    ctx1.define("foo", "bar")
+
+    with pytest.raises(TransformDefineDuplicateError):
+        ctx1.define("foo", "bar")