Skip to content

Commit 88d78d8

Browse files
mgornymgorny
authored
Some fixes for AoT plugins (#124)
* Use `multi_value=False` for locally-defined properties Use `multi_value=False` rather than `True` for AoT properties. As Jonathan pointed out, multi-value properties only make sense for filtering, given that the system may support only a subset of all properties. However, AoT properties are always supported, so there is no real difference between a wheel built with multiple values for a given property, and one built with just the highest value. Signed-off-by: Michał Górny <mgorny@quansight.com> * Use a dedicated AoT plugin for testing Use a dedicated AoT plugin rather than piggy-backing on a regular plugin, where independent changes can trigger test failures. Signed-off-by: Michał Górny <mgorny@quansight.com> * Make a random test property multi-value for better testing Signed-off-by: Michał Górny <mgorny@quansight.com> * Validate that AoT plugins don't return multi-value features Signed-off-by: Michał Górny <mgorny@quansight.com> --------- Signed-off-by: Michał Górny <mgorny@quansight.com>
1 parent 62fcf1b commit 88d78d8

5 files changed

Lines changed: 105 additions & 33 deletions

File tree

‎tests/mocked_plugins.py‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,14 @@ class MockedEntryPoint:
1818
class MockedPluginA(PluginType):
1919
namespace = "test_namespace" # pyright: ignore[reportAssignmentType,reportIncompatibleMethodOverride]
2020

21-
is_build_plugin = True
22-
2321
@staticmethod
2422
def get_all_configs() -> list[VariantFeatureConfigType]:
2523
return [
2624
VariantFeatureConfig(
2725
"name1", ["val1a", "val1b", "val1c", "val1d"], multi_value=False
2826
),
2927
VariantFeatureConfig(
30-
"name2", ["val2a", "val2b", "val2c"], multi_value=False
28+
"name2", ["val2a", "val2b", "val2c"], multi_value=True
3129
),
3230
]
3331

@@ -36,7 +34,7 @@ def get_supported_configs() -> list[VariantFeatureConfigType]:
3634
return [
3735
VariantFeatureConfig("name1", ["val1a", "val1b"], multi_value=False),
3836
VariantFeatureConfig(
39-
"name2", ["val2a", "val2b", "val2c"], multi_value=False
37+
"name2", ["val2a", "val2b", "val2c"], multi_value=True
4038
),
4139
]
4240

@@ -90,6 +88,48 @@ def get_supported_configs() -> list[VariantFeatureConfigType]:
9088
return []
9189

9290

91+
class MockedAoTPlugin(PluginType):
92+
namespace = "aot_plugin"
93+
94+
is_build_plugin = True
95+
96+
@staticmethod
97+
def get_all_configs() -> list[VariantFeatureConfigType]:
98+
return [
99+
VariantFeatureConfig("name1", ["val1a", "val1b"], multi_value=False),
100+
VariantFeatureConfig(
101+
"name2", ["val2a", "val2b", "val2c"], multi_value=False
102+
),
103+
]
104+
105+
@staticmethod
106+
def get_supported_configs() -> list[VariantFeatureConfigType]:
107+
return [
108+
VariantFeatureConfig("name1", ["val1a", "val1b"], multi_value=False),
109+
VariantFeatureConfig(
110+
"name2", ["val2a", "val2b", "val2c"], multi_value=False
111+
),
112+
]
113+
114+
115+
class MultiValueAoTPlugin(PluginType):
116+
namespace = "aot_plugin"
117+
118+
is_build_plugin = True
119+
120+
@staticmethod
121+
def get_all_configs() -> list[VariantFeatureConfigType]:
122+
return [
123+
VariantFeatureConfig("name1", ["val1a"], multi_value=True),
124+
]
125+
126+
@staticmethod
127+
def get_supported_configs() -> list[VariantFeatureConfigType]:
128+
return [
129+
VariantFeatureConfig("name1", ["val1a"], multi_value=True),
130+
]
131+
132+
93133
class IndirectPath:
94134
class MoreIndirection:
95135
object_a = MockedPluginA()

‎tests/plugins/test_loader.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ def test_get_all_configs(
128128
"name1", ["val1a", "val1b", "val1c", "val1d"], multi_value=False
129129
),
130130
VariantFeatureConfig(
131-
"name2", ["val2a", "val2b", "val2c"], multi_value=False
131+
"name2", ["val2a", "val2b", "val2c"], multi_value=True
132132
),
133133
],
134134
),
@@ -150,7 +150,7 @@ def test_get_supported_configs(
150150
configs=[
151151
VariantFeatureConfig("name1", ["val1a", "val1b"], multi_value=False),
152152
VariantFeatureConfig(
153-
"name2", ["val2a", "val2b", "val2c"], multi_value=False
153+
"name2", ["val2a", "val2b", "val2c"], multi_value=True
154154
),
155155
],
156156
),
@@ -626,7 +626,7 @@ def test_package_defined_properties(include_build_plugins: bool) -> None:
626626
"val2b",
627627
"val2c",
628628
],
629-
multi_value=False,
629+
multi_value=True,
630630
),
631631
],
632632
),
@@ -639,7 +639,7 @@ def test_package_defined_properties(include_build_plugins: bool) -> None:
639639
"v3",
640640
"v4",
641641
],
642-
multi_value=True,
642+
multi_value=False,
643643
),
644644
],
645645
),
@@ -652,7 +652,7 @@ def test_package_defined_properties(include_build_plugins: bool) -> None:
652652
"v5",
653653
"v6",
654654
],
655-
multi_value=True,
655+
multi_value=False,
656656
),
657657
],
658658
),

