Test for restricted prefixes in package names

Co-authored-by: Shon Feder <[email protected]>

lib/lint.ml

@@ -12,6 +12,18 @@ let ( // ) = Filename.concat
 let ( >>/= ) x f = x >>= fun x -> f (Result.get_ok x)
 let exec ~cwd ~job cmd = Current.Process.exec ~cwd ~cancellable:true ~job ("", cmd)
 
+(** If either a restricted prefix or conflict class exists,
+    then the corresponding other must also exist. *)
+type prefix_conflict_class_mismatch =
+  | WrongPrefix of {
+      conflict_class: string;
+      required_prefix: string;
+    }
+  | WrongConflictClass of {
+      prefix: string;
+      required_conflict_class: string;
+    }
+
 type error =
   | UnnecessaryField of string
   | UnmatchedName of OpamPackage.Name.t
@@ -29,6 +41,8 @@ type error =
   | WeakChecksum of string
   | PinDepends
   | ExtraFiles
+  | RestrictedPrefix of string
+  | PrefixConflictClassMismatch of prefix_conflict_class_mismatch
 
 type host_os = Macos | Other [@@deriving to_yojson]
 
@@ -370,6 +384,75 @@ module Check = struct
     | None | Some [] -> errors
     | Some _ -> (pkg, ExtraFiles) :: errors
 
+  module Prefix = struct
+    (* For context, see https://github.com/ocurrent/opam-repo-ci/pull/316#issuecomment-2160069803 *)
+    let prefix_conflict_class_map = [
+      ("mysys2-", "msys2-env");
+      ("arch-", "ocaml-arch");
+      ("ocaml-env-mingw", "ocaml-env-mingw");
+      ("ocaml-env-msvc", "ocaml-env-msvc");
+      ("host-arch-", "ocaml-host-arch");
+      ("host-system-", "ocaml-host-system");
+      ("system-", "ocaml-system");
+    ]
+
+    let conflict_class_prefix_map = List.map (fun (a, b) -> (b, a)) prefix_conflict_class_map
+
+    let prefixes = List.map fst prefix_conflict_class_map
+
+    let check_name_restricted_prefix ~errors ~pkg =
+      let name = OpamPackage.name_to_string pkg in
+      List.fold_left
+        (fun errors prefix ->
+          if String.starts_with ~prefix name then
+            (pkg, RestrictedPrefix prefix) :: errors
+          else
+            errors)
+        errors
+        prefixes
+
+    let check_prefix_without_conflict_class ~errors ~pkg name conflict_classes =
+      let prefix = List.find_opt
+        (fun prefix -> String.starts_with ~prefix name)
+        prefixes
+      in
+      match prefix with
+      | None -> errors
+      | Some prefix ->
+        match List.assoc_opt prefix prefix_conflict_class_map with
+        | None ->
+          Logs.err
+            (fun m -> m "BUG: prefix '%s' not found in conflict class map" prefix);
+          errors
+        | Some required_conflict_class ->
+          if List.mem required_conflict_class conflict_classes then
+            errors
+          else
+            (pkg, PrefixConflictClassMismatch
+              (WrongConflictClass { prefix; required_conflict_class })) :: errors
+
+    let check_conflict_class_without_prefix ~errors ~pkg name conflict_classes =
+      List.fold_left
+        (fun errors conflict_class ->
+          match List.assoc_opt conflict_class conflict_class_prefix_map with
+          | None -> errors
+          | Some prefix ->
+            if String.starts_with ~prefix name then
+              errors
+            else
+              (pkg, PrefixConflictClassMismatch
+                (WrongPrefix { conflict_class; required_prefix=prefix })) :: errors
+        )
+        errors
+        conflict_classes
+
+    let check_prefix_conflict_class_mismatch ~errors ~pkg opam =
+      let conflict_classes = List.map OpamPackage.Name.to_string @@ OpamFile.OPAM.conflict_class opam in
+      let name = OpamPackage.name_to_string pkg in
+      let errors = check_prefix_without_conflict_class ~errors ~pkg name conflict_classes in
+      check_conflict_class_without_prefix ~errors ~pkg name conflict_classes
+  end
+
   let opam_lint ~check_extra_files ~errors ~pkg opam =
     OpamFileTools.lint ~check_extra_files ~check_upstream:true opam |>
     List.fold_left (fun errors x -> (pkg, OpamLint x) :: errors) errors
@@ -390,6 +473,7 @@ module Check = struct
           let errors = check_checksums ~errors ~pkg opam in
           let errors = check_no_pin_depends ~errors ~pkg opam in
           let errors = check_no_extra_files ~errors ~pkg opam in
+          let errors = Prefix.check_prefix_conflict_class_mismatch ~errors ~pkg opam in
           check_dune_constraints ~host_os ~errors ~pkg opam >>= fun errors ->
           (* Check directory structure correctness *)
           scan_dir ~cwd errors pkg >>= fun (errors, check_extra_files) ->
@@ -398,7 +482,8 @@ module Check = struct
           function
           | false -> errors
           | true ->
-              check_name_collisions ~errors ~pkg existing_packages
+            let errors = check_name_collisions ~errors ~pkg existing_packages in
+            Prefix.check_name_restricted_prefix ~errors ~pkg
     ) [] packages
 end
 
@@ -436,6 +521,16 @@ module Lint = struct
 
   let id = "opam-ci-lint"
 
+  let msg_of_prefix_conflict_class_mismatch ~pkg = function
+    | WrongPrefix { conflict_class; required_prefix } ->
+      Fmt.str
+        "Error in %s: package with conflict class '%s' requires name prefix '%s'"
+        pkg conflict_class required_prefix
+    | WrongConflictClass { prefix; required_conflict_class} ->
+      Fmt.str
+        "Error in %s: package with prefix '%s' requires conflict class '%s'"
+        pkg prefix required_conflict_class
+
   let msg_of_error (package, err) =
     let pkg = OpamPackage.to_string package in
     match err with
@@ -491,6 +586,10 @@ module Lint = struct
       Fmt.str "Error in %s: pin-depends present. This is not allowed in the opam-repository." pkg
     | ExtraFiles ->
       Fmt.str "Error in %s: extra-files present. This is not allowed in the opam-repository. Please use extra-source instead." pkg
+    | RestrictedPrefix prefix ->
+      Fmt.str "Warning in %s: package name has restricted prefix '%s'" pkg prefix
+    | PrefixConflictClassMismatch mismatch ->
+      msg_of_prefix_conflict_class_mismatch ~pkg mismatch
 
   let run { master } job { Key.src; packages } { Value.host_os } =
     Current.Job.start job ~pool ~level:Current.Level.Harmless >>= fun () ->

test/lint.t

@@ -19,7 +19,8 @@ Reset commit and clear build cache
 Tests the following:
 - [b.0.0.1] is missing the [author] field
 - [b.0.0.2] has an extra unknown field
-- [b.0.0.3] is correct
+- [b.0.0.3] has a pin-depends present, and a conflict class without the required prefix
+- [system-b.0.0.1] is using a restricted prefix in its name
 
   $ git apply "patches/b-incorrect-opam.patch"
   $ git add .
@@ -28,10 +29,13 @@ Tests the following:
   * b-incorrect-opam (HEAD -> new-branch-1)
   * a-1 (master)
   $ opam-repo-ci-local --repo="." --branch=new-branch-1 --lint-only --no-web-server
-  Error "3 errors:
+  Error "6 errors:
   Error in b.0.0.1:            warning 25: Missing field 'authors'
   Error in b.0.0.2:              error  3: File format error in 'unknown-field' at line 11, column 0: Invalid field unknown-field
-  Error in b.0.0.3: pin-depends present. This is not allowed in the opam-repository."
+  Error in b.0.0.3: package with conflict class 'ocaml-host-arch' requires name prefix 'host-arch-'
+  Error in b.0.0.3: pin-depends present. This is not allowed in the opam-repository.
+  Error in system-b.0.0.1: package with prefix 'system-' requires conflict class 'ocaml-system'
+  Warning in system-b.0.0.1: package name has restricted prefix 'system-'"
 
 Reset commit and clear build cache
 

test/patches/b-incorrect-opam.patch

@@ -1,20 +1,6 @@
-From 00dbea20350d84b089ff1c1014252fa31babcb2a Mon Sep 17 00:00:00 2001
-From: benmandrew <[email protected]>
-Date: Mon, 19 Feb 2024 14:25:05 +0100
-Subject: [PATCH] b
-
----
- packages/b/b.0.0.1/opam | 13 +++++++++++++
- packages/b/b.0.0.2/opam | 13 +++++++++++++
- packages/b/b.0.0.3/opam | 13 +++++++++++++
- 3 files changed, 39 insertions(+)
- create mode 100644 packages/b/b.0.0.1/opam
- create mode 100644 packages/b/b.0.0.2/opam
- create mode 100644 packages/b/b.0.0.3/opam
-
 diff --git a/packages/b/b.0.0.1/opam b/packages/b/b.0.0.1/opam
 new file mode 100644
-index 0000000..69040f4
+index 0000000..3697498
 --- /dev/null
 +++ b/packages/b/b.0.0.1/opam
 @@ -0,0 +1,13 @@
@@ -33,7 +19,7 @@ index 0000000..69040f4
 +]
 diff --git a/packages/b/b.0.0.2/opam b/packages/b/b.0.0.2/opam
 new file mode 100644
