Skip to content

Commit 494ea8f

Browse files
committed
Sort by version in Distro.compare
Distro.compare was previously a little too lexicographic, leaving Debian 10 as less than Debian 9. Revised version maintains lexicographic ordering of distribution names, while sorting by version number within each distribution.
1 parent c4822cc commit 494ea8f

File tree

4 files changed

+154
-6
lines changed

4 files changed

+154
-6
lines changed

src-opam/distro.ml

Lines changed: 116 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
(** Distro selection for various OPAM combinations. *)
1919

20-
open Astring
2120
open Sexplib.Conv
2221

2322
type distro =
@@ -1358,7 +1357,119 @@ let base_distro_tag ?(arch = `X86_64) d =
13581357
in
13591358
("mcr.microsoft.com/windows/server", tag)
13601359

1361-
let compare a b =
1362-
String.compare
1363-
(human_readable_string_of_distro a)
1364-
(human_readable_string_of_distro b)
1360+
let sort_key_of_distro (d : t) =
1361+
if d = `Debian `Stable then
1362+
50997 (* "Compatibility" with use of human_readable_string_of_distro before,
1363+
giving `Debian `Stable < `Debian `Testing but greater than all
1364+
numbered releases. *)
1365+
else
1366+
match resolve_alias d with
1367+
| `Alpine `V3_3 -> 10001
1368+
| `Alpine `V3_4 -> 10002
1369+
| `Alpine `V3_5 -> 10003
1370+
| `Alpine `V3_6 -> 10004
1371+
| `Alpine `V3_7 -> 10005
1372+
| `Alpine `V3_8 -> 10005
1373+
| `Alpine `V3_9 -> 10005
1374+
| `Alpine `V3_10 -> 10005
1375+
| `Alpine `V3_11 -> 10005
1376+
| `Alpine `V3_12 -> 10005
1377+
| `Alpine `V3_13 -> 10005
1378+
| `Alpine `V3_14 -> 10005
1379+
| `Alpine `V3_15 -> 10005
1380+
| `Alpine `V3_16 -> 10005
1381+
| `Alpine `V3_17 -> 10005
1382+
| `Alpine `V3_18 -> 10005
1383+
| `Alpine `V3_19 -> 10005
1384+
| `Alpine `V3_20 -> 10005
1385+
| `Alpine `V3_21 -> 10005
1386+
| `Alpine `V3_22 -> 10005
1387+
| `Archlinux `Latest -> 20000
1388+
| `CentOS `V6 -> 30000
1389+
| `CentOS `V7 -> 30001
1390+
| `CentOS `V8 -> 30002
1391+
| `CentOS `V9 -> 30003
1392+
| `CentOS `V10 -> 30004
1393+
| `Cygwin `Ltsc2016 -> 40000
1394+
| `Cygwin `Ltsc2019 -> 40001
1395+
| `Cygwin `Ltsc2022 -> 40002
1396+
| `Debian `V7 -> 50000
1397+
| `Debian `V8 -> 50001
1398+
| `Debian `V9 -> 50002
1399+
| `Debian `V10 -> 50003
1400+
| `Debian `V11 -> 50004
1401+
| `Debian `V12 -> 50005
1402+
| `Debian `V13 -> 50006
1403+
| `Debian `Testing -> 50998
1404+
| `Debian `Unstable -> 50999
1405+
| `Fedora `V21 -> 60000
1406+
| `Fedora `V22 -> 60001
1407+
| `Fedora `V23 -> 60002
1408+
| `Fedora `V24 -> 60003
1409+
| `Fedora `V25 -> 60004
1410+
| `Fedora `V26 -> 60005
1411+
| `Fedora `V27 -> 60006
1412+
| `Fedora `V28 -> 60007
1413+
| `Fedora `V29 -> 60008
1414+
| `Fedora `V30 -> 60009
1415+
| `Fedora `V31 -> 60010
1416+
| `Fedora `V32 -> 60011
1417+
| `Fedora `V33 -> 60012
1418+
| `Fedora `V34 -> 60013
1419+
| `Fedora `V35 -> 60014
1420+
| `Fedora `V36 -> 60015
1421+
| `Fedora `V37 -> 60016
1422+
| `Fedora `V38 -> 60017
1423+
| `Fedora `V39 -> 60018
1424+
| `Fedora `V40 -> 60019
1425+
| `Fedora `V41 -> 60020
1426+
| `Fedora `V42 -> 60021
1427+
| `Fedora `V43 -> 60022
1428+
| `OpenSUSE `V42_1 -> 70000
1429+
| `OpenSUSE `V42_2 -> 70001
1430+
| `OpenSUSE `V42_3 -> 70002
1431+
| `OpenSUSE `V15_0 -> 70003
1432+
| `OpenSUSE `V15_1 -> 70004
1433+
| `OpenSUSE `V15_2 -> 70005
1434+
| `OpenSUSE `V15_3 -> 70006
1435+
| `OpenSUSE `V15_4 -> 70007
1436+
| `OpenSUSE `V15_5 -> 70008
1437+
| `OpenSUSE `V15_6 -> 70009
1438+
| `OpenSUSE `V16_0 -> 70010
1439+
| `OpenSUSE `Tumbleweed -> 70999
1440+
| `OracleLinux `V7 -> 80000
1441+
| `OracleLinux `V8 -> 80001
1442+
| `OracleLinux `V9 -> 80002
1443+
| `OracleLinux `V10 -> 80003
1444+
| `Ubuntu `V12_04 -> 90000
1445+
| `Ubuntu `V14_04 -> 90001
1446+
| `Ubuntu `V15_04 -> 90002
1447+
| `Ubuntu `V15_10 -> 90003
1448+
| `Ubuntu `V16_04 -> 90004
1449+
| `Ubuntu `V16_10 -> 90005
1450+
| `Ubuntu `V17_04 -> 90006
1451+
| `Ubuntu `V17_10 -> 90007
1452+
| `Ubuntu `V18_04 -> 90008
1453+
| `Ubuntu `V18_10 -> 90009
1454+
| `Ubuntu `V19_04 -> 90010
1455+
| `Ubuntu `V19_10 -> 90011
1456+
| `Ubuntu `V20_04 -> 90012
1457+
| `Ubuntu `V20_10 -> 90013
1458+
| `Ubuntu `V21_04 -> 90014
1459+
| `Ubuntu `V21_10 -> 90015
1460+
| `Ubuntu `V22_04 -> 90016
1461+
| `Ubuntu `V22_10 -> 90017
1462+
| `Ubuntu `V23_04 -> 90018
1463+
| `Ubuntu `V23_10 -> 90019
1464+
| `Ubuntu `V24_04 -> 90020
1465+
| `Ubuntu `V24_10 -> 90021
1466+
| `Ubuntu `V25_04 -> 90022
1467+
| `Ubuntu `V25_10 -> 90023
1468+
| `Windows (`Mingw, `Ltsc2019) -> 100000
1469+
| `Windows (`Msvc, `Ltsc2019) -> 100001
1470+
| `WindowsServer (`Mingw, `Ltsc2022) -> 110000
1471+
| `WindowsServer (`Mingw, `Ltsc2025) -> 110001
1472+
| `WindowsServer (`Msvc, `Ltsc2022) -> 110002
1473+
| `WindowsServer (`Msvc, `Ltsc2025) -> 110003
1474+
1475+
let compare a b = Int.compare (sort_key_of_distro a) (sort_key_of_distro b)

src-opam/distro.mli

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,8 @@ val is_same_distro : t -> t -> bool
247247

248248
val compare : t -> t -> int
249249
(** [compare a b] is a comparison function for {!t}. The ordering is only total
250-
for {!distro} values. *)
250+
for {!distro} values and is lexicographic by distribution name and sorted by
251+
version order within each distribution. *)
251252

252253
val resolve_alias : t -> distro
253254
(** [resolve_alias t] will resolve [t] into a concrete version. This removes

test-opam/dockerfile.ml

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
let test_distro_compare () =
2+
let open Dockerfile_opam in
3+
(* Check that Distro.compare thinks that Distro.distros is in sort order *)
4+
match Distro.distros with
5+
| [] -> assert false
6+
| hd::tl ->
7+
let check (errors, prev) this =
8+
let errors =
9+
if Distro.is_same_distro prev this
10+
&& Dockerfile_opam.Distro.compare this prev < 0 then
11+
let this = Distro.human_readable_string_of_distro this in
12+
let prev = Distro.human_readable_string_of_distro prev in
13+
(Printf.sprintf "%s < %s" this prev) :: errors
14+
else
15+
errors
16+
in
17+
(errors, this)
18+
in
19+
let errors, _ = List.fold_left check ([], hd) tl in
20+
let msg = String.concat "\n" errors in
21+
Alcotest.(check' bool) ~msg ~expected:true ~actual:(errors = [])
22+
23+
let () =
24+
Alcotest.(
25+
run "test"
26+
[
27+
( "dockerfile-opam",
28+
[
29+
test_case "Distro.compare" `Quick
30+
test_distro_compare;
31+
] );
32+
])

test-opam/dune

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
(test
2+
(name dockerfile)
3+
(package dockerfile-opam)
4+
(libraries alcotest dockerfile-opam))

0 commit comments

Comments
 (0)