Skip to content

Commit df953e1

Browse files
committed
Apply referenced mixins in parse_args
Expand each requested mixin into its full application order using colcon_mixin.mixin.order and apply the result in reverse, so the prepend-based overlay produces the computed order while explicit command line arguments keep precedence. A referencing mixin is applied conceptually last and can override values inherited from its references. Skip the reserved 'mixin' key during overlay since it is metadata, not an argument. Report circular, unknown and malformed references as clean CLI errors. Mixins without a 'mixin' key behave exactly as before. Add parser integration tests covering nested references, scalar overrides, command line precedence and error reporting. Assisted-by: Claude Opus (Antigravity) for test case generation and architecture-level testing
1 parent c1ea71b commit df953e1

2 files changed

Lines changed: 85 additions & 5 deletions

File tree

colcon_mixin/mixin/mixin_argument.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@
2424
from colcon_core.plugin_system import satisfies_version
2525
from colcon_mixin.mixin import add_mixins
2626
from colcon_mixin.mixin import get_mixins
27+
from colcon_mixin.mixin.order import CircularMixinError
28+
from colcon_mixin.mixin.order import compute_application_order
29+
from colcon_mixin.mixin.order import InvalidMixinError
30+
from colcon_mixin.mixin.order import MissingMixinError
2731

2832
logger = colcon_logger.getChild(__name__)
2933

@@ -181,23 +185,36 @@ def collect_parsers_by_verb(root, parsers, parent_verbs=()):
181185
# update args based on selected mixins
182186
if 'mixin_verb' in args:
183187
mixins = mixins_by_verb.get(args.mixin_verb, {})
188+
context = '.'.join(args.mixin_verb)
184189
# validate the requested mixins in the order given on the command
185190
# line so an error reports the first unavailable mixin
186191
for mixin in args.mixin or ():
187192
if mixin not in mixins:
188-
context = '.'.join(args.mixin_verb)
189193
self._parser.error(
190194
"Mixin '{mixin}' is not available for '{context}'"
191195
.format_map(locals()))
196+
# expand each requested mixin into the full list of mixins to
197+
# apply: a mixin may reference other mixins through a 'mixin' key,
198+
# which are applied first so the requesting mixin can override
199+
# them (depth-first post-order, see colcon_mixin.mixin.order)
200+
application_order = []
201+
for mixin in args.mixin or ():
202+
try:
203+
application_order += compute_application_order(
204+
mixins, mixin)
205+
except (
206+
CircularMixinError, InvalidMixinError, MissingMixinError,
207+
) as e:
208+
self._parser.error(str(e))
192209
# apply the mixins in reverse order: since _update_args() handles
193210
# each mixin by prepending its list values, iterating in reverse
194-
# makes the resulting list follow the order the mixins were given
195-
# on the command line while keeping any explicit command line
196-
for mixin in reversed(args.mixin or ()):
211+
# makes the resulting list follow the computed application order
212+
# while keeping any explicit command line arguments last
213+
for mixin in reversed(application_order):
197214
mixin_args = mixins[mixin]
198215
logger.debug(
199216
"Using mixin '{mixin}': {mixin_args}".format_map(locals()))
200-
self._update_args(args, mixin_args, '.'.join(args.mixin_verb))
217+
self._update_args(args, mixin_args, context)
201218

202219
# undo default value wrapping injected in the add_argument() method
203220
for k, v in args.__dict__.items():
@@ -261,6 +278,10 @@ def _update_mixin_argument(self, argument, mixins):
261278
def _update_args(self, args, mixin_args, context):
262279
destinations = self.get_destinations()
263280
for mixin_key, mixin_value in mixin_args.items():
281+
if mixin_key == 'mixin':
282+
# reserved metadata key listing referenced mixins; it is not
283+
# an argument to overlay (see colcon_mixin.mixin.order)
284+
continue
264285
if mixin_key not in destinations:
265286
logger.warning(
266287
"Mixin key '{mixin_key}' is not a valid argument for "

test/test_mixin_argument.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,26 @@
2020
}
2121

2222

23+
NESTED_MIXINS = {
24+
'base': {'cmake-args': ['FOO=BASE']},
25+
'child': {'mixin': ['base'], 'cmake-args': ['FOO=CHILD']},
26+
}
27+
28+
29+
SCALAR_NESTED_MIXINS = {
30+
'base': {'build-base': 'BASE'},
31+
'child': {'mixin': ['base'], 'build-base': 'CHILD'},
32+
}
33+
34+
35+
CANONICAL_MIXINS = {
36+
'A': {'mixin': ['B', 'D'], 'cmake-args': ['A']},
37+
'B': {'mixin': ['C'], 'cmake-args': ['B']},
38+
'C': {'mixin': ['D'], 'cmake-args': ['C']},
39+
'D': {'cmake-args': ['D']},
40+
}
41+
42+
2343
def _parse(argv, mixins=MIXINS):
2444
base = argparse.ArgumentParser(prog='colcon')
2545
decorator = MixinArgumentDecorator(base)
@@ -88,3 +108,42 @@ def test_unavailable_mixin_reports_first(capsys):
88108
with pytest.raises(SystemExit):
89109
_parse(['build', '--mixin', 'bad', 'a'])
90110
assert "Mixin 'bad' is not available" in capsys.readouterr().err
111+
112+
113+
def test_referenced_mixin_applied_before_referencing():
114+
args = _parse(['build', '--mixin', 'child'], NESTED_MIXINS)
115+
assert args.cmake_args == ['FOO=BASE', 'FOO=CHILD']
116+
117+
118+
def test_referencing_mixin_overrides_referenced_scalar():
119+
args = _parse(['build', '--mixin', 'child'], SCALAR_NESTED_MIXINS)
120+
assert args.build_base == 'CHILD'
121+
122+
123+
def test_canonical_reference_graph_order():
124+
# D appears twice, once per path
125+
args = _parse(['build', '--mixin', 'A'], CANONICAL_MIXINS)
126+
assert args.cmake_args == ['D', 'C', 'B', 'D', 'A']
127+
128+
129+
def test_nested_mixin_command_line_arguments_take_precedence():
130+
args = _parse(
131+
['build', '--mixin', 'child', '--cmake-args=FOO=CLI'], NESTED_MIXINS)
132+
assert args.cmake_args == ['FOO=BASE', 'FOO=CHILD', 'FOO=CLI']
133+
134+
135+
def test_circular_mixin_reference_reports_error(capsys):
136+
mixins = {
137+
'a': {'mixin': ['b']},
138+
'b': {'mixin': ['a']},
139+
}
140+
with pytest.raises(SystemExit):
141+
_parse(['build', '--mixin', 'a'], mixins)
142+
assert 'Circular mixin reference' in capsys.readouterr().err
143+
144+
145+
def test_unknown_referenced_mixin_reports_error(capsys):
146+
mixins = {'a': {'mixin': ['missing']}}
147+
with pytest.raises(SystemExit):
148+
_parse(['build', '--mixin', 'a'], mixins)
149+
assert "unknown mixin 'missing'" in capsys.readouterr().err

0 commit comments

Comments
 (0)