-index 0000000..1581508
+index 0000000..a47aabf
 --- /dev/null
 +++ b/packages/b/b.0.0.2/opam
 @@ -0,0 +1,15 @@
@@ -54,10 +40,10 @@ index 0000000..1581508
 +]
 diff --git a/packages/b/b.0.0.3/opam b/packages/b/b.0.0.3/opam
 new file mode 100644
-index 0000000..5ec1dc4
+index 0000000..5bab63e
 --- /dev/null
 +++ b/packages/b/b.0.0.3/opam
-@@ -0,0 +1,15 @@
+@@ -0,0 +1,18 @@
 +opam-version: "2.0"
 +synopsis: "Synopsis"
 +description: "Description"
@@ -72,6 +58,27 @@ index 0000000..5ec1dc4
 +depends: [
 +  "a-1" {< "0.0.2"}
 +]
++conflict-class: [
++  "ocaml-host-arch"
++]
 +pin-depends: [ "foo.~dev" "git+https://github.com/bar/foo" ]
--- 
-2.43.0
+diff --git a/packages/system-b/system-b.0.0.1/opam b/packages/system-b/system-b.0.0.1/opam
+new file mode 100644
+index 0000000..0808e56
+--- /dev/null
++++ b/packages/system-b/system-b.0.0.1/opam
+@@ -0,0 +1,14 @@
++opam-version: "2.0"
++synopsis: "Synopsis"
++description: "Description"
++maintainer: "Maintainer"
++author: "Author"
++license: "Apache-2.0"
++homepage: "https://github.com/ocurrent/opam-repo-ci"
++bug-reports: "https://github.com/ocurrent/opam-repo-ci/issues"
++dev-repo: "git+https://github.com/ocurrent/opam-repo-ci.git"
++doc: "https://ocurrent.github.io/ocurrent/"
++build: []
++depends: [
++  "a-1" {>= "0.0.2"}
++]