Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust maintainer contact lint #393

Merged
merged 2 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions opam-ci-check/lib/lint.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 *)
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 6 additions & 5 deletions opam-ci-check/lib/lint_error.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ->
Expand Down
61 changes: 60 additions & 1 deletion opam-ci-check/test/lint.t
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 <[email protected]>"
+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 <[email protected]>"/' \
> 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 <[email protected]>"
+maintainer: ["Maintainer1" "Maintaner2 <[email protected]>"]
-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 <[email protected]>"
+maintainer: ["Maintainer1" "Maintaner2"]
$ opam-ci-check lint -r . -c a-1.0.0.1
Linting opam-repository at $TESTCASE_ROOT/. ...
No errors
4 changes: 0 additions & 4 deletions test/lint.t
Original file line number Diff line number Diff line change
Expand Up @@ -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'"