Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CA-400560: Support tilde in RPM version/release comparison #6092

Merged
merged 2 commits into from
Nov 5, 2024
Merged

CA-400560: Support tilde in RPM version/release comparison #6092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
395d5ad
CA-400560: Fix version segment division error
changlei-li Oct 29, 2024
a6187d5
CA-400560: Support tilde in RPM version/release comparison
changlei-li Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions ocaml/tests/test_rpm.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ module PkgOfFullnameTest = Generic.MakeStateless (struct
}
)
)
; ( Io.Line "libpath-utils-2:0.2.1~rc1-29.xs8~2_1.x86_64"
, Ok
(Some
Pkg.
{
name= "libpath-utils"
; epoch= Some 2
; version= "0.2.1~rc1"
; release= "29.xs8~2_1"
; arch= "x86_64"
}
)
)
; (Io.Line "libpath-utils-:0.2.1-29.el7.x86_64", Ok None)
; (Io.Line "libpath-utils-2:0.2.1-29.el7x86_64", Ok None)
; (* all RPM packages installed by default *)
Expand Down Expand Up @@ -163,14 +176,23 @@ module PkgCompareVersionStringsTest = Generic.MakeStateless (struct
; (("1.0", "1.a"), ">")
; (("2.50", "2.5"), ">")
; (("XS3", "xs2"), "<")
; (("1.2.3", "1.2.3a"), ">")
; (("1.2.3", "1.2.3a"), "<")
; (("xs4", "xs.4"), "=")
; (("2a", "2.0"), "<")
; (("2a", "2b"), "<")
; (("1.0", "1.xs2"), ">")
; (("1.0_xs", "1.0.xs"), "=")
; (("1.0x3", "1.0x04"), ">")
; (("1.0O3", "1.0O04"), ">")
; (("1.0x3", "1.0x04"), "<")
; (("1.0O3", "1.0O04"), "<")
; (("1.2.3", "1.2.3~rc1"), ">")
; (("1.2.3~rc1", "1.2.3~rc2"), "<")
; (("1.2.3~rc1", "1.2.3~rc1"), "=")
; (("1.2.3~rc1", "1.2.3~rc1.1"), "<")
; (("1.2.3~rc1.1", "1.2.3~rc1.2"), "<")
; (("1.2.3~rc1.1", "1.2.3~rc1_1"), "=")
; (("1.2.3.xs8", "1.2.3.xs8~2_1"), ">")
; (("1.2.3.xs8~2_1", "1.2.3.xs8~2_1~beta"), ">")
; (("1.2.3.xs8~", "1.2.3.xs8"), "<")
]
end)

Expand Down
96 changes: 52 additions & 44 deletions ocaml/xapi/rpm.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@ module Pkg = struct

type order = LT | EQ | GT

type segment_of_version = Int of int | Str of string
type version_segment = Int of int | Str of string | Tilde

let string_of_order = function LT -> "<" | EQ -> "=" | GT -> ">"

let order_of_int = function 0 -> EQ | r when r > 0 -> GT | _ -> LT

let error_msg = Printf.sprintf "Failed to parse '%s'"

let parse_epoch_version_release epoch_ver_rel =
Expand Down Expand Up @@ -157,9 +159,41 @@ module Pkg = struct
| None, None ->
EQ

let compare_version_segment s1 s2 =
match (s1, s2) with
| Int i1, Int i2 ->
Int.compare i1 i2 |> order_of_int
| Str s1, Str s2 ->
String.compare s1 s2 |> order_of_int
| Tilde, Tilde ->
EQ
| Int _, Str _ ->
GT
| Str _, Int _ ->
LT
| Tilde, _ ->
LT
| _, Tilde ->
GT

let split_version_string =
let r = Re.Posix.compile_pat {|[a-zA-Z]+|[0-9]+|~|} in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to detect when a version string contains illegal characters? For example, the above regex does not allow a underscore to be part of the string.

Copy link
Contributor Author

@changlei-li changlei-li Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All characters expect for [alphabetic]+, [digit]+ and ~ are acted as delimiter and omitted in the results:

