Monorepo management for opam overlays
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

monopam: lint each opam file's depends independently

The previous algorithm pooled every opam file in a subtree into one
all_deps set, so a dep declared in any sibling package satisfied the
check for all of them. That hid real gaps: dune build -p <pkg> only
sees the depends of <pkg>'s own opam file, so a package can build
under workspace mode while -p mode fails with "Library X not found".

Compute the re_export-expanded all_deps from each package's own
direct deps. Sibling packages in the same subtree are still treated
as present via own_set, which is correct because dune always sees
them together.

+159 -60
+159 -60
lib/lint.ml
··· 299 299 if is_builtin lib then None else Some (lib_to_package index lib)) 300 300 |> String_set.of_list 301 301 302 + (** Owning opam package for a single dune stanza. Prefers an explicit 303 + [(package foo)] field, otherwise infers it from [(public_name n)] — using 304 + the part before the first dot when the name is dotted ([foo.bar] -> [foo]). 305 + Returns [None] for private stanzas (no [public_name], no [package]). *) 306 + let stanza_owner fields = 307 + match field "package" fields with 308 + | Some (Sexp.Atom p :: _) -> Some p 309 + | _ -> ( 310 + match field "public_name" fields with 311 + | Some (Sexp.Atom pn :: _) -> ( 312 + match String.index_opt pn '.' with 313 + | Some i -> Some (String.sub pn 0 i) 314 + | None -> Some pn) 315 + | _ -> None) 316 + 317 + let libs_of_fields fields = 318 + match field "libraries" fields with 319 + | None -> [] 320 + | Some libs -> 321 + List.filter_map 322 + (function 323 + | Sexp.Atom s when not (String.starts_with ~prefix:"%" s) -> Some s 324 + | Sexp.List [ Sexp.Atom "re_export"; Sexp.Atom s ] -> Some s 325 + | _ -> None) 326 + libs 327 + 328 + (** Per-package map: for each owning opam package, the set of external packages 329 + referenced in its [(library)]/[(executable[s])] dune stanzas. Private 330 + stanzas (no [public_name]/no [package]) are skipped — they belong to no opam 331 + package and are covered by the subtree-wide unused check. 332 + 333 + A first pass collects the [(name X) (public_name Y)] mapping so a sibling 334 + reference written as the internal OCaml name (e.g. [atproto_handle] for 335 + public [atproto-handle]) resolves to the public package name. *) 336 + let dune_packages_by_owner ~fs ~index subtree_path = 337 + let dune_files = dune_files_in ~fs subtree_path in 338 + let stanzas = 339 + List.concat_map 340 + (fun df -> 341 + match load_file fs df with 342 + | None -> [] 343 + | Some content -> parse_sexps content) 344 + dune_files 345 + in 346 + let internal_to_public = Hashtbl.create 16 in 347 + List.iter 348 + (function 349 + | Sexp.List (Sexp.Atom kind :: fields) 350 + when List.mem kind [ "library"; "executable"; "executables" ] -> ( 351 + match (field "name" fields, field "public_name" fields) with 352 + | Some (Sexp.Atom n :: _), Some (Sexp.Atom pn :: _) -> 353 + let pub = 354 + match String.index_opt pn '.' with 355 + | Some i -> String.sub pn 0 i 356 + | None -> pn 357 + in 358 + Hashtbl.replace internal_to_public n pub 359 + | _ -> ()) 360 + | _ -> ()) 361 + stanzas; 362 + let resolve lib = 363 + match Hashtbl.find_opt internal_to_public lib with 364 + | Some pub -> pub 365 + | None -> lib_to_package index lib 366 + in 367 + let tbl = Hashtbl.create 16 in 368 + let add owner pkg = 369 + let cur = try Hashtbl.find tbl owner with Not_found -> String_set.empty in 370 + Hashtbl.replace tbl owner (String_set.add pkg cur) 371 + in 372 + List.iter 373 + (function 374 + | Sexp.List (Sexp.Atom kind :: fields) 375 + when List.mem kind [ "library"; "executable"; "executables" ] -> ( 376 + match stanza_owner fields with 377 + | None -> () 378 + | Some owner -> 379 + List.iter 380 + (fun lib -> 381 + if not (is_builtin lib) then add owner (resolve lib)) 382 + (libs_of_fields fields)) 383 + | _ -> ()) 384 + stanzas; 385 + fun pkg -> try Hashtbl.find tbl pkg with Not_found -> String_set.empty 386 + 302 387 (* ---- Types ---- *) 303 388 304 389 type kind = Missing | Unused ··· 396 481 | exception _ -> false) 397 482 |> List.sort String.compare 398 483 399 - let check_package ~index ~build_lib ~dune_pkgs ~own_set ~all_deps ~subtree 400 - (pkg_name, runtime_deps, _all_deps) ~fs = 484 + let check_package ~index ~build_lib ~dune_pkgs ~dune_owner_pkgs ~own_set 485 + ~all_deps ~subtree (pkg_name, runtime_deps, _all_deps) ~fs = 401 486 let meta_path = Fpath.(build_lib / pkg_name / "META") in 402 - match load_meta ~fs meta_path with 403 - | Error msg when msg = "I/O error" -> 404 - Log.debug (fun m -> m "%s/%s: no META file (not built?)" subtree pkg_name); 405 - [] 406 - | Error msg -> 407 - Log.warn (fun m -> m "%s/%s: META parse error: %s" subtree pkg_name msg); 408 - [] 409 - | Ok expr -> 410 - let required_libs = 411 - collect_requires expr |> List.sort_uniq String.compare 412 - in 413 - let meta_pkgs = 414 - List.fold_left 415 - (fun acc lib -> 416 - if is_builtin lib then acc 417 - else 418 - let pkg = lib_to_package index lib in 419 - if String_set.mem pkg own_set then acc else String_set.add pkg acc) 420 - String_set.empty required_libs 421 - in 422 - let missing = 423 - String_set.fold 424 - (fun pkg acc -> 425 - if not (String_set.mem pkg all_deps) then 426 - { subtree; kind = Missing; package = pkg } :: acc 427 - else acc) 428 - meta_pkgs [] 429 - in 430 - let needed = 431 - String_set.union meta_pkgs 432 - (String_set.union dune_pkgs (String_set.union own_set implicit_deps)) 433 - in 434 - let unused = 435 - String_set.diff runtime_deps needed 436 - |> String_set.filter (fun p -> not (is_conf_pkg p)) 437 - in 438 - let unused_issues = 439 - String_set.fold 440 - (fun pkg acc -> { subtree; kind = Unused; package = pkg } :: acc) 441 - unused [] 442 - in 443 - missing @ unused_issues 487 + let meta_pkgs, meta_available = 488 + match load_meta ~fs meta_path with 489 + | Error msg when msg = "I/O error" -> 490 + Log.debug (fun m -> 491 + m "%s/%s: no META file (not built?)" subtree pkg_name); 492 + (String_set.empty, false) 493 + | Error msg -> 494 + Log.warn (fun m -> m "%s/%s: META parse error: %s" subtree pkg_name msg); 495 + (String_set.empty, false) 496 + | Ok expr -> 497 + let required_libs = 498 + collect_requires expr |> List.sort_uniq String.compare 499 + in 500 + let pkgs = 501 + List.fold_left 502 + (fun acc lib -> 503 + if is_builtin lib then acc 504 + else 505 + let pkg = lib_to_package index lib in 506 + if String_set.mem pkg own_set then acc 507 + else String_set.add pkg acc) 508 + String_set.empty required_libs 509 + in 510 + (pkgs, true) 511 + in 512 + (* Fallback source when META is absent (build failed for this -p build): 513 + libraries referenced from this opam's own (library)/(executable) stanzas, 514 + minus sibling packages. Private stanzas (tests, anonymous libs) carry no 515 + package ownership and are excluded. *) 516 + let dune_self = String_set.diff (dune_owner_pkgs pkg_name) own_set in 517 + let required_pkgs = String_set.union meta_pkgs dune_self in 518 + let missing = 519 + String_set.fold 520 + (fun pkg acc -> 521 + if not (String_set.mem pkg all_deps) then 522 + { subtree; kind = Missing; package = pkg } :: acc 523 + else acc) 524 + required_pkgs [] 525 + in 526 + if meta_available then 527 + let needed = 528 + String_set.union meta_pkgs 529 + (String_set.union dune_pkgs (String_set.union own_set implicit_deps)) 530 + in 531 + let unused = 532 + String_set.diff runtime_deps needed 533 + |> String_set.filter (fun p -> not (is_conf_pkg p)) 534 + in 535 + let unused_issues = 536 + String_set.fold 537 + (fun pkg acc -> { subtree; kind = Unused; package = pkg } :: acc) 538 + unused [] 539 + in 540 + missing @ unused_issues 541 + else missing 444 542 445 543 let sort_issues (issues : issue list) = 446 544 List.sort ··· 499 597 else begin 500 598 incr scanned; 501 599 let own_set = String_set.of_list (List.map (fun (n, _, _) -> n) pkgs) in 502 - let direct_deps = 503 - List.fold_left 504 - (fun acc (_, _, all) -> String_set.union acc all) 505 - String_set.empty pkgs 506 - in 507 - (* Expand declared deps with whatever they re-export, so consumers 508 - don't need to list transitive re-exports explicitly. *) 509 - let all_deps = 510 - String_set.fold 511 - (fun pkg acc -> 512 - String_set.union acc (reexports_of_pkg ~fs ~build_lib ~index pkg)) 513 - direct_deps direct_deps 514 - in 515 600 let dune_pkgs = dune_needed_packages ~fs ~index subtree_path in 601 + let dune_owner_pkgs = dune_packages_by_owner ~fs ~index subtree_path in 516 602 List.iter 517 - (fun pkg -> 603 + (fun ((_, _, direct_deps) as pkg) -> 604 + (* Each opam file declares its own depends; `dune build -p <pkg>` 605 + only sees that one file. Expand this package's declared deps 606 + with whatever they re-export, so consumers don't need to list 607 + transitive re-exports explicitly. Sibling packages in the same 608 + subtree are treated as present via `own_set`, not by pooling 609 + their depends into this check. *) 610 + let all_deps = 611 + String_set.fold 612 + (fun p acc -> 613 + String_set.union acc 614 + (reexports_of_pkg ~fs ~build_lib ~index p)) 615 + direct_deps direct_deps 616 + in 518 617 let new_issues = 519 - check_package ~index ~build_lib ~dune_pkgs ~own_set ~all_deps 520 - ~subtree pkg ~fs 618 + check_package ~index ~build_lib ~dune_pkgs ~dune_owner_pkgs 619 + ~own_set ~all_deps ~subtree pkg ~fs 521 620 in 522 621 issues := new_issues @ !issues) 523 622 pkgs