forked from
anil.recoil.org/monopam
Monorepo management for opam overlays
monopam: lint Missing_test is subtree-wide, not per-package
A multi-package subtree like ocaml-cfdp ([cfdp] + [cfdp-eio]) or
ocaml-atp (a dozen lexicon and CLI packages) has shared [test/]
trees whose stanzas don't carry a [(package ...)] annotation, so
there's no way to attribute a given [(test (libraries ...))] to a
specific opam package. Reporting the same missing-test against
every package in the subtree was noisy false-positive city -- e.g.
[cfdp-eio] got blamed for not declaring [alcotest] when [cfdp]'s
test/dune was the actual user.
Move the [Missing_test] check out of [check_package] (which is
called per-opam-package) into the per-subtree loop in [run]. The
new check accepts any package that appears in the union of all
opam packages' declared deps within the subtree, treating the
subtree as a whole.
[Unused_test] stays per-package -- a [:with-test] dep that no
stanza references really is dead weight in that specific opam
file.
Cleared ~60 false-positive missing-test reports; the remaining
~30 are real misses to fix package by package.
+40
-23
+40
-23
lib/lint.ml
+40
-23
lib/lint.ml
···
853
853
| exception _ -> false)
854
854
|> List.sort String.compare
855
855
856
-
let check_package ~index ~build_lib ~dune_pkgs ~dune_test_pkgs ~dune_owner_pkgs
857
-
~own_set ~all_deps ~subtree (pkg : opam_pkg) ~fs =
856
+
let check_package ~index ~build_lib ~dune_pkgs ~dune_owner_pkgs ~own_set
857
+
~all_deps ~subtree (pkg : opam_pkg) ~fs =
858
858
let pkg_name = pkg.name in
859
859
let runtime_deps = pkg.runtime in
860
860
let test_deps = pkg.test in
···
900
900
else acc)
901
901
required_pkgs []
902
902
in
903
-
(* Test-scope checks are independent of META availability: tests are
904
-
compiled from this subtree's dune files, not from installed METAs. *)
905
-
let test_missing =
906
-
String_set.fold
907
-
(fun pkg acc ->
908
-
if String_set.mem pkg own_set then acc
909
-
else if String_set.mem pkg all_deps then acc
910
-
else { subtree; kind = Missing_test; package = pkg } :: acc)
911
-
dune_test_pkgs []
912
-
in
913
-
(* A [:with-test] dep is "used" if any stanza references it -- runtime,
914
-
test, or executable. Some packages put fuzz drivers in [(executable ...)]
915
-
stanzas that only run via [(rule (alias runtest))], so they're
916
-
legitimately scoped [:with-test] in opam even though the stanza kind is
917
-
[executable]. Trust the package's scope choice; only flag [:with-test]
918
-
deps that nothing references at all. *)
903
+
(* Test-scope unused: a [:with-test] dep is "used" if any stanza references
904
+
it -- runtime, test, or executable. Some packages put fuzz drivers in
905
+
[(executable ...)] stanzas that only run via [(rule (alias runtest))],
906
+
so they're legitimately scoped [:with-test] even though the stanza
907
+
kind is [executable]. Trust the package's scope choice; only flag
908
+
[:with-test] deps that nothing references at all.
909
+
910
+
The missing-test counterpart is computed once per subtree by the caller
911
+
in [run] -- not here -- because a multi-package subtree (e.g.
912
+
ocaml-cfdp = cfdp + cfdp-eio) can't tell which test stanza belongs to
913
+
which opam package without ownership annotations. *)
919
914
let test_unused =
920
915
String_set.diff test_deps
921
916
(String_set.union dune_pkgs
···
928
923
(fun pkg acc -> { subtree; kind = Unused_test; package = pkg } :: acc)
929
924
test_unused []
930
925
in
931
-
let test_issues = test_missing @ test_unused_issues in
932
926
if meta_available then
933
927
let needed =
934
928
String_set.union meta_pkgs
···
943
937
(fun pkg acc -> { subtree; kind = Unused; package = pkg } :: acc)
944
938
unused []
945
939
in
946
-
missing @ unused_issues @ test_issues
947
-
else missing @ test_issues
940
+
missing @ unused_issues @ test_unused_issues
941
+
else missing @ test_unused_issues
948
942
949
943
let sort_issues (issues : issue list) =
950
944
List.sort
···
1073
1067
let dune_owner_pkgs =
1074
1068
dune_packages_by_owner ~fs ~index ~own_set subtree_path
1075
1069
in
1070
+
(* Subtree-wide [Missing_test] check: a test stanza in a multi-
1071
+
package subtree (cfdp + cfdp-eio, atp + bsky + ...) can't be
1072
+
attributed to one specific opam package without ownership
1073
+
annotations. So accept any package in the union of all opam
1074
+
packages' declared deps as "declared somewhere in this
1075
+
subtree". Reports issues that nothing in the subtree
1076
+
declares. *)
1077
+
let subtree_all_deps =
1078
+
List.fold_left
1079
+
(fun acc (p : opam_pkg) -> String_set.union acc p.all)
1080
+
String_set.empty pkgs
1081
+
in
1082
+
let test_missing =
1083
+
String_set.fold
1084
+
(fun pkg acc ->
1085
+
if String_set.mem pkg own_set then acc
1086
+
else if String_set.mem pkg subtree_all_deps then acc
1087
+
else if String_set.mem pkg implicit_deps then acc
1088
+
else if is_conf_pkg pkg then acc
1089
+
else { subtree; kind = Missing_test; package = pkg } :: acc)
1090
+
dune_test_pkgs []
1091
+
in
1092
+
issues := test_missing @ !issues;
1076
1093
List.iter
1077
1094
(fun (pkg : opam_pkg) ->
1078
1095
let direct_deps = pkg.all in
···
1094
1111
~subtree pkg
1095
1112
@ !issues;
1096
1113
let new_issues =
1097
-
check_package ~index ~build_lib ~dune_pkgs ~dune_test_pkgs
1098
-
~dune_owner_pkgs ~own_set ~all_deps ~subtree pkg ~fs
1114
+
check_package ~index ~build_lib ~dune_pkgs ~dune_owner_pkgs
1115
+
~own_set ~all_deps ~subtree pkg ~fs
1099
1116
in
1100
1117
issues := new_issues @ !issues)
1101
1118
pkgs;