Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions colcon_mixin/mixin/mixin_argument.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
from colcon_core.plugin_system import satisfies_version
from colcon_mixin.mixin import add_mixins
from colcon_mixin.mixin import get_mixins
from colcon_mixin.mixin.order import CircularMixinError
from colcon_mixin.mixin.order import compute_application_order
from colcon_mixin.mixin.order import InvalidMixinError
from colcon_mixin.mixin.order import MissingMixinError

logger = colcon_logger.getChild(__name__)

Expand Down Expand Up @@ -181,16 +185,36 @@ def collect_parsers_by_verb(root, parsers, parent_verbs=()):
# update args based on selected mixins
if 'mixin_verb' in args:
mixins = mixins_by_verb.get(args.mixin_verb, {})
context = '.'.join(args.mixin_verb)
# validate the requested mixins in the order given on the command
# line so an error reports the first unavailable mixin
for mixin in args.mixin or ():
if mixin not in mixins:
context = '.'.join(args.mixin_verb)
self._parser.error(
"Mixin '{mixin}' is not available for '{context}'"
.format_map(locals()))
# expand each requested mixin into the full list of mixins to
# apply: a mixin may reference other mixins through a 'mixin' key,
# which are applied first so the requesting mixin can override
# them (depth-first post-order, see colcon_mixin.mixin.order)
application_order = []
for mixin in args.mixin or ():
try:
application_order += compute_application_order(
mixins, mixin)
except (
CircularMixinError, InvalidMixinError, MissingMixinError,
) as e:
self._parser.error(str(e))
# apply the mixins in reverse order: since _update_args() handles
# each mixin by prepending its list values, iterating in reverse
# makes the resulting list follow the computed application order
# while keeping any explicit command line arguments last
for mixin in reversed(application_order):
mixin_args = mixins[mixin]
logger.debug(
"Using mixin '{mixin}': {mixin_args}".format_map(locals()))
self._update_args(args, mixin_args, '.'.join(args.mixin_verb))
self._update_args(args, mixin_args, context)

