From 603e8bed22f7fec99f6da16b57f598f7e16e3e49 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Fri, 14 Aug 2020 17:54:37 +0200 Subject: [PATCH] Ignore unknown fields in manifest with `--force` (#495) * Use manifest source (local, index) for finer errors * Generate `metadata-version` during release saving * Script to prepend manifest metadata versions Not really necessary since we don't expect index manifests to include such metadata version. Still, it makes clear the change. * Testsuite adjustments for extra error output Since we now take advantage of the manifest provenance (index/local), some error messages require tweaking. * Review: remove `metadata-version` field * Allow unknown metadata fields with --force Usually new fields will correspond with new features. Ignoring these from an old `alr` version might be good enough for some commands to still work. * Test for ignoring unknown properties --- src/alire/alire-properties-from_toml.adb | 79 +++++++++++-------- src/alire/alire-toml_adapters.adb | 25 +++++- src/alire/alire-toml_adapters.ads | 5 ++ src/alire/alire.adb | 4 +- src/alire/alire.ads | 4 +- testsuite/drivers/alr.py | 3 + .../index/ignore-unknown-property/test.py | 32 ++++++++ .../index/ignore-unknown-property/test.yaml | 1 + 8 files changed, 113 insertions(+), 40 deletions(-) create mode 100644 testsuite/tests/index/ignore-unknown-property/test.py create mode 100644 testsuite/tests/index/ignore-unknown-property/test.yaml diff --git a/src/alire/alire-properties-from_toml.adb b/src/alire/alire-properties-from_toml.adb index 23b15824..80d08e19 100644 --- a/src/alire/alire-properties-from_toml.adb +++ b/src/alire/alire-properties-from_toml.adb @@ -1,3 +1,5 @@ +with AAA.Enum_Tools; + with Alire.TOML_Expressions.Cases; with Alire.Utils; @@ -18,50 +20,59 @@ package body Alire.Properties.From_TOML is return Props : Conditional.Properties do loop declare - Val : TOML.TOML_Value; - Key : constant String := From.Pop (Val); - Prop : Property_Keys; + function Is_Valid is + new AAA.Enum_Tools.Is_Valid (Property_Keys); + Val : TOML.TOML_Value; + Key : constant String := From.Pop (Val); + Prop : Property_Keys; + Ada_Key : constant String := TOML_Adapters.Adafy (Key); begin if Key = "" then - return; + return; -- No more keys end if; -- Extract property name from string Trace.Debug ("Loading property key = " & Key); - begin - Prop := Property_Keys'Value (TOML_Adapters.Adafy (Key)); - -- It is a valid key - exception - when Constraint_Error => - From.Checked_Error ("invalid property: " & Key); - end; + Process_Property : -- Single-pass loop to emulate Continue + loop + if Is_Valid (Ada_Key) then + Prop := Property_Keys'Value (TOML_Adapters.Adafy (Key)); - -- Check that the property is expected in this section. - if Loaders (Prop) = null then - From.Checked_Error ("property '" & Key - & "' must not appear in section " - & Utils.To_Lower_Case (Section'Image)); - end if; + -- Check that the property is expected in this section. + if Loaders (Prop) = null then + From.Recoverable_Error + ("property '" & Key + & "' must not appear in section " + & Utils.To_Lower_Case (Section'Image)); + exit Process_Property; + end if; + + -- Divert to the expr resolver if prop can be dynamic: + if Loaders_During_Case (Prop) /= null then + Props := Props and + TOML_Expressions.Cases.Load_Property + (Key => Key, + From => From.Descend (Val, "property"), + Loader => Loaders_During_Case (Prop)); + else + -- Ensure no dynamic expression in the incoming values + if TOML_Expressions.Contains_Expression (Val) then + From.Checked_Error + ("property '" & Key + & "' must not contain dynamic expressions"); + end if; - -- Divert to the expr resolver if prop can be dynamic: - if Loaders_During_Case (Prop) /= null then - Props := Props and - TOML_Expressions.Cases.Load_Property - (Key => Key, - From => From.Descend (Val, "property"), - Loader => Loaders_During_Case (Prop)); - else - -- Ensure no dynamic expression in the incoming values: - if TOML_Expressions.Contains_Expression (Val) then - From.Checked_Error - ("property '" & Key - & "' must not contain dynamic expressions"); + -- Actually load the property: + Props := Props and Loaders (Prop) + (From.Descend (Key, Val, Key)); + end if; + + else + From.Recoverable_Error ("invalid property: " & Key); end if; - -- Actually load the property: - Props := Props and Loaders (Prop) - (From.Descend (Key, Val, Key)); - end if; + exit Process_Property; + end loop Process_Property; end; end loop; end return; diff --git a/src/alire/alire-toml_adapters.adb b/src/alire/alire-toml_adapters.adb index 2721709a..b1cdcdcb 100644 --- a/src/alire/alire-toml_adapters.adb +++ b/src/alire/alire-toml_adapters.adb @@ -41,6 +41,22 @@ package body Alire.TOML_Adapters is raise Alire.Checked_Error with Errors.Set (Queue.Message (Message)); end Checked_Error; + ----------------------- + -- Recoverable_Error -- + ----------------------- + + procedure Recoverable_Error (Queue : Key_Queue; + Message : String; + Recover : Boolean := Alire.Force) + is + begin + if Recover then + Recoverable_Error (Queue.Message (Message), Recover); + else + Queue.Checked_Error (Message); + end if; + end Recoverable_Error; + ----------------- -- Checked_Pop -- ----------------- @@ -225,7 +241,12 @@ package body Alire.TOML_Adapters is end loop; if Errored then - return Outcome_Failure (+Message); + if Force then + Recoverable_Error (+Message); + return Outcome_Success; + else + return Outcome_Failure (+Message); + end if; else return Outcome_Success; end if; @@ -238,7 +259,7 @@ package body Alire.TOML_Adapters is procedure Report_Extra_Keys (Queue : Key_Queue) is Result : constant Outcome := Queue.Report_Extra_Keys; begin - if not Result.Success then + if not Force and then not Result.Success then raise Alire.Checked_Error with Errors.Set (Message (Result)); end if; end Report_Extra_Keys; diff --git a/src/alire/alire-toml_adapters.ads b/src/alire/alire-toml_adapters.ads index 44d11725..bc4e169b 100644 --- a/src/alire/alire-toml_adapters.ads +++ b/src/alire/alire-toml_adapters.ads @@ -48,6 +48,11 @@ package Alire.TOML_Adapters with Preelaborate is No_Return; -- Raise a Checked error with Context & ": " & Message, using Alire.Errors. + procedure Recoverable_Error (Queue : Key_Queue; + Message : String; + Recover : Boolean := Alire.Force); + -- As Checked_Error, but emit a warning instead when Recover is True + function Checked_Pop (Queue : Key_Queue; Key : String; Kind : TOML.Any_Value_Kind) diff --git a/src/alire/alire.adb b/src/alire/alire.adb index 511634fd..939d65ea 100644 --- a/src/alire/alire.adb +++ b/src/alire/alire.adb @@ -193,9 +193,9 @@ package body Alire is -- Recoverable_Error -- ----------------------- - procedure Recoverable_Error (Msg : String) is + procedure Recoverable_Error (Msg : String; Recover : Boolean := Force) is begin - if Force then + if Recover then Trace.Warning (Msg); else Raise_Checked_Error (Msg); diff --git a/src/alire/alire.ads b/src/alire/alire.ads index dc5e1511..5dfb58e6 100644 --- a/src/alire/alire.ads +++ b/src/alire/alire.ads @@ -194,8 +194,8 @@ package Alire with Preelaborate is -- message (Msg) and raise Checked_Error. There is no limitation on the -- length of Msg. - procedure Recoverable_Error (Msg : String); - -- When Force, emit a warning and return normally. When not Force call + procedure Recoverable_Error (Msg : String; Recover : Boolean := Force); + -- When Recover, emit a warning and return normally. When not Recover call -- Raise_Checked_Error instead. --------------- diff --git a/testsuite/drivers/alr.py b/testsuite/drivers/alr.py index 7aecffd8..fe3b27d9 100644 --- a/testsuite/drivers/alr.py +++ b/testsuite/drivers/alr.py @@ -55,6 +55,7 @@ def run_alr(*args, **kwargs): complain_on_error = kwargs.pop('complain_on_error', True) debug = kwargs.pop('debug', True) + force = kwargs.pop('force', False) quiet = kwargs.pop('quiet', True) if kwargs: first_unknown_kwarg = sorted(kwargs)[0] @@ -64,6 +65,8 @@ def run_alr(*args, **kwargs): argv.append('-n') # always non-interactive if debug: argv.append('-d') + if force: + argv.append('-f') if quiet: argv.append('-q') argv.extend(args) diff --git a/testsuite/tests/index/ignore-unknown-property/test.py b/testsuite/tests/index/ignore-unknown-property/test.py new file mode 100644 index 00000000..65ec2a52 --- /dev/null +++ b/testsuite/tests/index/ignore-unknown-property/test.py @@ -0,0 +1,32 @@ +""" +Check that unknown properties can be force-ignored +""" + +from drivers.alr import run_alr +from drivers.asserts import assert_match +from os import chdir + +import re + +# Create a regular working release +run_alr("init", "--bin", "xxx") +chdir("xxx") + +# Add spurious metadata +with open("alire/xxx.toml", "r") as file: + lines = file.readlines() +with open("alire/xxx.toml", "w") as file: + file.write("fancy-new-feat = false\n") + file.writelines(lines) + +# Verify the regular error +p = run_alr('show', complain_on_error=False) +assert p.status != 0, "command should have errored" +assert_match(".*invalid property:.*", p.out, flags=re.S) + +# Verify the force-ignore +p = run_alr('show', quiet=False, force=True) # Should not complain +assert_match(".*Warning:.*invalid property:.*", # Should be a warning + p.out, flags=re.S) + +print("SUCCESS") diff --git a/testsuite/tests/index/ignore-unknown-property/test.yaml b/testsuite/tests/index/ignore-unknown-property/test.yaml new file mode 100644 index 00000000..32c747b3 --- /dev/null +++ b/testsuite/tests/index/ignore-unknown-property/test.yaml @@ -0,0 +1 @@ +driver: python-script -- 2.39.5