From 2d8fab44939cfc2b32bc0693ed75a6f5141b3252 Mon Sep 17 00:00:00 2001 From: Fabien Chouteau Date: Fri, 18 Nov 2022 09:55:04 +0100 Subject: [PATCH] Fix crate config conflicts errors (#1208) * Alire.Properties.Configurations: print boolean values in lowercase To match casing of TOML files where they come from. * Alire.Crate_Configuration: fix, improve, and test value conflicts - TOML value comparison was incorrect (use Equals function now) - The "set by" info was incorrect - Error message now show the conflicting values --- src/alire/alire-crate_configuration.adb | 24 +++++++++++-------- src/alire/alire-crate_configuration.ads | 7 +++--- src/alire/alire-properties-configurations.adb | 2 +- src/alire/alire-properties-configurations.ads | 3 +++ .../he/hello_world/hello_world-0.1.0.toml | 14 +++++++++++ .../value_conflict/my_index/index/index.toml | 1 + .../libcrate_config_a-1.0.0.toml | 12 ++++++++++ .../libcrate_config_b-1.0.0.toml | 15 ++++++++++++ .../libcrate_config_c-1.0.0.toml | 15 ++++++++++++ .../tests/crate_config/value_conflict/test.py | 24 +++++++++++++++++++ .../crate_config/value_conflict/test.yaml | 4 ++++ .../he/hello_world/hello_world-0.1.0.toml | 14 +++++++++++ .../value_match/my_index/index/index.toml | 1 + .../libcrate_config_a-1.0.0.toml | 12 ++++++++++ .../libcrate_config_b-1.0.0.toml | 15 ++++++++++++ .../libcrate_config_c-1.0.0.toml | 15 ++++++++++++ .../tests/crate_config/value_match/test.py | 13 ++++++++++ .../tests/crate_config/value_match/test.yaml | 4 ++++ testsuite/tests/show/jekyll/test.py | 4 ++-- 19 files changed, 183 insertions(+), 16 deletions(-) create mode 100644 testsuite/tests/crate_config/value_conflict/my_index/index/he/hello_world/hello_world-0.1.0.toml create mode 100644 testsuite/tests/crate_config/value_conflict/my_index/index/index.toml create mode 100644 testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_a/libcrate_config_a-1.0.0.toml create mode 100644 testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_b/libcrate_config_b-1.0.0.toml create mode 100644 testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_c/libcrate_config_c-1.0.0.toml create mode 100644 testsuite/tests/crate_config/value_conflict/test.py create mode 100644 testsuite/tests/crate_config/value_conflict/test.yaml create mode 100644 testsuite/tests/crate_config/value_match/my_index/index/he/hello_world/hello_world-0.1.0.toml create mode 100644 testsuite/tests/crate_config/value_match/my_index/index/index.toml create mode 100644 testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_a/libcrate_config_a-1.0.0.toml create mode 100644 testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_b/libcrate_config_b-1.0.0.toml create mode 100644 testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_c/libcrate_config_c-1.0.0.toml create mode 100644 testsuite/tests/crate_config/value_match/test.py create mode 100644 testsuite/tests/crate_config/value_match/test.yaml diff --git a/src/alire/alire-crate_configuration.adb b/src/alire/alire-crate_configuration.adb index dfcddb60..aaee4f7c 100644 --- a/src/alire/alire-crate_configuration.adb +++ b/src/alire/alire-crate_configuration.adb @@ -235,7 +235,8 @@ package body Alire.Crate_Configuration is (Profile_Maps.Key (Cursor), (Name => +Builtin_Build_Profile.Name, Value => TOML.Create_String - (To_Lower_Case (Profile_Maps.Element (Cursor)'Img)))); + (To_Lower_Case (Profile_Maps.Element (Cursor)'Img))), + Set_By => "Build profile map"); end loop; end Make_Build_Profile_Map; @@ -792,15 +793,15 @@ package body Alire.Crate_Configuration is -- Set_Value -- --------------- - procedure Set_Value (This : in out Global_Config; - Crate : Crate_Name; - Val : Assignment) + procedure Set_Value (This : in out Global_Config; + Crate : Crate_Name; + Val : Assignment; + Set_By : String) is Val_Name_Lower : constant String := To_Lower_Case (+Val.Name); Crate_Str : constant String := +Crate; Name : constant Unbounded_String := (+Crate_Str) & "." & Val_Name_Lower; begin - -- TODO check if setting configuration of a dependency if not This.Var_Map.Contains (Name) then @@ -819,14 +820,16 @@ package body Alire.Crate_Configuration is "'" & " for type " & Image (Ref.Type_Def.Element)); end if; - if Ref.Value /= No_TOML_Value and then Ref.Value /= Val.Value then + if Ref.Value.Is_Present and then not Ref.Value.Equals (Val.Value) + then Raise_Checked_Error ("Conflicting value for configuration variable '" & - (+Name) & "' from '" & (+Ref.Set_By) & "' and '" - & (+Crate) & "'."); + (+Name) & "' from '" & (+Ref.Set_By) & "' (" & + Image (Ref.Value) & ") and '" + & Set_By & "' (" & Image (Val.Value) & ")."); else Ref.Value := Val.Value; - Ref.Set_By := +(+Crate); + Ref.Set_By := +(Set_By); end if; end; end Set_Value; @@ -852,7 +855,8 @@ package body Alire.Crate_Configuration is Config_Value_Assignment (Prop); begin for Elt of List.List loop - This.Set_Value (To_Name (+List.Crate), Elt); + This.Set_Value (To_Name (+List.Crate), Elt, + Set_By => As_String (Crate)); end loop; end; end loop; diff --git a/src/alire/alire-crate_configuration.ads b/src/alire/alire-crate_configuration.ads index e9e0f7d1..abf904bc 100644 --- a/src/alire/alire-crate_configuration.ads +++ b/src/alire/alire-crate_configuration.ads @@ -145,9 +145,10 @@ private Root : in out Roots.Root; Crate : Crate_Name); - procedure Set_Value (This : in out Global_Config; - Crate : Crate_Name; - Val : Assignment); + procedure Set_Value (This : in out Global_Config; + Crate : Crate_Name; + Val : Assignment; + Set_By : String); procedure Load_Settings (This : in out Global_Config; Root : in out Roots.Root; diff --git a/src/alire/alire-properties-configurations.adb b/src/alire/alire-properties-configurations.adb index 1c8c14b1..e3afa89f 100644 --- a/src/alire/alire-properties-configurations.adb +++ b/src/alire/alire-properties-configurations.adb @@ -144,7 +144,7 @@ package body Alire.Properties.Configurations is when TOML_String => return Val.As_String; when TOML_Boolean => - return Val.As_Boolean'Img; + return To_Lower_Case (Val.As_Boolean'Img); when TOML_Float => return Image (Val.As_Float); when TOML_Integer => diff --git a/src/alire/alire-properties-configurations.ads b/src/alire/alire-properties-configurations.ads index 6564a795..2c643bb5 100644 --- a/src/alire/alire-properties-configurations.ads +++ b/src/alire/alire-properties-configurations.ads @@ -101,6 +101,9 @@ package Alire.Properties.Configurations with Preelaborate is function Typedef_From_Enum return Config_Type_Definition; function String_Typedef (Name : String) return Config_Type_Definition; + + function Image (Val : TOML.TOML_Value) return String; + private type Config_Entry is new Properties.Property with record diff --git a/testsuite/tests/crate_config/value_conflict/my_index/index/he/hello_world/hello_world-0.1.0.toml b/testsuite/tests/crate_config/value_conflict/my_index/index/he/hello_world/hello_world-0.1.0.toml new file mode 100644 index 00000000..4ba0c935 --- /dev/null +++ b/testsuite/tests/crate_config/value_conflict/my_index/index/he/hello_world/hello_world-0.1.0.toml @@ -0,0 +1,14 @@ +description = "\"Hello, world!\" demonstration project" +name = "hello_world" +version = "0.1.0" +licenses = "GPL-3.0-only" +maintainers = ["example@example.com"] +maintainers-logins = ["mylogin"] +executables=['main'] + +[origin] +url = "file:." + +[[depends-on]] +libcrate_config_b = "*" +libcrate_config_c = "*" diff --git a/testsuite/tests/crate_config/value_conflict/my_index/index/index.toml b/testsuite/tests/crate_config/value_conflict/my_index/index/index.toml new file mode 100644 index 00000000..bad265e4 --- /dev/null +++ b/testsuite/tests/crate_config/value_conflict/my_index/index/index.toml @@ -0,0 +1 @@ +version = "1.1" diff --git a/testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_a/libcrate_config_a-1.0.0.toml b/testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_a/libcrate_config_a-1.0.0.toml new file mode 100644 index 00000000..39d58201 --- /dev/null +++ b/testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_a/libcrate_config_a-1.0.0.toml @@ -0,0 +1,12 @@ +description = "crate config demonstration project" +name = "libcrate_config_a" +version = "1.0.0" +licenses = "GPL-3.0-only" +maintainers = ["example@example.com"] +maintainers-logins = ["mylogin"] + +[origin] +url = "file:." + +[configuration.variables] +Var_Bool = {type="Boolean"} diff --git a/testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_b/libcrate_config_b-1.0.0.toml b/testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_b/libcrate_config_b-1.0.0.toml new file mode 100644 index 00000000..1277e7db --- /dev/null +++ b/testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_b/libcrate_config_b-1.0.0.toml @@ -0,0 +1,15 @@ +description = "crate config demonstration project" +name = "libcrate_config_b" +version = "1.0.0" +licenses = "GPL-3.0-only" +maintainers = ["example@example.com"] +maintainers-logins = ["mylogin"] + +[[depends-on]] +libcrate_config_a = "*" + +[origin] +url = "file:." + +[configuration.values] +libcrate_config_a.Var_Bool = true diff --git a/testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_c/libcrate_config_c-1.0.0.toml b/testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_c/libcrate_config_c-1.0.0.toml new file mode 100644 index 00000000..10cfb360 --- /dev/null +++ b/testsuite/tests/crate_config/value_conflict/my_index/index/li/libcrate_config_c/libcrate_config_c-1.0.0.toml @@ -0,0 +1,15 @@ +description = "crate config demonstration project" +name = "libcrate_config_c" +version = "1.0.0" +licenses = "GPL-3.0-only" +maintainers = ["example@example.com"] +maintainers-logins = ["mylogin"] + +[[depends-on]] +libcrate_config_a = "*" + +[origin] +url = "file:." + +[configuration.values] +libcrate_config_a.Var_Bool = false diff --git a/testsuite/tests/crate_config/value_conflict/test.py b/testsuite/tests/crate_config/value_conflict/test.py new file mode 100644 index 00000000..69ad4cc1 --- /dev/null +++ b/testsuite/tests/crate_config/value_conflict/test.py @@ -0,0 +1,24 @@ +""" +Test that two crates setting the same config var to the different values is not +ok. +""" + +from drivers.alr import run_alr +from drivers.asserts import assert_match + +import os +import platform + +p = run_alr('get', 'hello_world', complain_on_error=False) +assert p.status != 0, "alr should have errored" + +print(p.out) +assert_match(".*\n" + "ERROR: Conflicting value for configuration variable" + " 'libcrate_config_a.var_bool'" + " from 'libcrate_config_b' \(true\)" + " and 'libcrate_config_c' \(false\).*" + , + p.out) + +print('SUCCESS') diff --git a/testsuite/tests/crate_config/value_conflict/test.yaml b/testsuite/tests/crate_config/value_conflict/test.yaml new file mode 100644 index 00000000..0a859639 --- /dev/null +++ b/testsuite/tests/crate_config/value_conflict/test.yaml @@ -0,0 +1,4 @@ +driver: python-script +indexes: + my_index: + in_fixtures: false diff --git a/testsuite/tests/crate_config/value_match/my_index/index/he/hello_world/hello_world-0.1.0.toml b/testsuite/tests/crate_config/value_match/my_index/index/he/hello_world/hello_world-0.1.0.toml new file mode 100644 index 00000000..4ba0c935 --- /dev/null +++ b/testsuite/tests/crate_config/value_match/my_index/index/he/hello_world/hello_world-0.1.0.toml @@ -0,0 +1,14 @@ +description = "\"Hello, world!\" demonstration project" +name = "hello_world" +version = "0.1.0" +licenses = "GPL-3.0-only" +maintainers = ["example@example.com"] +maintainers-logins = ["mylogin"] +executables=['main'] + +[origin] +url = "file:." + +[[depends-on]] +libcrate_config_b = "*" +libcrate_config_c = "*" diff --git a/testsuite/tests/crate_config/value_match/my_index/index/index.toml b/testsuite/tests/crate_config/value_match/my_index/index/index.toml new file mode 100644 index 00000000..bad265e4 --- /dev/null +++ b/testsuite/tests/crate_config/value_match/my_index/index/index.toml @@ -0,0 +1 @@ +version = "1.1" diff --git a/testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_a/libcrate_config_a-1.0.0.toml b/testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_a/libcrate_config_a-1.0.0.toml new file mode 100644 index 00000000..f55a82d7 --- /dev/null +++ b/testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_a/libcrate_config_a-1.0.0.toml @@ -0,0 +1,12 @@ +description = "crate config demonstration project" +name = "libcrate_config_a" +version = "1.0.0" +licenses = "GPL-3.0-only" +maintainers = ["example@example.com"] +maintainers-logins = ["mylogin"] + +[origin] +url = "file:." + +[configuration.variables] +Var_Bool = {type="Boolean", default=true} diff --git a/testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_b/libcrate_config_b-1.0.0.toml b/testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_b/libcrate_config_b-1.0.0.toml new file mode 100644 index 00000000..feea8050 --- /dev/null +++ b/testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_b/libcrate_config_b-1.0.0.toml @@ -0,0 +1,15 @@ +description = "crate config demonstration project" +name = "libcrate_config_b" +version = "1.0.0" +licenses = "GPL-3.0-only" +maintainers = ["example@example.com"] +maintainers-logins = ["mylogin"] + +[[depends-on]] +libcrate_config_a = "*" + +[origin] +url = "file:." + +[configuration.values] +libcrate_config_a.Var_Bool = false diff --git a/testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_c/libcrate_config_c-1.0.0.toml b/testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_c/libcrate_config_c-1.0.0.toml new file mode 100644 index 00000000..10cfb360 --- /dev/null +++ b/testsuite/tests/crate_config/value_match/my_index/index/li/libcrate_config_c/libcrate_config_c-1.0.0.toml @@ -0,0 +1,15 @@ +description = "crate config demonstration project" +name = "libcrate_config_c" +version = "1.0.0" +licenses = "GPL-3.0-only" +maintainers = ["example@example.com"] +maintainers-logins = ["mylogin"] + +[[depends-on]] +libcrate_config_a = "*" + +[origin] +url = "file:." + +[configuration.values] +libcrate_config_a.Var_Bool = false diff --git a/testsuite/tests/crate_config/value_match/test.py b/testsuite/tests/crate_config/value_match/test.py new file mode 100644 index 00000000..8c163114 --- /dev/null +++ b/testsuite/tests/crate_config/value_match/test.py @@ -0,0 +1,13 @@ +""" +Test that two crates setting the same config var to the same value is ok. +""" + +from drivers.alr import run_alr +from drivers.asserts import assert_match + +import os +import platform + +p = run_alr('get', 'hello_world') + +print('SUCCESS') diff --git a/testsuite/tests/crate_config/value_match/test.yaml b/testsuite/tests/crate_config/value_match/test.yaml new file mode 100644 index 00000000..0a859639 --- /dev/null +++ b/testsuite/tests/crate_config/value_match/test.yaml @@ -0,0 +1,4 @@ +driver: python-script +indexes: + my_index: + in_fixtures: false diff --git a/testsuite/tests/show/jekyll/test.py b/testsuite/tests/show/jekyll/test.py index cee9485f..0f0aa78c 100644 --- a/testsuite/tests/show/jekyll/test.py +++ b/testsuite/tests/show/jekyll/test.py @@ -29,8 +29,8 @@ assert_eq( '{name: \'Var5\', type: \'Integer range -1 .. 1\', default: "0"},\n' '{name: \'Var6\', type: \'Real range -1.00000000000000E+00 .. 1.00000000000000E+00\', default: "0.00000000000000E+00"},\n' '{name: \'Var7\', type: \'Real range -inf .. +inf\', default: "0.00000000000000E+00"}]\n' - 'configuration_values: [{crate: \'hello\', settings: [{name: \'Var1\', value: "TRUE"}]},\n' - '{crate: \'libhello\', settings: [{name: \'Var1\', value: "FALSE"}]}]\n' + 'configuration_values: [{crate: \'hello\', settings: [{name: \'Var1\', value: "true"}]},\n' + '{crate: \'libhello\', settings: [{name: \'Var1\', value: "false"}]}]\n' '\n' '---\n' 'This is an example of long description in a multi-line string.\n' -- 2.39.5