-
Notifications
You must be signed in to change notification settings - Fork 33
Add counts of deps and rev-deps in the list of packages #259
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
base: master
Are you sure you want to change the base?
Changes from all commits
3b8e6f1
3ec980a
c511d73
bde1fb7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -113,6 +113,75 @@ let rev_depends deps = | |||||||||||||||||
| (OpamPackage.Set.add pkg) OpamPackage.Set.empty)) | ||||||||||||||||||
| deps OpamPackage.Map.empty | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| (* Dependencies used for the dependency cone. They differ from [depends] above. | ||||||||||||||||||
| They are an approximation of what [opam list --required-by] would show because we cannot | ||||||||||||||||||
| resolve variables that need a switch (os, arch, ...). To avoid including dependencies of multiple platforms | ||||||||||||||||||
| (say both linux and macos), we remove disjunctions (by picking a single branch). | ||||||||||||||||||
|
|
||||||||||||||||||
| [filter_default] is true to match OpamSwitchState.dependencies, which keeps a dependency whose | ||||||||||||||||||
| filter it cannot decide. | ||||||||||||||||||
| *) | ||||||||||||||||||
| let rec remove_disjunction = function | ||||||||||||||||||
| | (OpamFormula.Empty as x) | (Atom _ as x) -> x | ||||||||||||||||||
| | Block x -> remove_disjunction x | ||||||||||||||||||
| | And (a, b) -> And (remove_disjunction a, remove_disjunction b) | ||||||||||||||||||
| | Or (a, _) -> remove_disjunction a | ||||||||||||||||||
|
|
||||||||||||||||||
| let cone_depends st = | ||||||||||||||||||
| OpamPackage.Map.fold (fun pkg opam -> | ||||||||||||||||||
| let deps = | ||||||||||||||||||
| OpamFormula.packages st.OpamStateTypes.packages @@ | ||||||||||||||||||
| remove_disjunction @@ | ||||||||||||||||||
| OpamPackageVar.all_depends ~build:true ~post:false ~test:false | ||||||||||||||||||
| ~doc:false ~dev_setup:false ~depopts:false ~filter_default:true | ||||||||||||||||||
| st opam | ||||||||||||||||||
| in | ||||||||||||||||||
| OpamPackage.Map.add pkg deps) | ||||||||||||||||||
| st.opams OpamPackage.Map.empty | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| (* A graph where nodes are packages and there is an edge a->b if package a depends on package b. *) | ||||||||||||||||||
| module PkgGraph = struct | ||||||||||||||||||
| type t = package_set package_map | ||||||||||||||||||
| module V = OpamPackage | ||||||||||||||||||
| let iter_vertex f deps = OpamPackage.Map.iter (fun pkg _ -> f pkg) deps | ||||||||||||||||||
| let iter_succ f deps pkg = | ||||||||||||||||||
| match OpamPackage.Map.find_opt pkg deps with | ||||||||||||||||||
| | None -> () | ||||||||||||||||||
| | Some pkgs -> OpamPackage.Set.iter f pkgs | ||||||||||||||||||
| end | ||||||||||||||||||
|
|
||||||||||||||||||
| module PkgTopo = Graph.Topological.Make (PkgGraph) | ||||||||||||||||||
|
|
||||||||||||||||||
| let dependency_cone_sizes depends = | ||||||||||||||||||
| (* packages in reverse topological order *) | ||||||||||||||||||
| let pkgs = PkgTopo.fold (fun pkg acc -> pkg :: acc) depends [] in | ||||||||||||||||||
| let cones = | ||||||||||||||||||
| List.fold_left (fun acc pkg -> | ||||||||||||||||||
| let deps = | ||||||||||||||||||
| OpamStd.Option.default OpamPackage.Set.empty | ||||||||||||||||||
| (OpamPackage.Map.find_opt pkg depends) | ||||||||||||||||||
| in | ||||||||||||||||||
| (* Some packages include [ocaml] as a dependency. We remove [ocaml] from their cone to avoid including | ||||||||||||||||||
| also ocaml-base-compiler, ocaml-option-bytecode-only, ... | ||||||||||||||||||
| Except for [ocaml] itself, for which we want to see the full cone. *) | ||||||||||||||||||
| let deps = match OpamPackage.Name.to_string (OpamPackage.name pkg) with | ||||||||||||||||||
| | "ocaml" -> deps | ||||||||||||||||||
| | _ -> OpamPackage.Set.filter (fun p -> OpamPackage.Name.to_string p.name <> "ocaml") deps | ||||||||||||||||||
| in | ||||||||||||||||||
| let cone = | ||||||||||||||||||
| OpamPackage.Set.fold (fun dep cone -> | ||||||||||||||||||
| match OpamPackage.Map.find_opt dep acc with | ||||||||||||||||||
| | Some dep_cone -> OpamPackage.Name.Set.union dep_cone cone | ||||||||||||||||||
| | None -> cone) | ||||||||||||||||||
| deps (OpamPackage.Name.Set.singleton pkg.name) | ||||||||||||||||||
| in | ||||||||||||||||||
| OpamPackage.Map.add pkg cone acc) | ||||||||||||||||||
| OpamPackage.Map.empty pkgs | ||||||||||||||||||
| in | ||||||||||||||||||
| OpamPackage.Map.map OpamPackage.Name.Set.cardinal cones | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To avoid having every almost package start at 10 dependencies, we could cheat a little and only show the number of dependencies starting from the
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good suggestion. But we should not substract An interesting example. Inspecting the dependency cone of
It further suggests:
Not planning to do anything about it in this PR, since |
||||||||||||||||||
|
|
||||||||||||||||||
| let to_page ~prefix universe pkg acc = | ||||||||||||||||||
| try | ||||||||||||||||||
| if Unix.isatty Unix.stdout then | ||||||||||||||||||
|
|
@@ -241,6 +310,8 @@ let load statistics repo_roots = | |||||||||||||||||
| let rdeps = rev_depends deps in | ||||||||||||||||||
| let depopts = depends st OpamFile.OPAM.depopts in | ||||||||||||||||||
| let rev_depopts = rev_depends depopts in | ||||||||||||||||||
| Printf.printf "++ Computing dependency cones.\n%!"; | ||||||||||||||||||
| let dependency_cone_sizes = dependency_cone_sizes (cone_depends st) in | ||||||||||||||||||
| Printf.printf "++ Getting package modification dates from git.\n%!"; | ||||||||||||||||||
| let dates = dates st in | ||||||||||||||||||
| let version_downloads, name_popularity = | ||||||||||||||||||
|
|
@@ -262,6 +333,7 @@ let load statistics repo_roots = | |||||||||||||||||
| name_popularity; | ||||||||||||||||||
| depends = deps; | ||||||||||||||||||
| rev_depends = rdeps; | ||||||||||||||||||
| dependency_cone_sizes; | ||||||||||||||||||
| depopts; | ||||||||||||||||||
| rev_depopts; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
@@ -314,6 +386,19 @@ let to_html ~content_dir ~sortby_links ~active ~compare_pkg univ = | |||||||||||||||||
| ] | ||||||||||||||||||
| | None -> [] | ||||||||||||||||||
| in | ||||||||||||||||||
| (* Number of reverse dependencies *) | ||||||||||||||||||
| let pkg_nb_rev_depends = | ||||||||||||||||||
| match OpamPackage.Map.find_opt pkg univ.rev_depends with | ||||||||||||||||||
| | None -> 0 | ||||||||||||||||||
| | Some rdeps -> | ||||||||||||||||||
| OpamPackage.Name.Set.cardinal (OpamPackage.names_of_packages rdeps) | ||||||||||||||||||
| in | ||||||||||||||||||
| (* Size of the dependency cone: everything an installation of this | ||||||||||||||||||
| package pulls in, itself included. *) | ||||||||||||||||||
| let pkg_dependency_cone_size = | ||||||||||||||||||
| OpamStd.Option.default 0 | ||||||||||||||||||
| (OpamPackage.Map.find_opt pkg univ.dependency_cone_sizes) | ||||||||||||||||||
| in | ||||||||||||||||||
| let tags = String.concat " " (OpamFile.OPAM.tags pkg_info) in | ||||||||||||||||||
| let pkg_tags = if tags = "" then [] else ["Tags: "^tags] in | ||||||||||||||||||
| let pkg_tooltip = String.concat " | " (pkg_download @ pkg_published @ pkg_tags) in | ||||||||||||||||||
|
|
@@ -329,7 +414,9 @@ let to_html ~content_dir ~sortby_links ~active ~compare_pkg univ = | |||||||||||||||||
| (Html.a ~href:pkg_href | ||||||||||||||||||
| (Html.string (OpamPackage.name_to_string pkg))) | ||||||||||||||||||
| @ Html.tag "td" (Html.string (OpamPackage.version_to_string pkg)) | ||||||||||||||||||
| @ Html.tag "td" synopsis)) | ||||||||||||||||||
| @ Html.tag "td" synopsis | ||||||||||||||||||
| @ Html.tag "td" (Html.int pkg_nb_rev_depends) | ||||||||||||||||||
| @ Html.tag "td" (Html.int pkg_dependency_cone_size))) | ||||||||||||||||||
| :: acc) | ||||||||||||||||||
| [] | ||||||||||||||||||
| (List.rev sorted_packages) | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need a graph here? Isn't getting the cardinal of max disjunction sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean by "max disjuction". If we computed the dependency cone for each package in the switch independently (using, for ex, OpamSwitchState.dependencies) there'd be too much recomputation.
Argument
dependshere is already the representation of a directed graph (a map from pkg to list of pkgs, ie, an adjacency list representation). Using this, we can reuse OcamlGraph for traversals to avoid recomputation. Or we can write a custom traversal (but it's still a graph traversal)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, yeah i was originally thinking of a brute force approach + hashtable to avoid recomputation, but it is 25x slower if you take the whole universe. The topological graph approach is clever, neat approximation.