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

Add override_for to dep options #14156

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion lib/mix/lib/mix/dep.ex
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ defmodule Mix.Dep do
if dep.opts[:from_umbrella] || other.opts[:from_umbrella] do
"Please remove the conflicting options from your definition"
else
"Ensure they match or specify one of the above in your deps and set \"override: true\""
"Ensure they match or specify one of the above in your deps and set \"override: true\" or \"override_for:\""
end
end

Expand Down
6 changes: 5 additions & 1 deletion lib/mix/lib/mix/dep/converger.ex
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ defmodule Mix.Dep.Converger do
end

cond do
in_upper? && other_opts[:override] ->
in_upper? && (other_opts[:override] || override_for?(other_opts, app)) ->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was most confused in this section of the converge logic.

If a dep passes this check once, does it never fail/process again? I initially had a test where I wanted dependency resolution to fail if override_for: [:foo] is specified, but :baz also required the override.

{:match, list}

not converge?(other, dep) ->
Expand Down Expand Up @@ -331,6 +331,10 @@ defmodule Mix.Dep.Converger do
end
end

defp override_for?(opts, app) do
!!opts[:override_for] && app in opts[:override_for]
end

defp with_matching_only_and_targets(other, dep, in_upper?) do
%{opts: opts} = dep

Expand Down
39 changes: 39 additions & 0 deletions lib/mix/lib/mix/dep/fetcher.ex
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ defmodule Mix.Dep.Fetcher do
Mix.Dep.Lock.write(lock, opts)
mark_as_fetched(parent_deps)

evaluate_overrides(Enum.filter(deps, & &1.opts[:override_for]), deps)

# See if any of the deps diverged and abort.
show_diverged!(Enum.filter(all_deps, &Mix.Dep.diverged?/1))

Expand Down Expand Up @@ -156,6 +158,43 @@ defmodule Mix.Dep.Fetcher do
end)
end

defp evaluate_overrides([], _deps), do: :ok

defp evaluate_overrides(override_deps, deps) do
for override_dep <- override_deps,
dep <- deps,
dep_dependency <- dep.deps,
override_dep.app == dep_dependency.app do
overridden? = dep.app in override_dep.opts[:override_for]

{:ok, satisfied?} =
Mix.Dep.Loader.vsn_match(dep_dependency.requirement, override_dep.requirement, dep.app)

cond do
overridden? && satisfied? ->
show_expired_override(override_dep, dep.app)

not overridden? && not satisfied? ->
show_missing_override(override_dep, dep.app)

true ->
:ok
end
end

:ok
end

defp show_expired_override(dep, app) do
Mix.shell().info(
"Dependency #{Mix.Dep.format_dep(dep)} no longer requires override_for on #{app}"
)
end

defp show_missing_override(dep, app) do
Mix.shell().info("Dependency #{Mix.Dep.format_dep(dep)} requires override_for on #{app}")
end

defp show_diverged!([]), do: :ok

defp show_diverged!(deps) do
Expand Down
7 changes: 7 additions & 0 deletions lib/mix/lib/mix/tasks/deps.ex
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,13 @@ defmodule Mix.Tasks.Deps do
* `:override` - if set to `true` the dependency will override any other
definitions of itself by other dependencies

* `:override_for` - if set to a list of applications requiring the
override (see the `override` opt), warnings will be emitted when other
apps are added that also require an override, and when the override is no
longer required. This setting behaves similarly to `override: true` for
dependency resolution. It does not fail resolution when more apps require
an override.

* `:manager` - Mix can also compile Rebar3 and makefile projects
and can fetch sub dependencies of Rebar3 projects. Mix will
try to infer the type of project but it can be overridden with this
Expand Down
41 changes: 41 additions & 0 deletions lib/mix/test/mix/dep_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,47 @@ defmodule Mix.DepTest do
end
end

describe "override_for" do
test "warns when overrides are no longer needed for an app" do
deps = [
{:deps_repo, "0.1.0", path: "custom/deps_repo"},
{:git_repo, "0.1.0",
git: MixTest.Case.fixture_path("git_repo"), override_for: [:deps_repo]}
]

with_deps(deps, fn ->
in_fixture("deps_status", fn ->
Mix.Tasks.Deps.Get.run([])
Mix.Tasks.Deps.Compile.run([])
end)
end)

message =
"Dependency git_repo (#{fixture_path("git_repo")}) no longer requires override_for on deps_repo"

assert_received {:mix_shell, :info, [^message]}
end

test "errors when more overrides are needed for an app" do
deps = [
{:deps_repo, "0.1.0", path: "custom/deps_repo"},
{:git_repo, "0.2.0", git: MixTest.Case.fixture_path("git_repo"), override_for: []}
]

with_deps(deps, fn ->
in_fixture("deps_status", fn ->
Mix.Tasks.Deps.Get.run([])
Mix.Tasks.Deps.Compile.run([])
end)
end)

message =
"Dependency git_repo (#{fixture_path("git_repo")}) requires override_for on deps_repo"

assert_received {:mix_shell, :info, [^message]}
end
end

describe "app generation" do
test "considers runtime from current app on nested deps" do
Process.put(:custom_deps_git_repo_opts, runtime: false)
Expand Down