From 5c4d786e3459faf2ed12cd3d729bc59c5f60fc7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Meadows-J=C3=B6nsson?= Date: Wed, 14 Oct 2026 20:26:56 +0200 Subject: [PATCH] Keep parsed requirements in the registry server The parsed requirements were kept in the process dictionary of the process calling the registry, which is the Mix process or, with Mix.install/2, the user's own process, and stayed there after the registry closed. Parse the requirements of a release's dependencies in the registry server instead and keep them in its state until the registry closes. Unsatisfiable and invalid requirements come back as errors, so the caller still warns and raises as before. Copying the parsed constraints in the replies makes resolving hexpm's dependencies take 27 ms instead of 22 ms, against 37 ms without keeping them parsed. --- lib/hex/registry/server.ex | 84 ++++++++++++++++++++----------- test/hex/registry/server_test.exs | 37 ++++++++++++++ 2 files changed, 93 insertions(+), 28 deletions(-) diff --git a/lib/hex/registry/server.ex b/lib/hex/registry/server.ex index bb3a687b..725c0e81 100644 --- a/lib/hex/registry/server.ex +++ b/lib/hex/registry/server.ex @@ -71,29 +71,33 @@ defmodule Hex.Registry.Server do end end - # Registries repeat the same few hundred requirements across thousands of - # releases, so keep the constraints parsed in this process. Unsatisfiable - # requirements aren't kept so they still warn for each release. def dependency_constraint(package, version, name, requirement) do - requirement = requirement || ">= 0.0.0" - key = {__MODULE__, :constraint, requirement} + constraint(package, version, name, parse_requirement(requirement)) + end - case Process.get(key) do - nil -> - constraint = Hex.Solver.parse_constraint!(requirement) - Process.put(key, constraint) - constraint + defp constraint(_package, _version, _name, {:ok, constraint}), do: constraint - constraint -> - constraint - end + # Unsatisfiable requirements warn for each release that has them + defp constraint( + package, + version, + name, + {:error, %Hex.Solver.UnsatisfiableRequirementError{} = error} + ) do + warn_unsatisfiable(package, version, name, error) + %Hex.Solver.Constraints.Empty{} + end + + defp constraint(_package, _version, _name, {:error, error}), do: raise(error) + + defp parse_requirement(requirement) do + {:ok, Hex.Solver.parse_constraint!(requirement || ">= 0.0.0")} rescue - error in Hex.Solver.UnsatisfiableRequirementError -> - warn_unsatisfiable(package, version, name, error) - %Hex.Solver.Constraints.Empty{} + error in [Hex.Solver.UnsatisfiableRequirementError, Version.InvalidRequirementError] -> + {:error, error} end - defp solver_dependency(package, version, {_repo, name, app, _requirement, _optional} = dep) do + defp solver_dependency(package, version, {_repo, name, app, _parsed, _optional} = dep) do dependency = dependency(package, version, dep) if valid_app_name?(app) do @@ -104,7 +108,7 @@ defmodule Hex.Registry.Server do end end - defp app_dependency(package, version, {_repo, name, app, _requirement, _optional} = dep) do + defp app_dependency(package, version, {_repo, name, app, _parsed, _optional} = dep) do unless valid_app_name?(app) do Mix.raise(invalid_app_name_message(package, version, name, app)) end @@ -112,14 +116,14 @@ defmodule Hex.Registry.Server do dependency(package, version, dep) end - defp dependency(package, version, {repo, name, app, requirement, optional}) do + defp dependency(package, version, {repo, name, app, parsed, optional}) do validate_name(package, version, "repository", repo, &valid_repo_name?/1) validate_name(package, version, "package", name, &valid_package_name?/1) %{ repo: if(repo != "hexpm", do: repo), name: name, - constraint: dependency_constraint(package, version, name, requirement), + constraint: constraint(package, version, name, parsed), optional: optional, label: app } @@ -238,7 +242,8 @@ defmodule Hex.Registry.Server do pending_fun: nil, pending_policies: MapSet.new(), fetched_policies: MapSet.new(), - waiting_policies: %{} + waiting_policies: %{}, + constraints: %{} } end @@ -351,13 +356,14 @@ defmodule Hex.Registry.Server do end def handle_call({:dependencies, repo, package, version}, from, state) do - maybe_wait({repo, package}, from, state, fn -> + maybe_wait({repo, package}, from, state, fn state -> case lookup(state.ets, {:deps, repo || "hexpm", package, to_string(version)}) do nil -> - :error + {:error, state} deps -> - {:ok, deps} + {deps, constraints} = Enum.map_reduce(deps, state.constraints, &parse_dependency/2) + {{:ok, deps}, %{state | constraints: constraints}} end end) end @@ -415,9 +421,12 @@ defmodule Hex.Registry.Server do written? = write_result(result, repo, package, state) - Enum.each(replys, fn {from, fun} -> - GenServer.reply(from, fun.()) - end) + state = + Enum.reduce(replys, state, fn {from, fun}, state -> + {reply, state} = run_waiting(fun, state) + GenServer.reply(from, reply) + state + end) state = %{ state @@ -873,7 +882,8 @@ defmodule Hex.Registry.Server do cond do {repo, package} in state.fetched -> - {:reply, fun.(), state} + {reply, state} = run_waiting(fun, state) + {:reply, reply, state} {repo, package} in state.pending -> tuple = {from, fun} @@ -886,6 +896,24 @@ defmodule Hex.Registry.Server do end end + # A fun that needs the state, such as one parsing dependencies, takes it + # and returns it with the reply + defp run_waiting(fun, state) when is_function(fun, 0), do: {fun.(), state} + defp run_waiting(fun, state) when is_function(fun, 1), do: fun.(state) + + # Registries repeat the same few hundred requirements across thousands of + # releases, so keep them parsed while the registry is open + defp parse_dependency({repo, name, app, requirement, optional}, constraints) do + case constraints do + %{^requirement => parsed} -> + {{repo, name, app, parsed, optional}, constraints} + + %{} -> + parsed = parse_requirement(requirement) + {{repo, name, app, parsed, optional}, Map.put(constraints, requirement, parsed)} + end + end + defp maybe_wait_policy({repo, name}, from, state, fun) do repo = repo || "hexpm" diff --git a/test/hex/registry/server_test.exs b/test/hex/registry/server_test.exs index 130db0e8..6ed97c0c 100644 --- a/test/hex/registry/server_test.exs +++ b/test/hex/registry/server_test.exs @@ -61,6 +61,43 @@ defmodule Hex.Registry.ServerTest do assert_received {:mix_shell, :error, ["\e[33mPackage other_parent 1.0.0 can't be used" <> _]} end + test "dependencies/3 keeps parsed requirements in the registry, not the calling process" do + :sys.replace_state(Registry, fn %{ets: tid, fetched: fetched} = state -> + :ets.insert( + tid, + {{:deps, "hexpm", "parent", "1.0.0"}, [{"hexpm", "child", "child", "~> 1.0", false}]} + ) + + %{state | fetched: MapSet.put(fetched, {"hexpm", "parent"})} + end) + + keys = Enum.sort(Process.get_keys()) + assert {:ok, [dependency]} = Registry.dependencies("hexpm", "parent", "1.0.0") + assert Enum.sort(Process.get_keys()) == keys + + assert %{constraints: %{"~> 1.0" => {:ok, constraint}}} = :sys.get_state(Registry) + assert dependency.constraint == constraint + end + + test "dependencies/3 raises for an invalid requirement and keeps the registry running" do + :sys.replace_state(Registry, fn %{ets: tid, fetched: fetched} = state -> + :ets.insert( + tid, + {{:deps, "hexpm", "parent", "1.0.0"}, [{"hexpm", "child", "child", "~> foo", false}]} + ) + + %{state | fetched: MapSet.put(fetched, {"hexpm", "parent"})} + end) + + registry = Process.whereis(Registry) + + assert_raise Version.InvalidRequirementError, fn -> + Registry.dependencies("hexpm", "parent", "1.0.0") + end + + assert Process.whereis(Registry) == registry + end + test "app_dependencies/3 accepts application names" do :sys.replace_state(Registry, fn %{ets: tid, fetched: fetched} = state -> :ets.insert(