From 3b675891d13230917ce7122a43c1f56add3e6af3 Mon Sep 17 00:00:00 2001
From: Shon Feder <shon.feder@gmail.com>
Date: Tue, 31 Dec 2024 23:46:09 -0500
Subject: [PATCH] Add linting check for upper bounds on archives

As per  https://github.com/ocaml/opam-repository/wiki/Package-Archiving:-Plan#preliminaries-for-phase-2-from-infra-team-ci
---
 opam-ci-check/lib/lint.ml       | 20 +++++++++++++--
 opam-ci-check/lib/lint_error.ml |  5 ++++
 opam-ci-check/test/lint.t       | 43 +++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/opam-ci-check/lib/lint.ml b/opam-ci-check/lib/lint.ml
index f93b2613..a962996f 100644
--- a/opam-ci-check/lib/lint.ml
+++ b/opam-ci-check/lib/lint.ml
@@ -341,8 +341,24 @@ module Checks = struct
         else None)
       other_pkgs
 
-  let check_deps_have_upper_bounds ~pkg:_ _opam =
-    []
+  let check_deps_have_upper_bounds ~pkg opam =
+    (* See https://github.com/ocaml/opam-repository/blob/master/governance/policies/archiving.md#archiving-a-package *)
+    let is_upper_bound_constraint
+      : OpamTypes.filter OpamTypes.filter_or_constraint -> bool
+      = function
+        | Constraint ((`Eq | `Leq | `Lt), _) -> true
+        | _ -> false
+    in
+    OpamFile.OPAM.depends opam
+    |> OpamFormula.fold_left (fun acc ((name, condition) : OpamTypes.name * OpamTypes.condition) ->
+        if OpamPackage.Name.to_string name = "ocaml" (* The compiler is special *)
+        || OpamFormula.exists is_upper_bound_constraint condition
+        then
+          acc
+        else
+          (pkg, MissingUpperBound (OpamPackage.Name.to_string name)) :: acc
+      )
+      []
 
   let check_x_reason_for_archival ~pkg:_ _opam =
     [] (* TODO *)
diff --git a/opam-ci-check/lib/lint_error.ml b/opam-ci-check/lib/lint_error.ml
index dbb2bacb..f947bdf8 100644
--- a/opam-ci-check/lib/lint_error.ml
+++ b/opam-ci-check/lib/lint_error.ml
@@ -36,6 +36,7 @@ type error =
   | RestrictedPrefix of string
   | PrefixConflictClassMismatch of prefix_conflict_class_mismatch
   | DefaultTagsPresent of string list
+  | MissingUpperBound of string
 
 (**/**)
 
@@ -163,3 +164,7 @@ let msg_of_error (package, (err : error)) =
         "Warning in %s: The package has not replaced the following default, \
          example tags: %s"
         pkg (String.concat ", " tags)
+  | MissingUpperBound dep_name ->
+      Printf.sprintf
+        "Error in %s: An upper bound constraint is missing on dependency '%s'"
+        pkg dep_name
diff --git a/opam-ci-check/test/lint.t b/opam-ci-check/test/lint.t
index b1bbcc65..bc4d0837 100644
--- a/opam-ci-check/test/lint.t
+++ b/opam-ci-check/test/lint.t
@@ -353,3 +353,46 @@ passes linting:
   Linting opam-repository at $TESTCASE_ROOT/. ...
   Error in a-1.0.0.1: No package source directory provided.
   [1]
+
+# Opam Archive linting tests
+
+  $ git reset -q --hard initial-state
+
+Test that we do not report an error when a wellformed package has no deps:
+
+  $ opam-ci-check lint -r . --check=archive-repo a-1.0.0.1
+  Linting opam-repository at $TESTCASE_ROOT/. ...
+  No errors
+
+Test that we report errors when a package has dependandies without an upper bound:
+
+  $ sed \
+  > -e 's/depends.*/depends: [ "foo" {with-test} "bar" {>= "0.0.1"} "baz" ]/' \
+  > packages/a-1/a-1.0.0.1/opam > opam.new
+  $ mv opam.new packages/a-1/a-1.0.0.1/opam
+  $ opam-ci-check lint -r . --check=archive-repo a-1.0.0.1
+  Linting opam-repository at $TESTCASE_ROOT/. ...
+  Error in a-1.0.0.1: An upper bound constraint is missing on dependency 'baz'
+  Error in a-1.0.0.1: An upper bound constraint is missing on dependency 'bar'
+  Error in a-1.0.0.1: An upper bound constraint is missing on dependency 'foo'
+  [1]
+
+Test that we do NOT report errors when all a packages dependandies have an upper bound:
+
+  $ sed \
+  > -e 's/depends.*/depends: ["foo" {with-test \& <= "0.1.0"} "bar" {>= "0.0.1" \& = "0.1.0"} "baz" {< "0.0.1"}]/' \
+  > packages/a-1/a-1.0.0.1/opam > opam.new
+  $ mv opam.new packages/a-1/a-1.0.0.1/opam
+  $ opam-ci-check lint -r . --check=archive-repo a-1.0.0.1
+  Linting opam-repository at $TESTCASE_ROOT/. ...
+  No errors
+
+Test that we do NOT report errors when the ocaml dependency has no upper bound:
+
+  $ sed \
+  > -e 's/depends.*/depends: ["ocaml"]/' \
+  > packages/a-1/a-1.0.0.1/opam > opam.new
+  $ mv opam.new packages/a-1/a-1.0.0.1/opam
+  $ opam-ci-check lint -r . --check=archive-repo a-1.0.0.1
+  Linting opam-repository at $TESTCASE_ROOT/. ...
+  No errors