1.2#3_45abc -> ["1"; "2"; "3"; "45"; "abc"]
1.2.3α45 -> ["1"; "2"; "3"; "45"]
12汉字45 -> ["12"; "45"]

fun s -> s |> Re.all r |> List.map (fun g -> Re.Group.get g 0)

let normalize v =
let version_segment_of_string = function
| "~" ->
Tilde
| s -> (
try Int (int_of_string s) with _ -> Str s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be aware that int_of_string "0xabc" does not throw an exception as you might expect.,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. No need to test the string is all digits when convert string to Int as the original implementation, because the split method has promised the string is all digits or all alphabetic. There is no situation like "0b11" may mislead int_of_string. So just use int_of_string exception to distinguish Str and Int.
Due to the function has precondition that the input is from the split result, it is not a complete function to convert string to version_segment. So I just put it in the normalize function scope.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative would be to use String.sscanf "%d". But you are correct that the problem does not arise because the split makes sure this is only digtits or only letters.

)
in
v |> split_version_string |> List.map version_segment_of_string

let compare_version_strings s1 s2 =
(* Compare versions or releases of RPM packages
* I.E. for "libpath-utils-0.2.1-29.el7.x86_64" and "libpath-utils-0.2.1a-30.el7.x86_64",
* I.E. for "libpath-utils-0.2.1-29.el7.x86_64" and
* "libpath-utils-0.2.1a-30.el7.x86_64",
* this function compares:
* versions between "0.2.1" and "0.2.1a", or
* releases between "29.el7" and "30.el7".
Expand All @@ -173,58 +207,32 @@ module Pkg = struct
* "1.0" ">" "1.a"
* "2.50" ">" "2.5"
* "XS3" "<" "xs2"
* "1.2.3" ">" "1.2.3a"
changlei-li marked this conversation as resolved.
Show resolved Hide resolved
* "1.2.3" "<" "1.2.3a"
* "xs4" "=" "xs.4"
* "2a" "<" "2.0"
* "2a" "<" "2b"
* "1.0" ">" "1.xs2"
* "1.0_xs" "=" "1.0.xs"
* "1.xs8" ">" "1.xs8~2_1"
* "1.2.3" ">" "1.2.3~beta"
* Some corner cases that don't follow standard RPM versioning conventions
* with tilde:
* "1.2.3~rc1~beta" "<" "1.2.3~rc1"
* "1.2.3~" "<" "1.2.3"
*)
let normalize v =
let split_letters_and_numbers s =
let r = Re.Posix.compile_pat {|^([^0-9]+)([0-9]+)$|} in
match Re.exec_opt r s with
| Some groups ->
[Re.Group.get groups 1; Re.Group.get groups 2]
| None ->
[s]
in
let number = Re.Posix.compile_pat "^[0-9]+$" in
v
|> Astring.String.cuts ~sep:"."
|> List.concat_map (fun s -> Astring.String.cuts ~sep:"_" s)
|> List.concat_map (fun s -> split_letters_and_numbers s)
|> List.map (fun s ->
if Re.execp number s then
match int_of_string s with i -> Int i | exception _ -> Str s
else
Str s
)
in
let rec compare_segments l1 l2 =
match (l1, l2) with
| c1 :: t1, c2 :: t2 -> (
match (c1, c2) with
| Int s1, Int s2 ->
if s1 > s2 then
GT
else if s1 = s2 then
compare_segments t1 t2
else
LT
| Int _, Str _ ->
GT
| Str _, Int _ ->
LT
| Str s1, Str s2 ->
let r = String.compare s1 s2 in
if r < 0 then
LT
else if r > 0 then
GT
else
compare_segments t1 t2
match compare_version_segment c1 c2 with
| EQ ->
compare_segments t1 t2
| r ->
r
)
| Tilde :: _, [] ->
LT
| [], Tilde :: _ ->
GT
| _ :: _, [] ->
GT
| [], _ :: _ ->
Expand Down
Loading