From bb04c93b87a893f340a7f36312f4da6dbd967e60 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Mon, 29 Jan 2024 12:09:03 +0100 Subject: [PATCH] Fix regex for version detection of Fedora externals (#1545) * Fix improper regex in rpm-based detector * Test for bugfix * Document pending breaking changes These require an `alr` or index version bump * Self-review --- BREAKING.md | 9 +++ .../alire-origins-deployers-system-apt.adb | 6 +- .../alire-origins-deployers-system-pacman.adb | 5 +- ...-origins-deployers-system-rpm_wrappers.adb | 14 +++-- .../alire-origins-deployers-system-zypper.adb | 6 +- src/alire/alire-selftest.adb | 41 ++++++++++++- src/alire/alire-selftest.ads | 2 + src/alire/alire-utils-regex.adb | 61 +++++++++++++++++++ src/alire/alire-utils-regex.ads | 15 +++++ src/alire/alire-utils.adb | 37 ----------- src/alire/alire-utils.ads | 8 --- 11 files changed, 147 insertions(+), 57 deletions(-) create mode 100644 BREAKING.md create mode 100644 src/alire/alire-utils-regex.adb create mode 100644 src/alire/alire-utils-regex.ads diff --git a/BREAKING.md b/BREAKING.md new file mode 100644 index 00000000..c23c3980 --- /dev/null +++ b/BREAKING.md @@ -0,0 +1,9 @@ +Log of breaking changes in index or alr. + +### alr 2.0.0 + index 1.2.2 + +### alr 1.2.2 + index 1.2.1 + +- Unable to load externals containing regex special characters in the system +package name (fixed in #1545). +- Paths in `[environment]` are not converted to the native platform convention. diff --git a/src/alire/alire-origins-deployers-system-apt.adb b/src/alire/alire-origins-deployers-system-apt.adb index b9adb121..4f4db6ff 100644 --- a/src/alire/alire-origins-deployers-system-apt.adb +++ b/src/alire/alire-origins-deployers-system-apt.adb @@ -1,7 +1,8 @@ with AAA.Strings; use AAA.Strings; -with Alire.OS_Lib.Subprocess; with Alire.Errors; +with Alire.OS_Lib.Subprocess; +with Alire.Utils.Regex; package body Alire.Origins.Deployers.System.Apt is @@ -62,7 +63,8 @@ package body Alire.Origins.Deployers.System.Apt is if Contains (Line, "Version:") then Trace.Debug ("Extracting native version from apt output: " & Line); declare - Match : constant String := Utils.First_Match (Regexp, Line); + Match : constant String := + Utils.Regex.First_Match (Regexp, Line); begin if Match /= "" then Trace.Debug ("Candidate version string: " & Match); diff --git a/src/alire/alire-origins-deployers-system-pacman.adb b/src/alire/alire-origins-deployers-system-pacman.adb index 6a87bff5..4aad7bab 100644 --- a/src/alire/alire-origins-deployers-system-pacman.adb +++ b/src/alire/alire-origins-deployers-system-pacman.adb @@ -1,7 +1,8 @@ with AAA.Strings; use AAA.Strings; -with Alire.OS_Lib.Subprocess; with Alire.Errors; +with Alire.OS_Lib.Subprocess; +with Alire.Utils.Regex; package body Alire.Origins.Deployers.System.Pacman is @@ -81,7 +82,7 @@ package body Alire.Origins.Deployers.System.Pacman is declare Match : constant String := - Utils.First_Match (Regexp, Package_Line); + Utils.Regex.First_Match (Regexp, Package_Line); begin if Match /= "" then Trace.Detail ("Candidate version string: " & Match); diff --git a/src/alire/alire-origins-deployers-system-rpm_wrappers.adb b/src/alire/alire-origins-deployers-system-rpm_wrappers.adb index 51bcaba9..9c615ed5 100644 --- a/src/alire/alire-origins-deployers-system-rpm_wrappers.adb +++ b/src/alire/alire-origins-deployers-system-rpm_wrappers.adb @@ -1,8 +1,9 @@ with AAA.Strings; use AAA.Strings; -with Alire.OS_Lib.Subprocess; with Alire.Errors; +with Alire.OS_Lib.Subprocess; with Alire.Platforms.Current; +with Alire.Utils.Regex; package body Alire.Origins.Deployers.System.RPM_Wrappers is @@ -83,9 +84,12 @@ package body Alire.Origins.Deployers.System.RPM_Wrappers is function Detect_Not_Installed return Version_Outcomes.Outcome is + -- Package name at beginning of line, architecture, version Regexp : constant String := - "^" & AAA.Strings.To_Lower_Case (Full_Pkg_Name) & - "[^\s]*\s+(?:\d+:)?([0-9.]+)"; + "^" + & Utils.Regex.Escape + (AAA.Strings.To_Lower_Case (Full_Pkg_Name)) + & "[^\s]*\s+(?:\d+:)?([0-9.]+)"; -- A line looks like: -- gtk3.x86_64 1:3.24.24-1.fc33 updates @@ -106,8 +110,8 @@ package body Alire.Origins.Deployers.System.RPM_Wrappers is " output: " & Line & " with regex " & Regexp); declare Match : constant String := - Utils.First_Match (Regexp, - AAA.Strings.To_Lower_Case (Line)); + Utils.Regex.First_Match + (Regexp, AAA.Strings.To_Lower_Case (Line)); begin if Match /= "" then Trace.Debug ("Candidate version string: " & Match); diff --git a/src/alire/alire-origins-deployers-system-zypper.adb b/src/alire/alire-origins-deployers-system-zypper.adb index 61a9be22..fca28d6b 100644 --- a/src/alire/alire-origins-deployers-system-zypper.adb +++ b/src/alire/alire-origins-deployers-system-zypper.adb @@ -1,7 +1,8 @@ with AAA.Strings; use AAA.Strings; -with Alire.OS_Lib.Subprocess; with Alire.Errors; +with Alire.OS_Lib.Subprocess; +with Alire.Utils.Regex; package body Alire.Origins.Deployers.System.Zypper is @@ -70,7 +71,8 @@ package body Alire.Origins.Deployers.System.Zypper is Trace.Debug ("Extracting native version from zypper output: " & Line); declare - Match : constant String := Utils.First_Match (Regexp, Line); + Match : constant String := + Utils.Regex.First_Match (Regexp, Line); begin if Match /= "" then Trace.Debug ("Candidate version string: " & Match); diff --git a/src/alire/alire-selftest.adb b/src/alire/alire-selftest.adb index 9d2ab80e..cdaf422a 100644 --- a/src/alire/alire-selftest.adb +++ b/src/alire/alire-selftest.adb @@ -1,7 +1,9 @@ -with Alire.Utils; with Alire.Config.Edit; +with Alire.Utils.Regex; with Alire.VCSs.Git; +with GNAT.Regpat; + package body Alire.Selftest is -- Tests are Check_* procedures that end normally or raise some exception. @@ -153,6 +155,42 @@ package body Alire.Selftest is "https://github.com/user/project"); end Check_Git_To_HTTP; + -------------------------- + -- Check_Regex_Escaping -- + -------------------------- + + procedure Check_Regex_Escaping is + begin + -- See issue #1545 + + -- This should succeed + declare + Match : constant String := Utils.Regex.First_Match + ("^" + & Utils.Regex.Escape ("libstdc++-static") + & "[^\s]*\s+(?:\d+:)?([0-9.]+)", + "libstdc++-static.x86_64 1:2.3.4-5.fc33 updates"); + begin + pragma Assert (Match = "2.3.4", "Match was: " & Match); + end; + + -- This should "fail" + begin + declare + Match : constant String := Utils.Regex.First_Match + ("^libstdc++-static" + & "[^\s]*\s+(?:\d+:)?([0-9.]+)", + "libstdc++-static.x86_64 1:2.3.4-5.fc33 updates") + with Unreferenced; + begin + raise Program_Error with "Previous call should have raised"; + end; + exception + when GNAT.Regpat.Expression_Error => + null; -- Expected + end; + end Check_Regex_Escaping; + --------- -- Run -- --------- @@ -163,6 +201,7 @@ package body Alire.Selftest is Check_Email_Checks; Check_GitHub_Logins; Check_Git_To_HTTP; + Check_Regex_Escaping; Trace.Detail ("Self-checks passed"); exception diff --git a/src/alire/alire-selftest.ads b/src/alire/alire-selftest.ads index 7a52b7b4..59128c1c 100644 --- a/src/alire/alire-selftest.ads +++ b/src/alire/alire-selftest.ads @@ -4,4 +4,6 @@ package Alire.Selftest is -- Runs all self-tests. Should end silently or raise some exception on -- failure. + -- This should eventually be moved to an Ada-based unit testing suite + end Alire.Selftest; diff --git a/src/alire/alire-utils-regex.adb b/src/alire/alire-utils-regex.adb new file mode 100644 index 00000000..323839ad --- /dev/null +++ b/src/alire/alire-utils-regex.adb @@ -0,0 +1,61 @@ +with GNAT.Regpat; + +package body Alire.Utils.Regex is + + ------------ + -- Escape -- + ------------ + + function Escape (Literal : String) return String is + Targets : constant String := "\()[].*+?^"; + Result : Ustring; + use Ustrings; + begin + for Char of Literal loop + if (for some Nono of Targets => Char = Nono) then + Append (Result, "\" & Char); + else + Append (Result, Char); + end if; + end loop; + + return +Result; + end Escape; + + ----------------- + -- First_Match -- + ----------------- + + function First_Match (Regex : String; Text : String) return String is + + ----------------------- + -- Count_Parentheses -- + ----------------------- + + function Count_Parentheses return Positive is + Count : Natural := 0; + begin + for Char of Regex loop + if Char = '(' then + Count := Count + 1; + end if; + end loop; + return Count; + end Count_Parentheses; + + use GNAT.Regpat; + Matches : Match_Array (1 .. Count_Parentheses); + -- This is a safe estimation, as some '(' may not be part of a capture + + begin + Match (Regex, Text, Matches); + + for I in Matches'Range loop + if Matches (I) /= No_Match then + return Text (Matches (I).First .. Matches (I).Last); + end if; + end loop; + + return ""; + end First_Match; +end Alire.Utils.Regex; diff --git a/src/alire/alire-utils-regex.ads b/src/alire/alire-utils-regex.ads new file mode 100644 index 00000000..30c32c8b --- /dev/null +++ b/src/alire/alire-utils-regex.ads @@ -0,0 +1,15 @@ +package Alire.Utils.Regex is + + function Escape (Literal : String) return String; + -- Prepare a string to be interpreted literally by GNAT.Regpat, like e.g. + -- "stdc++" --> "stdc\+\+" + + function First_Match (Regex : String; Text : String) return String + with Pre => (for some Char of Regex => Char = '('); + -- Wrapper on GNAT.Regpat. It returns the first match found, which is not + -- necessarily the first parenthesized expression. E.g., in a pattern like: + -- (abc)|(efg), it will return the "efg" match, even if to GNAT.Regpat that + -- is the second matching expression. In case of no match, it will return + -- an empty string. At least one capture must be attempted in the Regex. + +end Alire.Utils.Regex; diff --git a/src/alire/alire-utils.adb b/src/alire/alire-utils.adb index 9de24c5e..1f8efb48 100644 --- a/src/alire/alire-utils.adb +++ b/src/alire/alire-utils.adb @@ -129,43 +129,6 @@ package body Alire.Utils is end return; end Count_True; - ----------------- - -- First_Match -- - ----------------- - - function First_Match (Regex : String; Text : String) return String is - - ----------------------- - -- Count_Parentheses -- - ----------------------- - - function Count_Parentheses return Positive is - Count : Natural := 0; - begin - for Char of Regex loop - if Char = '(' then - Count := Count + 1; - end if; - end loop; - return Count; - end Count_Parentheses; - - use GNAT.Regpat; - Matches : Match_Array (1 .. Count_Parentheses); - -- This is a safe estimation, as some '(' may not be part of a capture - - begin - Match (Regex, Text, Matches); - - for I in Matches'Range loop - if Matches (I) /= No_Match then - return Text (Matches (I).First .. Matches (I).Last); - end if; - end loop; - - return ""; - end First_Match; - ------------------------------- -- Is_Valid_Full_Person_Name -- ------------------------------- diff --git a/src/alire/alire-utils.ads b/src/alire/alire-utils.ads index cbbdaa0b..4b26227a 100644 --- a/src/alire/alire-utils.ads +++ b/src/alire/alire-utils.ads @@ -84,14 +84,6 @@ package Alire.Utils with Preelaborate is -- Flatten String keys of Indefinite_Ordered_Maps into string -- representation. - function First_Match (Regex : String; Text : String) return String - with Pre => (for some Char of Regex => Char = '('); - -- Wrapper on GNAT.Regpat. It returns the first match found, which is not - -- necessarily the first parenthesized expression. E.g., in a pattern like: - -- (abc)|(efg), it will return the "efg" match, even if to GNAT.Regpat that - -- is the second matching expression. In case of no match, it will return - -- an empty string. At least one capture must be attempted in the Regex. - private function Quote (S : String) return String -- 2.39.5