Adjust maintainer contact lint (#393)

* Adjust maintainer contact lint Closes ocaml/infrastructure#152 by addressing the subsequenst requests for tweaked behavior. * Done use -i in sed For compat in tests between linux and BSD based.
ocurrent · Nov 27, 2024 · 29c20bb · 29c20bb
1 parent b469658
commit 29c20bb
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 17 deletions.
diff --git a/opam-ci-check/lib/lint.ml b/opam-ci-check/lib/lint.ml
@@ -265,13 +265,20 @@ module Checks = struct
         if is_build then (pkg, DuneIsBuild) :: errors else errors
     | None -> []
 
-  let check_maintainer_email ~pkg opam =
+
+  let check_maintainer_contact ~pkg opam =
+    let is_present bug_reports = bug_reports <> [] in
+    let includes_an_email maintainers =
+      List.exists
+          (fun m -> Str.string_match (Str.regexp ".*<?.*@.*>?") m 0)
+          maintainers
+    in
+    let bug_reports = OpamFile.OPAM.bug_reports opam in
     let maintainers = OpamFile.OPAM.maintainer opam in
-    List.filter_map
-      (fun m ->
-        if Str.string_match (Str.regexp ".*<?.*@.*>?") m 0 then None
-        else Some (pkg, MaintainerEmailMissing m))
-      maintainers
+    if is_present bug_reports || includes_an_email maintainers then
+      []
+    else
+      [ (pkg, MaintainerWithoutContact maintainers) ]
 
   let check_tags ~pkg opam =
     (* Check if any of the default tags are present *)
@@ -351,7 +358,7 @@ module Checks = struct
         check_checksums;
         check_package_dir ~repo_dir;
         opam_lint;
-        check_maintainer_email;
+        check_maintainer_contact;
         check_tags;
         check_no_pin_depends;
         check_no_extra_files;

diff --git a/opam-ci-check/lib/lint_error.ml b/opam-ci-check/lib/lint_error.ml
@@ -26,7 +26,7 @@ type error =
   | UnexpectedFile of string
   | ForbiddenPerm of string
   | OpamLint of (int * [ `Warning | `Error ] * string)
-  | MaintainerEmailMissing of string
+  | MaintainerWithoutContact of string list
   | FailedToDownload of string
   | NameCollision of string
   | WeakChecksum of string
@@ -149,11 +149,12 @@ let msg_of_error (package, (err : error)) =
   | OpamLint warn ->
       let warn = OpamFileTools.warns_to_string [ warn ] in
       Printf.sprintf "Error in %s: %s" pkg warn
-  | MaintainerEmailMissing maintainer ->
+  | MaintainerWithoutContact maintainer ->
       Printf.sprintf
-        "Error in %s: Maintainer email missing. Please add a maintainer email \
-         to the opam file. Maintainer: %s"
-        pkg maintainer
+        "Error in %s: There is no way to contact the maintainer(s) '%s'. A package \
+         must either specify a url for 'bug-reports' or provide an email \
+         address in the 'maintainer' field."
+        pkg (String.concat ", " maintainer)
   | ParseError ->
       Printf.sprintf "Error in %s: Failed to parse the opam file" pkg
   | DefaultTagsPresent tags ->

diff --git a/opam-ci-check/test/lint.t b/opam-ci-check/test/lint.t
@@ -60,7 +60,6 @@ Test the following:
   $ opam-ci-check lint -r . -c b.0.0.1
   Linting opam-repository at $TESTCASE_ROOT/. ...
   Error in b.0.0.1:            warning 25: Missing field 'authors'
-  Error in b.0.0.1: Maintainer email missing. Please add a maintainer email to the opam file. Maintainer: Jane
   Warning in b.0.0.1: The package has not replaced the following default, example tags: topics, project
   [1]
   $ opam-ci-check lint -r . -c b.0.0.2
@@ -187,3 +186,63 @@ Test presence of unexpected files in a-1.0.0.2 package
   Linting opam-repository at $TESTCASE_ROOT/. ...
   Error in a-1.0.0.2: Forbidden permission for file packages/a-1/a-1.0.0.2/opam. All files should have permissions 644.
   [1]
+
+# Maintainer contact lint
+
+The maintainer contact lint requires that a package EITHER provide a URL for the
+`bug-tracker` OR that at least one email is provided in the `maintainer` field.
+If neither of there requirements are met, the check should fail, otherwise it
+should  passes.
+
+Add multiple maintainers with no email and remove the bug-reports field from a
+valid package:
+
+  $ git reset -q --hard initial-state
+  $ sed \
+  > -e 's/maintainer.*/maintainer: ["Maintainer1" "Maintaner2"]/' \
+  > -e '/bug-reports.*/d' \
+  > packages/a-1/a-1.0.0.1/opam > opam.new
+  $ mv opam.new packages/a-1/a-1.0.0.1/opam
+  $ git diff packages/a-1/a-1.0.0.1/opam | grep '^[+-][^+-]'
+  -maintainer: "Maintainer <me@example.com>"
+  +maintainer: ["Maintainer1" "Maintaner2"]
+  -bug-reports: "https://github.com/ocurrent/opam-repo-ci/issues"
+
+Test that we report the expected linting error:
+
+  $ opam-ci-check lint -r . -c a-1.0.0.1
+  Linting opam-repository at $TESTCASE_ROOT/. ...
+  Error in a-1.0.0.1:            warning 36: Missing field 'bug-reports'
+  Error in a-1.0.0.1: There is no way to contact the maintainer(s) 'Maintainer1, Maintaner2'. A package must either specify a url for 'bug-reports' or provide an email address in the 'maintainer' field.
+  [1]
+
+Add one email to the maintainers, and ensure it now passes the maintainer
+contact lint:
+
+  $ sed \
+  > -e 's/"Maintaner2"/"Maintaner2 <me@example.com>"/' \
+  > packages/a-1/a-1.0.0.1/opam > opam.new
+  $ mv opam.new packages/a-1/a-1.0.0.1/opam
+  $ git diff packages/a-1/a-1.0.0.1/opam | grep '^[+-][^+-]'
+  -maintainer: "Maintainer <me@example.com>"
+  +maintainer: ["Maintainer1" "Maintaner2 <me@example.com>"]
+  -bug-reports: "https://github.com/ocurrent/opam-repo-ci/issues"
+  $ opam-ci-check lint -r . -c a-1.0.0.1
+  Linting opam-repository at $TESTCASE_ROOT/. ...
+  Error in a-1.0.0.1:            warning 36: Missing field 'bug-reports'
+  [1]
+
+Just remove the email address, leaving the bug-reports and ensure that it now
+passes linting:
+
+  $ git reset -q --hard initial-state
+  $ sed \
+  > -e 's/maintainer.*/maintainer: ["Maintainer1" "Maintaner2"]/' \
+  > packages/a-1/a-1.0.0.1/opam > opam.new
+  $ mv opam.new packages/a-1/a-1.0.0.1/opam
+  $ git diff packages/a-1/a-1.0.0.1/opam | grep '^[+-][^+-]'
+  -maintainer: "Maintainer <me@example.com>"
+  +maintainer: ["Maintainer1" "Maintaner2"]
+  $ opam-ci-check lint -r . -c a-1.0.0.1
+  Linting opam-repository at $TESTCASE_ROOT/. ...
+  No errors
diff --git a/test/lint.t b/test/lint.t
@@ -31,11 +31,7 @@ Tests the following:
   $ opam-repo-ci-local --repo="." --branch=new-branch-1 --lint-only --no-web-server
   Error "Warning in system-b.0.0.1: package name has restricted prefix 'system-'
   Error in system-b.0.0.1: package with prefix 'system-' requires conflict class 'ocaml-system'
-  Error in system-b.0.0.1: Maintainer email missing. Please add a maintainer email to the opam file. Maintainer: Maintainer
   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 b.0.0.3: Maintainer email missing. Please add a maintainer email to the opam file. Maintainer: Maintainer
-  Error in b.0.0.2: Maintainer email missing. Please add a maintainer email to the opam file. Maintainer: Maintainer
   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.1: Maintainer email missing. Please add a maintainer email to the opam file. Maintainer: Maintainer
   Error in b.0.0.1:            warning 25: Missing field 'authors'"