‎tests/test_api.py‎

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -336,8 +336,6 @@ def test_validate_variant(optional: bool) -> None:
336336
VariantProperty("incompatible_namespace", "flag5", "on"): False,
337337
VariantProperty("missing_namespace", "name", "val"): None,
338338
VariantProperty("private", "build_type", "debug"): True,
339-
VariantProperty("private", "build_type", "minsizerel"): False,
340-
VariantProperty("private", "build_type", "release"): True,
341339
VariantProperty("private", "cow", "moo"): False,
342340
}
343341

@@ -691,14 +689,14 @@ def test_make_variant_dist_info_expand_build_plugin_properties(
691689
) -> None:
692690
vdesc = VariantDescription(
693691
[
694-
VariantProperty("test_namespace", "name1", "val1a"),
692+
VariantProperty("aot_plugin", "name1", "val1a"),
695693
]
696694
)
697-
plugin_api = "tests.mocked_plugins:MockedPluginA"
695+
plugin_api = "tests.mocked_plugins:MockedAoTPlugin"
698696
vinfo = VariantInfo(
699-
namespace_priorities=["test_namespace"],
697+
namespace_priorities=["aot_plugin"],
700698
providers={
701-
"test_namespace": ProviderInfo(
699+
"aot_plugin": ProviderInfo(
702700
requires=["variantlib"],
703701
plugin_api=plugin_api,
704702
optional=True,
@@ -710,37 +708,37 @@ def test_make_variant_dist_info_expand_build_plugin_properties(
710708
expected: VariantsJsonDict = {
711709
VARIANTS_JSON_SCHEMA_KEY: VARIANTS_JSON_SCHEMA_URL,
712710
VARIANT_INFO_DEFAULT_PRIO_KEY: {
713-
VARIANT_INFO_NAMESPACE_KEY: ["test_namespace"],
711+
VARIANT_INFO_NAMESPACE_KEY: ["aot_plugin"],
714712
},
715713
VARIANT_INFO_PROVIDER_DATA_KEY: {
716-
"test_namespace": {
714+
"aot_plugin": {
717715
VARIANT_INFO_PROVIDER_REQUIRES_KEY: ["variantlib"],
718716
VARIANT_INFO_PROVIDER_OPTIONAL_KEY: True,
719717
VARIANT_INFO_PROVIDER_PLUGIN_API_KEY: plugin_api,
720718
},
721719
},
722720
VARIANTS_JSON_VARIANT_DATA_KEY: {
723721
"test": {
724-
"test_namespace": {
722+
"aot_plugin": {
725723
"name1": ["val1a"],
726724
}
727725
},
728726
},
729727
}
730728

731729
if plugin_use == PluginUse.NONE:
732-
expected[VARIANT_INFO_PROVIDER_DATA_KEY]["test_namespace"][
730+
expected[VARIANT_INFO_PROVIDER_DATA_KEY]["aot_plugin"][
733731
VARIANT_INFO_PROVIDER_PLUGIN_USE_KEY
734732
] = "none"
735733
if plugin_use == PluginUse.BUILD:
736-
expected[VARIANT_INFO_PROVIDER_DATA_KEY]["test_namespace"][
734+
expected[VARIANT_INFO_PROVIDER_DATA_KEY]["aot_plugin"][
737735
VARIANT_INFO_PROVIDER_PLUGIN_USE_KEY
738736
] = "build"
739737
expected[VARIANT_INFO_DEFAULT_PRIO_KEY][VARIANT_INFO_FEATURE_KEY] = {
740-
"test_namespace": ["name1", "name2"],
738+
"aot_plugin": ["name1", "name2"],
741739
}
742740
expected[VARIANT_INFO_DEFAULT_PRIO_KEY][VARIANT_INFO_PROPERTY_KEY] = {
743-
"test_namespace": {
741+
"aot_plugin": {
744742
"name1": ["val1a", "val1b"],
745743
"name2": ["val2a", "val2b", "val2c"],
746744
},
@@ -759,17 +757,48 @@ def test_make_variant_dist_info_expand_build_plugin_properties(
759757
)
760758

761759

762-
def test_make_variant_dist_info_invalid_build_plugin() -> None:
760+
def test_make_variant_dist_info_invalid_aot_plugin_property() -> None:
763761
vdesc = VariantDescription(
764762
[
765-
VariantProperty("test_namespace", "name1", "val1d"),
763+
VariantProperty("aot_plugin", "name1", "val1d"),
766764
]
767765
)
768-
plugin_api = "tests.mocked_plugins:MockedPluginA"
766+
plugin_api = "tests.mocked_plugins:MockedAoTPlugin"
769767
vinfo = VariantInfo(
770-
namespace_priorities=["test_namespace"],
768+
namespace_priorities=["aot_plugin"],
771769
providers={
772-
"test_namespace": ProviderInfo(
770+
"aot_plugin": ProviderInfo(
771+
requires=["variantlib"],
772+
plugin_api=plugin_api,
773+
optional=True,
774+
plugin_use=PluginUse.BUILD,
775+
)
776+
},
777+
)
778+
779+
with pytest.raises(
780+
ValidationError,
781+
match=r"Property 'aot_plugin :: name1 :: val1d' is not installable "
782+
r"according to the respective provider plugin",
783+
):
784+
make_variant_dist_info(
785+
vdesc,
786+
variant_info=vinfo,
787+
expand_build_plugin_properties=True,
788+
)
789+
790+
791+
def test_make_variant_dist_info_invalid_aot_plugin_multi_value() -> None:
792+
vdesc = VariantDescription(
793+
[
794+
VariantProperty("aot_plugin", "name1", "val1a"),
795+
]
796+
)
797+
plugin_api = "tests.mocked_plugins:MultiValueAoTPlugin"
798+
vinfo = VariantInfo(
799+
namespace_priorities=["aot_plugin"],
800+
providers={
801+
"aot_plugin": ProviderInfo(
773802
requires=["variantlib"],
774803
plugin_api=plugin_api,
775804
optional=True,
@@ -780,9 +809,7 @@ def test_make_variant_dist_info_invalid_build_plugin() -> None:
780809

781810
with pytest.raises(
782811
ValidationError,
783-
match=r"Property 'test_namespace :: name1 :: val1d' is not installable "
784-
r"according to the respective provider plugin; is plugin-use == 'build' valid "
785-
"for this plugin?",
812+
match=r"Feature 'aot_plugin :: name1' is multi-value",
786813
):
787814
make_variant_dist_info(
788815
vdesc,

‎variantlib/api.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,11 @@ def make_variant_dist_info(
233233
if config.namespace not in build_namespaces:
234234
continue
235235
for vfeat in config.configs:
236+
if vfeat.multi_value:
237+
raise ValidationError(
238+
f"Feature '{config.namespace} :: {vfeat.name}' is "
239+
"multi-value, which is invalid for ahead-of-time plugins"
240+
)
236241
feature_prios = variant_json.feature_priorities.setdefault(
237242
config.namespace, []
238243
)
@@ -261,8 +266,8 @@ def make_variant_dist_info(
261266
):
262267
raise ValidationError(
263268
f"Property {vprop.to_str()!r} is not installable according to the "
264-
"respective provider plugin; is plugin-use == 'build' valid for "
265-
"this plugin?"
269+
"respective provider plugin, which is invalid for ahead-of-time "
270+
"plugins"
266271
)
267272

268273
return variant_json.to_str()

‎variantlib/plugins/loader.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ def _get_configs(
199199
namespace: ProviderConfig(
200200
namespace=namespace,
201201
configs=[
202-
VariantFeatureConfig(name=name, values=values, multi_value=True)
202+
VariantFeatureConfig(name=name, values=values, multi_value=False)
203203
for name, values in features.items()
204204
],
205205
)

0 commit comments

Comments
 (0)