# undo default value wrapping injected in the add_argument() method
for k, v in args.__dict__.items():
Expand Down Expand Up @@ -254,6 +278,10 @@ def _update_mixin_argument(self, argument, mixins):
def _update_args(self, args, mixin_args, context):
destinations = self.get_destinations()
for mixin_key, mixin_value in mixin_args.items():
if mixin_key == 'mixin':
# reserved metadata key listing referenced mixins; it is not
# an argument to overlay (see colcon_mixin.mixin.order)
continue
if mixin_key not in destinations:
logger.warning(
"Mixin key '{mixin_key}' is not a valid argument for "
Expand Down
69 changes: 69 additions & 0 deletions colcon_mixin/mixin/order.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Copyright 2026 Innocent Pious
# Licensed under the Apache License, Version 2.0

"""Compute the order in which mixins referencing other mixins are applied."""


class CircularMixinError(Exception):
"""Raised when the mixin reference graph contains a cycle."""


class MissingMixinError(Exception):
"""Raised when a referenced mixin name does not exist."""


class InvalidMixinError(Exception):
"""Raised when a 'mixin' key is not a list of strings."""


def _read_references(mixins, name):
refs = mixins[name].get('mixin', [])
if not isinstance(refs, list) or not all(
isinstance(ref, str) for ref in refs
):
raise InvalidMixinError(
"Mixin '{name}' has an invalid 'mixin' key: expected a list of "
'strings'.format_map(locals()))
return refs


def compute_application_order(mixins, requested):
"""Return the mixins to apply for a requested mixin, dependencies first.

A mixin may reference other mixins through a 'mixin' key. References are
ordered before the mixin that references them (depth-first, post-order),
so the referencing mixin is applied last and can override them. A mixin
reached through several paths is applied once per path to keep
last-applied-wins semantics, so duplicates in the result are intentional.

:raises CircularMixinError: on a circular reference.
:raises MissingMixinError: if a referenced mixin does not exist.
:raises InvalidMixinError: if a 'mixin' key is malformed.
"""
order = []
stack_path = [] # names on the active recursion path, for cycle detection

def visit(name, referrer):
if name not in mixins:
if referrer is None:
raise MissingMixinError(
"Requested mixin '{name}' does not exist"
.format_map(locals()))
raise MissingMixinError(
"Mixin '{referrer}' references unknown mixin '{name}'"
.format_map(locals()))

if name in stack_path:
path = ' -> '.join(stack_path + [name])
raise CircularMixinError(
'Circular mixin reference: {path}'.format_map(locals()))

refs = _read_references(mixins, name)
stack_path.append(name)
for ref in refs:
visit(ref, name)
stack_path.pop()
order.append(name)

visit(requested, None)
return order
3 changes: 3 additions & 0 deletions test/spell_check.words
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ argparse
basenames
basepath
blocklist
capsys
cmake
colcon
completers
defaultdict
Expand All @@ -17,6 +19,7 @@ plugin
prepending
pydocstyle
pytest
readouterr
rtype
scspell
setuptools
Expand Down
149 changes: 149 additions & 0 deletions test/test_mixin_argument.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
# Copyright 2026 Open Source Robotics Foundation, Inc.
# Licensed under the Apache License, Version 2.0

import argparse
from unittest.mock import patch

from colcon_mixin.mixin.mixin_argument import MixinArgumentDecorator
import pytest

MIXINS = {
'a': {'cmake-args': ['FOO=A']},
'b': {'cmake-args': ['FOO=B']},
'c': {'cmake-args': ['FOO=C']},
}


SCALAR_MIXINS = {
'a': {'build-base': 'BASE_A'},
'b': {'build-base': 'BASE_B'},
}


NESTED_MIXINS = {
'base': {'cmake-args': ['FOO=BASE']},
'child': {'mixin': ['base'], 'cmake-args': ['FOO=CHILD']},
}


SCALAR_NESTED_MIXINS = {
'base': {'build-base': 'BASE'},
'child': {'mixin': ['base'], 'build-base': 'CHILD'},
}


CANONICAL_MIXINS = {
'A': {'mixin': ['B', 'D'], 'cmake-args': ['A']},
'B': {'mixin': ['C'], 'cmake-args': ['B']},
'C': {'mixin': ['D'], 'cmake-args': ['C']},
'D': {'cmake-args': ['D']},
}


def _parse(argv, mixins=MIXINS):
base = argparse.ArgumentParser(prog='colcon')
decorator = MixinArgumentDecorator(base)
subparsers = decorator.add_subparsers(dest='verb')
build = subparsers.add_parser('build')
build.add_argument('--cmake-args', nargs='*', default=[])
build.add_argument('--build-base', default='build')
mixins_by_verb = {('build',): mixins}
with patch(
'colcon_mixin.mixin.mixin_argument.get_mixins',
return_value=mixins_by_verb,
):
return decorator.parse_args(argv)


def test_multiple_mixins_preserve_command_line_order():
# order the mixins were given on the command line
args = _parse(['build', '--mixin', 'a', 'b'])
assert args.cmake_args == ['FOO=A', 'FOO=B']


def test_multiple_mixins_follow_reversed_selection():
args = _parse(['build', '--mixin', 'b', 'a'])
assert args.cmake_args == ['FOO=B', 'FOO=A']


def test_three_mixins_preserve_order():
args = _parse(['build', '--mixin', 'a', 'b', 'c'])
assert args.cmake_args == ['FOO=A', 'FOO=B', 'FOO=C']


def test_command_line_arguments_take_precedence():
# explicit command line arguments must come last so they win
args = _parse(['build', '--mixin', 'a', 'b', '--cmake-args=FOO=CLI'])
assert args.cmake_args == ['FOO=A', 'FOO=B', 'FOO=CLI']


def test_single_mixin():
args = _parse(['build', '--mixin', 'a'])
assert args.cmake_args == ['FOO=A']


def test_no_mixin():
args = _parse(['build'])
assert args.cmake_args == []


def test_scalar_last_listed_mixin_wins():
# a scalar value from a later mixin must override an earlier one
args = _parse(['build', '--mixin', 'a', 'b'], SCALAR_MIXINS)
assert args.build_base == 'BASE_B'


def test_scalar_reversed_selection():
args = _parse(['build', '--mixin', 'b', 'a'], SCALAR_MIXINS)
assert args.build_base == 'BASE_A'


def test_scalar_command_line_argument_take_precedence():
args = _parse(
['build', '--mixin', 'a', 'b', '--build-base', 'CLI'], SCALAR_MIXINS)
assert args.build_base == 'CLI'


def test_unavailable_mixin_reports_first(capsys):
with pytest.raises(SystemExit):
_parse(['build', '--mixin', 'bad', 'a'])
assert "Mixin 'bad' is not available" in capsys.readouterr().err


def test_referenced_mixin_applied_before_referencing():
args = _parse(['build', '--mixin', 'child'], NESTED_MIXINS)
assert args.cmake_args == ['FOO=BASE', 'FOO=CHILD']


def test_referencing_mixin_overrides_referenced_scalar():
args = _parse(['build', '--mixin', 'child'], SCALAR_NESTED_MIXINS)
assert args.build_base == 'CHILD'


def test_canonical_reference_graph_order():
# D appears twice, once per path
args = _parse(['build', '--mixin', 'A'], CANONICAL_MIXINS)
assert args.cmake_args == ['D', 'C', 'B', 'D', 'A']


def test_nested_mixin_command_line_arguments_take_precedence():
args = _parse(
['build', '--mixin', 'child', '--cmake-args=FOO=CLI'], NESTED_MIXINS)
assert args.cmake_args == ['FOO=BASE', 'FOO=CHILD', 'FOO=CLI']


def test_circular_mixin_reference_reports_error(capsys):
mixins = {
'a': {'mixin': ['b']},
'b': {'mixin': ['a']},
}
with pytest.raises(SystemExit):
_parse(['build', '--mixin', 'a'], mixins)
assert 'Circular mixin reference' in capsys.readouterr().err


def test_unknown_referenced_mixin_reports_error(capsys):
mixins = {'a': {'mixin': ['missing']}}
with pytest.raises(SystemExit):
_parse(['build', '--mixin', 'a'], mixins)
assert "unknown mixin 'missing'" in capsys.readouterr().err
Loading
Loading